From 07b2d9043121b3f8b6b4377fb006ab10d31e6611 Mon Sep 17 00:00:00 2001
From: theodab <theodab@google.com>
Date: Thu, 4 Jan 2018 09:43:08 -0800
Subject: [PATCH] Prefer no role text streams when no preferred role

Beforehand, if there were some text streams with a given language
which had a role, and some which did not, the no-role text streams
would never be chosen.
Now, they are chosen if there is no preferred role.

Closes #1212

Change-Id: I28b426304b1a828f500012425b8d89c94ccc3178
---
 lib/util/stream_utils.js       |  7 ++++++
 test/util/stream_utils_unit.js | 42 ++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/lib/util/stream_utils.js b/lib/util/stream_utils.js
index 8fa81316b0..59c3e53693 100644
--- a/lib/util/stream_utils.js
+++ b/lib/util/stream_utils.js
@@ -637,6 +637,13 @@ shaka.util.StreamUtils.filterStreamsByLanguageAndRole = function(
     } else {
       shaka.log.warning('No exact match for the text role could be found.');
     }
+  } else {
+    // Prefer text streams with no roles, if they exist.
+    var noRoleMatches = chosen.filter(function(stream) {
+      return stream.roles.length == 0;
+    });
+    if (noRoleMatches.length)
+      return noRoleMatches;
   }
 
   // Either there was no role preference, or it could not be satisfied.
diff --git a/test/util/stream_utils_unit.js b/test/util/stream_utils_unit.js
index a1677b324b..ebd91642c7 100644
--- a/test/util/stream_utils_unit.js
+++ b/test/util/stream_utils_unit.js
@@ -336,6 +336,48 @@ describe('StreamUtils', function() {
       expect(chosen[0]).toBe(manifest.periods[0].textStreams[0]);
     });
 
+    it('prefers no-role streams if there is no preferred role', function() {
+      manifest = new shaka.test.ManifestGenerator()
+        .addPeriod(0)
+          .addTextStream(0)
+            .language('en')
+            .roles(['commentary'])
+          .addTextStream(1)
+            .language('en')
+          .addTextStream(2)
+            .language('en')
+            .roles(['secondary'])
+        .build();
+
+      var chosen = filterStreamsByLanguageAndRole(
+          manifest.periods[0].textStreams,
+          'en',
+          '');
+      expect(chosen.length).toBe(1);
+      expect(chosen[0].roles.length).toBe(0); // Pick a stream with no role.
+    });
+
+    it('ignores no-role streams if there is a preferred role', function() {
+      manifest = new shaka.test.ManifestGenerator()
+        .addPeriod(0)
+          .addTextStream(0)
+            .language('en')
+            .roles(['commentary'])
+          .addTextStream(1)
+            .language('en')
+          .addTextStream(2)
+            .language('en')
+            .roles(['secondary'])
+        .build();
+
+      var chosen = filterStreamsByLanguageAndRole(
+          manifest.periods[0].textStreams,
+          'en',
+          'main'); // A role that is not present.
+      expect(chosen.length).toBe(1);
+      expect(chosen[0].roles.length).toBe(1); // Pick a stream with a role.
+    });
+
     it('chooses only one role, even if none is preferred', function() {
       // Regression test for https://github.com/google/shaka-player/issues/949
       manifest = new shaka.test.ManifestGenerator()