fix: Fix handling of role-less audio tracks

These two issues turned out to be related:

1. No way to select a track without a particular role (shaka-project#2906)
2. Player should default to role-less audio tracks (shaka-project#2909)

Before this fix, the Player would default to the first audio track
with a language match, which may have been an audio description track
or commentary track.  If the user doesn't have a preference of track
roles, we should default to a role-less track when possible.

There was also no way to explicitly select a role-less track in
selectAudioLanguage.  Before this fix, a call without a role would
effect a language-only match, ignoring roles and potentially selecting
a description or commentary track.

Now, not specifying a role (either in initial preferences or in
selectAudioLanguage) will result in a preference for role-less tracks
when possible.

Fixes shaka-project#2906
Fixes shaka-project#2909

Change-Id: Ie6957eaecd1dcf9b7b65d62afa8ba9d9a5bdefe9

lib/media/adaptation_set_criteria.js

@@ -88,9 +88,8 @@ shaka.media.PreferenceBasedCriteria = class {
    * @param {string} role
    * @param {number} channelCount
    * @param {string=} label
-   * @param {string=} type
    */
-  constructor(language, role, channelCount, label = '', type = '') {
+  constructor(language, role, channelCount, label = '') {
     /** @private {string} */
     this.language_ = language;
     /** @private {string} */
@@ -99,8 +98,6 @@ shaka.media.PreferenceBasedCriteria = class {
     this.channelCount_ = channelCount;
     /** @private {string} */
     this.label_ = label;
-    /** @private {string} */
-    this.type_ = type;
   }
 
   /** @override */
@@ -121,15 +118,13 @@ shaka.media.PreferenceBasedCriteria = class {
       current = variants;
     }
 
-    // Now refine the choice based on role preference.
-    if (this.role_) {
-      const byRole = Class.filterVariantsByRole_(current, this.role_,
-          this.type_);
-      if (byRole.length) {
-        current = byRole;
-      } else {
-        shaka.log.warning('No exact match for variant role could be found.');
-      }
+    // Now refine the choice based on role preference.  Even the empty string
+    // works here, and will match variants without any roles.
+    const byRole = Class.filterVariantsByRole_(current, this.role_);
+    if (byRole.length) {
+      current = byRole;
+    } else {
+      shaka.log.warning('No exact match for variant role could be found.');
     }
 
     if (this.channelCount_) {
@@ -196,20 +191,19 @@ shaka.media.PreferenceBasedCriteria = class {
    *
    * @param {!Array.<shaka.extern.Variant>} variants
    * @param {string} preferredRole
-   * @param {string} type
    * @return {!Array.<shaka.extern.Variant>}
    * @private
    */
-  static filterVariantsByRole_(variants, preferredRole, type) {
+  static filterVariantsByRole_(variants, preferredRole) {
     return variants.filter((variant) => {
-      if (type) {
-        const stream = variant[type];
-        return stream && stream.roles.includes(preferredRole);
+      if (!variant.audio) {
+        return false;
+      }
+
+      if (preferredRole) {
+        return variant.audio.roles.includes(preferredRole);
       } else {
-        const audio = variant.audio;
-        const video = variant.video;
-        return (audio && audio.roles.includes(preferredRole)) ||
-               (video && video.roles.includes(preferredRole));
+        return variant.audio.roles.length == 0;
       }
     });
   }

lib/player.js

@@ -3380,22 +3380,42 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
     if (this.manifest_ && this.playhead_) {
       this.currentAdaptationSetCriteria_ =
           new shaka.media.PreferenceBasedCriteria(language, role || '',
-          /* channelCount= */ 0, /* label= */ '', /* type= */ 'audio');
+          /* channelCount= */ 0, /* label= */ '');
 
       this.chooseVariantAndSwitch_();
     } else if (this.video_ && this.video_.audioTracks) {
       const audioTracks = Array.from(this.video_.audioTracks);
       const selectedLanguage = LanguageUtils.normalize(language);
 
+      let languageMatch = null;
+      let languageAndRoleMatch = null;
+
       for (const audioTrack of audioTracks) {
         const track = shaka.util.StreamUtils.html5AudioTrackToTrack(audioTrack);
-        if (LanguageUtils.normalize(track.language) == selectedLanguage &&
-          (!role || track.roles.includes(role))) {
-          // This will reset the "enabled" of other tracks to false.
-          audioTrack.enabled = true;
+
+        if (LanguageUtils.normalize(track.language) == selectedLanguage) {
+          languageMatch = audioTrack;
+
+          if (role) {
+            if (track.roles.includes(role)) {
+              languageAndRoleMatch = audioTrack;
+            }
+          } else {  // no role
+            if (track.roles.length == 0) {
+              languageAndRoleMatch = audioTrack;
+            }
+          }
         }
       }
-      this.onVariantChanged_();
+
+      // This will reset the "enabled" of other tracks to false.
+      if (languageAndRoleMatch) {
+        languageAndRoleMatch.enabled = true;
+        this.onVariantChanged_();
+      } else if (languageMatch) {
+        languageMatch.enabled = true;
+        this.onVariantChanged_();
+      }
     }
   }
 

test/player_unit.js

@@ -1527,6 +1527,67 @@ describe('Player', () => {
       expect(getActiveTextTrack().id).toBe(spanishTextTrack.id);
     });
 
+    // Regression test for https://github.com/google/shaka-player/issues/2906
+    // and https://github.com/google/shaka-player/issues/2909.
+    it('selectAudioLanguage() can choose role-less tracks', async () => {
+      // For this test, we use a different (and simpler) manifest.
+      // Both audio tracks are English; one has a role, and one has no roles.
+      // The role=description track comes first to reproduce the conditions in
+      // #2909.
+      manifest = shaka.test.ManifestGenerator.generate((manifest) => {
+        manifest.addVariant(100, (variant) => {
+          variant.bandwidth = 1300;
+          variant.language = 'en';
+          variant.addVideo(1, (stream) => {
+            stream.originalId = 'video';
+            stream.bandwidth = 1000;
+            stream.width = 100;
+            stream.height = 200;
+            stream.frameRate = 1000000 / 42000;
+            stream.pixelAspectRatio = '59:54';
+            stream.roles = [];
+          });
+          variant.addAudio(2, (stream) => {
+            stream.originalId = 'audio-en-description';
+            stream.bandwidth = 100;
+            stream.channelsCount = 2;
+            stream.audioSamplingRate = 48000;
+            stream.roles = ['description'];
+          });
+        });
+        manifest.addVariant(101, (variant) => {
+          variant.bandwidth = 2300;
+          variant.language = 'en';
+          variant.addExistingStream(1);  // video
+          variant.addAudio(3, (stream) => {
+            stream.originalId = 'audio-en';
+            stream.bandwidth = 100;
+            stream.channelsCount = 2;
+            stream.audioSamplingRate = 48000;
+            stream.roles = [];
+          });
+        });
+      });
+
+      // No explicit preferred audio language is also part of #2909.
+      player.configure('preferredAudioLanguage', undefined);
+
+      // Load again to get this test-specific manifest loaded.
+      await player.load(fakeManifestUri, 0, fakeMimeType);
+
+      // #2909: The initial choice should be for the role-less track, even
+      // though it is second in the manifest.
+      expect(getActiveVariantTrack().audioRoles).toEqual([]);
+
+      player.selectAudioLanguage('en', 'description');
+      expect(getActiveVariantTrack().audioRoles).toEqual(['description']);
+
+      // #2906: Selecting no particular role should prefer the track without any
+      // roles.
+      player.selectAudioLanguage('en');
+      expect(getActiveVariantTrack().audioRoles).toEqual([]);
+    });
+
     it('selectTextLanguage() does not change selected variant track', () => {
       // This came up in a custom application that allows to select
       // from all tracks regardless of selected language.