attach attributes property to playlist objects in cases the m3u8-pars…

src/master-playlist-controller.js

@@ -836,7 +836,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
     let videoPlaylist = this.masterPlaylistLoader_.media();
     let result;
 
-    if (videoPlaylist.attributes && videoPlaylist.attributes.AUDIO) {
+    if (videoPlaylist.attributes.AUDIO) {
       result = this.audioGroups_[videoPlaylist.attributes.AUDIO];
     }
 
@@ -855,7 +855,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
       return null;
     }
 
-    if (videoPlaylist.attributes && videoPlaylist.attributes.SUBTITLES) {
+    if (videoPlaylist.attributes.SUBTITLES) {
       result = this.subtitleGroups_.groups[videoPlaylist.attributes.SUBTITLES];
     }
 
@@ -1619,7 +1619,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
     let videoCodec = null;
     let codecs;
 
-    if (media.attributes && media.attributes.CODECS) {
+    if (media.attributes.CODECS) {
       codecs = parseCodecs(media.attributes.CODECS);
       videoCodec = codecs.videoCodec;
       codecCount = codecs.codecCount;
@@ -1630,7 +1630,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
         videoCodec: null
       };
 
-      if (variant.attributes && variant.attributes.CODECS) {
+      if (variant.attributes.CODECS) {
         let codecString = variant.attributes.CODECS;
 
         variantCodecs = parseCodecs(codecString);

src/playlist-loader.js

@@ -6,7 +6,7 @@
  *
  */
 import resolveUrl from './resolve-url';
-import { mergeOptions, EventTarget } from 'video.js';
+import { mergeOptions, EventTarget, log } from 'video.js';
 import { isEnabled } from './playlist.js';
 import m3u8 from 'm3u8-parser';
 import window from 'global/window';
@@ -172,6 +172,9 @@ const PlaylistLoader = function(srcUrl, hls, withCredentials) {
     parser.push(xhr.responseText);
     parser.end();
     parser.manifest.uri = url;
+    // m3u8-parser does not attach an attributes property to media playlists so make
+    // sure that the property is attached to avoid undefined reference errors
+    parser.manifest.attributes = parser.manifest.attributes || {};
 
     // merge this playlist into the master
     update = updateMaster(loader.master, parser.manifest);
@@ -250,12 +253,7 @@ const PlaylistLoader = function(srcUrl, hls, withCredentials) {
         return false;
       }
 
-      let bandwidth = 0;
-
-      if (playlist && playlist.attributes) {
-        bandwidth = playlist.attributes.BANDWIDTH;
-      }
-      return bandwidth < currentBandwidth;
+      return (playlist.attributes.BANDWIDTH || 0) < currentBandwidth;
 
     }).length === 0);
   };
@@ -510,6 +508,18 @@ const PlaylistLoader = function(srcUrl, hls, withCredentials) {
           playlist = loader.master.playlists[i];
           loader.master.playlists[playlist.uri] = playlist;
           playlist.resolvedUri = resolveUrl(loader.master.uri, playlist.uri);
+
+          if (!playlist.attributes) {
+            // Although the spec states an #EXT-X-STREAM-INF tag MUST have a
+            // BANDWIDTH attribute, we can play the stream without it. This means a poorly
+            // formatted master playlist may not have an attribute list. An attributes
+            // property is added here to prevent undefined references when we encounter
+            // this scenario.
+            playlist.attributes = {};
+
+            log.warn(
+              'Invalid playlist STREAM-INF detected. Missing BANDWIDTH attribute.');
+          }
         }
 
         // resolve any media group URIs
@@ -552,6 +562,9 @@ const PlaylistLoader = function(srcUrl, hls, withCredentials) {
       };
       loader.master.playlists[srcUrl] = loader.master.playlists[0];
       loader.master.playlists[0].resolvedUri = srcUrl;
+      // m3u8-parser does not attach an attributes property to media playlists so make
+      // sure that the property is attached to avoid undefined reference errors
+      loader.master.playlists[0].attributes = loader.master.playlists[0].attributes || {};
       haveMetadata(req, srcUrl);
       return loader.trigger('loadedmetadata');
     });

src/playlist-selectors.js

@@ -62,11 +62,11 @@ export const comparePlaylistBandwidth = function(left, right) {
   let leftBandwidth;
   let rightBandwidth;
 
-  if (left.attributes && left.attributes.BANDWIDTH) {
+  if (left.attributes.BANDWIDTH) {
     leftBandwidth = left.attributes.BANDWIDTH;
   }
   leftBandwidth = leftBandwidth || window.Number.MAX_VALUE;
-  if (right.attributes && right.attributes.BANDWIDTH) {
+  if (right.attributes.BANDWIDTH) {
     rightBandwidth = right.attributes.BANDWIDTH;
   }
   rightBandwidth = rightBandwidth || window.Number.MAX_VALUE;
@@ -87,16 +87,14 @@ export const comparePlaylistResolution = function(left, right) {
   let leftWidth;
   let rightWidth;
 
-  if (left.attributes &&
-      left.attributes.RESOLUTION &&
+  if (left.attributes.RESOLUTION &&
       left.attributes.RESOLUTION.width) {
     leftWidth = left.attributes.RESOLUTION.width;
   }
 
   leftWidth = leftWidth || window.Number.MAX_VALUE;
 
-  if (right.attributes &&
-      right.attributes.RESOLUTION &&
+  if (right.attributes.RESOLUTION &&
       right.attributes.RESOLUTION.width) {
     rightWidth = right.attributes.RESOLUTION.width;
   }
@@ -135,11 +133,9 @@ const simpleSelector = function(master, playerBandwidth, playerWidth, playerHeig
     let height;
     let bandwidth;
 
-    if (playlist.attributes) {
-      width = playlist.attributes.RESOLUTION && playlist.attributes.RESOLUTION.width;
-      height = playlist.attributes.RESOLUTION && playlist.attributes.RESOLUTION.height;
-      bandwidth = playlist.attributes.BANDWIDTH;
-    }
+    width = playlist.attributes.RESOLUTION && playlist.attributes.RESOLUTION.width;
+    height = playlist.attributes.RESOLUTION && playlist.attributes.RESOLUTION.height;
+    bandwidth = playlist.attributes.BANDWIDTH;
 
     bandwidth = bandwidth || window.Number.MAX_VALUE;
 

src/rendition-mixin.js

@@ -54,21 +54,16 @@ class Representation {
                               .fastQualityChange_
                               .bind(hlsHandler.masterPlaylistController_);
 
-    // Carefully descend into the playlist's attributes since most
-    // properties are optional
-    if (playlist.attributes) {
-      let attributes = playlist.attributes;
+    // some playlist attributes are optional
+    if (playlist.attributes.RESOLUTION) {
+      const resolution = playlist.attributes.RESOLUTION;
 
-      if (attributes.RESOLUTION) {
-        let resolution = attributes.RESOLUTION;
-
-        this.width = resolution.width;
-        this.height = resolution.height;
-      }
-
-      this.bandwidth = attributes.BANDWIDTH;
+      this.width = resolution.width;
+      this.height = resolution.height;
     }
 
+    this.bandwidth = playlist.attributes.BANDWIDTH;
+
     // The id is simply the ordinality of the media playlist
     // within the master playlist
     this.id = id;

src/segment-loader.js

@@ -753,7 +753,7 @@ export default class SegmentLoader extends videojs.EventTarget {
         //       the lowestEnabledRendition.
         !this.xhrOptions_.timeout ||
         // Don't abort if we have no bandwidth information to estimate segment sizes
-        !(this.playlist_.attributes && this.playlist_.attributes.BANDWIDTH)) {
+        !(this.playlist_.attributes.BANDWIDTH)) {
       return false;
     }
 

test/playlist-loader.test.js

@@ -67,12 +67,34 @@ QUnit.test('moves to HAVE_MASTER after loading a master playlist', function(asse
   });
   this.requests.pop().respond(200, null,
                               '#EXTM3U\n' +
-                              '#EXT-X-STREAM-INF:\n' +
+                              '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                               'media.m3u8\n');
   assert.ok(loader.master, 'the master playlist is available');
   assert.strictEqual(state, 'HAVE_MASTER', 'the state at loadedplaylist correct');
 });
 
+QUnit.test('logs warning for master playlist with invalid STREAM-INF', function(assert) {
+  let loader = new PlaylistLoader('master.m3u8', this.fakeHls);
+
+  loader.load();
+
+  this.requests.pop().respond(200, null,
+                              '#EXTM3U\n' +
+                              '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
+                              'video1/media.m3u8\n' +
+                              '#EXT-X-STREAM-INF:\n' +
+                              'video2/media.m3u8\n');
+
+  assert.ok(loader.master, 'infers a master playlist');
+  assert.equal(loader.master.playlists[1].uri, 'video2/media.m3u8',
+    'parsed invalid stream');
+  assert.ok(loader.master.playlists[1].attributes, 'attached attributes property');
+  assert.equal(this.env.log.warn.calls, 1, 'logged a warning');
+  assert.equal(this.env.log.warn.args[0],
+    'Invalid playlist STREAM-INF detected. Missing BANDWIDTH attribute.',
+    'logged a warning');
+});
+
 QUnit.test('jumps to HAVE_METADATA when initialized with a media playlist',
 function(assert) {
   let loadedmetadatas = 0;
@@ -91,6 +113,7 @@ function(assert) {
   assert.ok(loader.master, 'infers a master playlist');
   assert.ok(loader.media(), 'sets the media playlist');
   assert.ok(loader.media().uri, 'sets the media playlist URI');
+  assert.ok(loader.media().attributes, 'sets the media playlist attributes');
   assert.strictEqual(loader.state, 'HAVE_METADATA', 'the state is correct');
   assert.strictEqual(this.requests.length, 0, 'no more requests are made');
   assert.strictEqual(loadedmetadatas, 1, 'fired one loadedmetadata');
@@ -103,7 +126,7 @@ QUnit.test('resolves relative media playlist URIs', function(assert) {
 
   this.requests.shift().respond(200, null,
                                 '#EXTM3U\n' +
-                                '#EXT-X-STREAM-INF:\n' +
+                                '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                                 'video/media.m3u8\n');
   assert.equal(loader.master.playlists[0].resolvedUri, urlTo('video/media.m3u8'),
               'resolved media URI');
@@ -117,9 +140,9 @@ function(assert) {
 
   this.requests.shift().respond(200, null,
                                 '#EXTM3U\n' +
-                                '#EXT-X-STREAM-INF:\n' +
+                                '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                                 'video1/media.m3u8\n' +
-                                '#EXT-X-STREAM-INF:\n' +
+                                '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                                 'video2/media.m3u8\n');
   assert.equal(loader.enabledPlaylists_(), 2, 'Returned initial amount of playlists');
   loader.master.playlists[0].excludeUntil = Date.now() + 100000;
@@ -133,9 +156,9 @@ QUnit.test('playlist loader detects if we are on lowest rendition', function(ass
   loader.load();
   this.requests.shift().respond(200, null,
                                 '#EXTM3U\n' +
-                                '#EXT-X-STREAM-INF:\n' +
+                                '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                                 'video1/media.m3u8\n' +
-                                '#EXT-X-STREAM-INF:\n' +
+                                '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                                 'video2/media.m3u8\n');
   loader.media = function() {
     return {attributes: {BANDWIDTH: 10}};
@@ -182,7 +205,7 @@ QUnit.test('recognizes absolute URIs and requests them unmodified', function(ass
 
   this.requests.shift().respond(200, null,
                                 '#EXTM3U\n' +
-                                '#EXT-X-STREAM-INF:\n' +
+                                '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                                 'http://example.com/video/media.m3u8\n');
   assert.equal(loader.master.playlists[0].resolvedUri,
               'http://example.com/video/media.m3u8', 'resolved media URI');
@@ -203,7 +226,7 @@ QUnit.test('recognizes domain-relative URLs', function(assert) {
 
   this.requests.shift().respond(200, null,
                                 '#EXTM3U\n' +
-                                '#EXT-X-STREAM-INF:\n' +
+                                '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                                 '/media.m3u8\n');
   assert.equal(loader.master.playlists[0].resolvedUri,
               window.location.protocol + '//' +
@@ -318,6 +341,7 @@ function(assert) {
                               '0.ts\n');
   assert.ok(loader.master, 'infers a master playlist');
   assert.ok(loader.media(), 'sets the media playlist');
+  assert.ok(loader.media().attributes, 'sets the media playlist attributes');
   assert.strictEqual(loader.state, 'HAVE_METADATA', 'the state is correct');
 });
 
@@ -336,8 +360,9 @@ QUnit.test('moves to HAVE_METADATA after loading a media playlist', function(ass
   });
   this.requests.pop().respond(200, null,
                               '#EXTM3U\n' +
-                              '#EXT-X-STREAM-INF:\n' +
+                              '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                               'media.m3u8\n' +
+                              '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                               'alt.m3u8\n');
   assert.strictEqual(loadedPlaylist, 1, 'fired loadedplaylist once');
   assert.strictEqual(loadedMetadata, 0, 'did not fire loadedmetadata');
@@ -436,7 +461,7 @@ QUnit.test('errors when an initial media playlist request fails', function(asser
   });
   this.requests.pop().respond(200, null,
                               '#EXTM3U\n' +
-                              '#EXT-X-STREAM-INF:\n' +
+                              '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
                               'media.m3u8\n');
 
   assert.strictEqual(errors.length, 0, 'emitted no errors');

test/rendition-mixin.test.js

@@ -8,24 +8,19 @@ const makeMockPlaylist = function(options) {
   options = options || {};
 
   let playlist = {
-    segments: []
+    segments: [],
+    attributes: {}
   };
 
-  if ('bandwidth' in options) {
-    playlist.attributes = playlist.attributes || {};
-
-    playlist.attributes.BANDWIDTH = options.bandwidth;
-  }
+  playlist.attributes.BANDWIDTH = options.bandwidth;
 
   if ('width' in options) {
-    playlist.attributes = playlist.attributes || {};
     playlist.attributes.RESOLUTION = playlist.attributes.RESOLUTION || {};
 
     playlist.attributes.RESOLUTION.width = options.width;
   }
 
   if ('height' in options) {
-    playlist.attributes = playlist.attributes || {};
     playlist.attributes.RESOLUTION = playlist.attributes.RESOLUTION || {};
 
     playlist.attributes.RESOLUTION.height = options.height;