Initial DASH Support #8

gesinger · 2017-11-07T23:16:01Z

Description

Initial DASH support with DRM support via videojs-contrib-eme

Will update the documentation, but wanted to put the PR up for early review.

Requirements Checklist

Feature implemented / Bug fixed
If necessary, more likely in a feature request than a bug fix
- Unit Tests updated or fixed
- Docs/guides updated
- Example created (starter template on JSBin)
Reviewed by Two Core Contributors

…ce buffers

…before appending data

…reation

…aylist loaders

…out source buffer emitter

forbesjo · 2017-11-07T23:24:45Z

It would be nice to have videojs-contrib-eme as a dev dependency and included in the test pages

mjneil · 2017-11-07T23:27:34Z

src/videojs-http-streaming.js

+    return options;
+  }
+
+  // upsert the content types based on the selected playlist


mjneil · 2017-11-07T23:31:59Z

src/videojs-http-streaming.js

@@ -418,6 +455,18 @@ class HlsHandler extends Component {
    this.masterPlaylistController_.on('selectedinitialmedia', () => {
      // Add the manual rendition mix-in to HlsHandler
      renditionSelectionMixin(this);
+
+      if (this.options_.sourceType === 'dash') {


It'd be nice to put this in like a this.setupEmeOptions_()

mjneil · 2017-11-07T23:39:12Z

src/master-playlist-controller.js

+      // if the first mime type has muxed video and audio then we shouldn't wait on the
+      // second source buffer
+      mimeTypes.length > 1 && mimeTypes[0].indexOf(',') === -1 ?
+        new videojs.EventTarget() : null;


we might want to think about expanding this emitter concept to its own controller class. When we start tearing apart flash and pulling the muxer into vhs and removing contrib-media-sources, we will still need a place for some of the contrib-media-sources logic to live e.g. buffer management logic that was added to address the edge/ie corruption issues

I agree that we may end up needing that, but for now I think it's best to hold off. We may not get to it for a while, and if we do get to it sooner, it may end up having different requirements than we suppose right now.

OshinKaramian · 2017-11-08T19:21:24Z

src/dash-playlist-loader.js

+    }
+  }
+
+  media(playlist) {


Would it be possible to get function comments here? Mostly I'm asking as this has a few pieces of nuance (the playlist can be a string or object, and thus will be handled differently, as well as the fact that it is both a getter and a setter).

OshinKaramian · 2017-11-08T19:26:37Z

src/dash-playlist-loader.js

+
+    const mediaChange = !this.media_ || playlist.uri !== this.media_.uri;
+
+    this.state = 'HAVE_METADATA';


Are these states doc'ed anywhere? Might be nice to have them doc'ed in some form, as it might make debugging easier (particularly if there is a sequence to these events).

these states mimic the PlaylistLoader states
https://github.com/videojs/http-streaming/blob/master/docs/arch.md

Ah nice, this is what I was looking for.

mjneil · 2017-11-08T22:37:30Z

src/dash-playlist-loader.js

+      for (let i = 0; i < this.master.playlists.length; i++) {
+        const phonyUri = `placeholder-uri-${i}`;
+
+        this.master.playlists[i].uri = phonyUri;


should also add resolvedUri here, quite a few places outside of this class use resolvedUri instead of uri

maybe not quite a few since media-groups has source type checking now, but there is a spot in MPC and external plugins may look for it

I forgot, it actually gets added in setupMediaPlaylists

…fied on media

Note: requires linking or building videojs-contrib-eme (to ensure a build in dist/)

mjneil · 2017-11-21T21:01:21Z

src/sync-controller.js

@@ -516,7 +516,7 @@ export default class SyncController extends videojs.EventTarget {
        time: segment.start,
        accuracy: 0
      };
-    } else if (playlist.discontinuityStarts.length) {
+    } else if (playlist.discontinuityStarts && playlist.discontinuityStarts.length) {


This will probably depend on how we plan to handle multi-period dash, and this extra check isn't problematic in anyway, but m3u8-parser always attaches an empty array for discontinuityStarts. If we plan to treat multi-period dash as discontinuities, then this check may be unnecessary, but still ok to be there

Good call. I think we can just leave it in until we handle multi-period dash in the mpd-parser (so we know for sure).

forbesjo · 2017-11-27T21:45:53Z

src/videojs-http-streaming.js

+  const sourceType = simpleTypeFromSourceType(type);
+
+  if (sourceType === 'dash') {
+    if (!options.hls.overrideNative && Hls.supportsNativeDash) {


I think we should always override native DASH playback

forbesjo · 2017-11-28T21:05:35Z

src/videojs-http-streaming.js

+      // isn't the most appropriate form of reference for video.js (since all APIs should
+      // be provided through core video.js), it is a common pattern for plugins, and vhs
+      // will act accordingly.
+      _player.vhs = this;


We need this to map to deprecated player.dash and player.hls.

Did we have a player.hls? I though just player.tech_.hls

Uses could access player.hls.representations

I forgot that it's already available (albeit deprecated). Since it still exists they should be able to continue to use it without issue.

…e is on

forbesjo

Requires a videojs-contrib-eme version

forbesjo · 2017-12-12T17:25:37Z

package.json

@@ -92,6 +92,7 @@
    "global": "^4.3.0",
    "m3u8-parser": "2.1.0",
    "mux.js": "4.3.2",
+    "mpd-parser": "0.0.1",


mpd-parser is now on 0.2.0

forbesjo · 2017-12-12T17:26:02Z

package.json

@@ -134,6 +135,7 @@
    "shelljs": "^0.5.3",
    "sinon": "1.10.2",
    "uglify-js": "^2.5.0",
+    "videojs-contrib-eme": "http://github.com/gesinger/videojs-contrib-eme#options-rework",


Remember to point to a contrib-eme version with the necessary changes

forbesjo · 2017-12-13T21:21:40Z

Looks good to me

mjneil · 2017-12-13T21:23:58Z

README.md

+Lead Maintainers:
+- Jon-Carlos Rivera [@imbcmdth](https://github.com/imbcmdth)
+- Joe Forbes [@forbesjo](https://github.com/forbesjo)
+- Matthew Neil [@mjneil](https://github.com/mjneil)


mjneil · 2017-12-13T21:27:32Z

src/master-playlist-controller.js

@@ -105,6 +106,29 @@ const getCodecs = function(media) {
  return defaultCodecs;
 };

+const audioProfileFromDefault = (master, audioGroupId) => {


Maybe this would be a good opportunity to move this and other codec related helper functions into util/codecs.js to help cleanup this file a bit?

mjneil · 2017-12-13T21:33:32Z

src/videojs-http-streaming.js

+  };
+};
+
+const setupEmeOptions = (hlsHandler) => {


probably would be better to use vhsHandler naming

just realized, are we not making name changes for HlsHandler my hls->vhs name change suggestions probably shouldn't be done then

mjneil · 2017-12-13T21:34:24Z

src/videojs-http-streaming.js

@@ -546,7 +622,7 @@ const HlsSourceHandler = function(mode) {
      tech.hls = new HlsHandler(source, tech, localOptions);


Do we want to make this be tech.vhs and point tech.hls = tech.vhs for backwards comp?

mjneil · 2017-12-13T21:58:22Z

utils/manifest/dash.mpd

+<MPD xmlns="urn:mpeg:dash:schema:mpd:2011" profiles="urn:mpeg:dash:profile:full:2011" minBufferTime="1.5" mediaPresentationDuration="PT4S">
+  <Period>
+    <BaseURL>main/</BaseURL>
+    <AdaptationSet mimeType="video/mp4">


Should we add an AdaptionSet for vtt since mpd-parser 0.2.0 supports vtt now?

forbesjo · 2017-12-14T16:24:57Z

Going to merge this and open the suggested comments in a new PR.

chrome canary support

…e-updater-duration Use source updater to update media source duration

gesinger and others added 22 commits November 7, 2017 18:00

Working commit for dash support

f2daf93

Working commit including some cleanup of dash support

ec36b88

set up demo page for testing

3d3f28d

use installed parser

c02746d

pass along source uri to resolve segments

afd4166

pass along source type

d975099

FIGURE OUT: check source type in monitor buffer tick

e0d51bb

Remove unused variable and unnecessary new from dash playlist loader

6052320

Pass source type to segment loader

416af97

Add mpd-parser to package.json

8f6d062

Fix media groups tests by adding source type for initialization

7b2c72f

Detect and configure EME for DASH

56aa7fc

Tell segment loader whether content is demuxed so it can wait on sour…

d6a9342

…ce buffers

In the case of demuxed audio and video, wait for both source buffers …

4d5d73a

…before appending data

Add tests for canHandleSource and canPlayType DASH support

2a52a3e

Add tests for simpleTypeFromSourceType

7943f57

Add test for proper PlaylistLoader creation on MasterPlaylistLoader c…

341b5c3

…reation

Add test for dash mime types

6895fe3

Add tests for intializing AUDIO and SUBTITLES groups with HLS/DASH pl…

2409edc

…aylist loaders

Add tests for configuring mime types on segment loaders with and with…

3a611aa

…out source buffer emitter

Add tests for source buffer emitter async updates in source updater

84e5e6a

Add tests for DashPlaylistLoader and fix phony URIs

e029754

mjneil reviewed Nov 7, 2017

View reviewed changes

src/videojs-http-streaming.js Outdated

return options;

}

// upsert the content types based on the selected playlist

Copy link

Contributor

mjneil Nov 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upsert

mjneil reviewed Nov 7, 2017

View reviewed changes

OshinKaramian reviewed Nov 8, 2017

View reviewed changes

mjneil reviewed Nov 8, 2017

View reviewed changes

gesinger added 2 commits November 14, 2017 15:34

Use codecs specified in default alternate audio playlist if not speci…

f8fdfdd

…fied on media

Require overrideNative to play DASH on browsers that support it natively

796cf8b

gesinger added 8 commits November 15, 2017 19:44

Add videojs-contrib-eme as a dev dependency and include it on test pages

9f1140f

Rework videojs-contrib-eme options based on pending branch

2d13405

Note: requires linking or building videojs-contrib-eme (to ensure a build in dist/)

Trigger loadedmetadata async from playlist loader to mimic HLS behavior

7fc599a

Fix a couple DASH playlist loader test failures

4857242

Resolve async calls before leaving media group tests

6c24a61

Update header of README

7e93a0e

Move setup of videojs-contrib-eme options to its own function

396d065

Add player.vhs reference to HlsHandler instance

8bec7d1

mjneil reviewed Nov 21, 2017

View reviewed changes

forbesjo reviewed Nov 27, 2017

View reviewed changes

forbesjo reviewed Nov 28, 2017

View reviewed changes

gesinger added 2 commits November 28, 2017 17:05

Add player.dash reference to HlsHandler instance

edc6a27

Disable native audio and video tracks on test page when overrideNativ…

7ca2f10

…e is on

forbesjo suggested changes Nov 29, 2017

View reviewed changes

forbesjo mentioned this pull request Dec 12, 2017

feat: extend stats object #10

Merged

4 tasks

forbesjo reviewed Dec 12, 2017

View reviewed changes

Update mpd-parser to 0.2.0 and videojs-contrib-eme to 3.0.0

e7f8422

forbesjo approved these changes Dec 13, 2017

View reviewed changes

mjneil reviewed Dec 13, 2017

View reviewed changes

forbesjo merged commit 4021637 into videojs:master Dec 14, 2017

forbesjo pushed a commit to forbesjo/http-streaming that referenced this pull request Jan 30, 2018

Merge pull request videojs#8 from ishita12/test

830680c

chrome canary support

gesinger added a commit that referenced this pull request Apr 11, 2019

Merge pull request #8 from gesinger/transmux-before-append-lhls-sourc…

24f1e3b

…e-updater-duration Use source updater to update media source duration

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial DASH Support #8

Initial DASH Support #8

gesinger commented Nov 7, 2017

forbesjo commented Nov 7, 2017

mjneil Nov 7, 2017

mjneil Nov 7, 2017

mjneil Nov 7, 2017

gesinger Nov 13, 2017

OshinKaramian Nov 8, 2017

OshinKaramian Nov 8, 2017

mjneil Nov 8, 2017

OshinKaramian Nov 8, 2017

mjneil Nov 8, 2017

mjneil Nov 8, 2017

gesinger Nov 20, 2017 •

edited

Loading

mjneil Nov 21, 2017

gesinger Nov 28, 2017

forbesjo Nov 27, 2017

forbesjo Nov 28, 2017

gesinger Nov 28, 2017

forbesjo Nov 28, 2017

gesinger Nov 28, 2017

forbesjo left a comment

forbesjo Dec 12, 2017

forbesjo Dec 12, 2017

forbesjo commented Dec 13, 2017

mjneil Dec 13, 2017

mjneil Dec 13, 2017 •

edited

Loading

mjneil Dec 13, 2017

mjneil Dec 13, 2017

mjneil Dec 13, 2017

mjneil Dec 13, 2017

forbesjo commented Dec 14, 2017


		const mediaChange = !this.media_ \|\| playlist.uri !== this.media_.uri;

		this.state = 'HAVE_METADATA';

		@@ -546,7 +622,7 @@ const HlsSourceHandler = function(mode) {
		tech.hls = new HlsHandler(source, tech, localOptions);

Initial DASH Support #8

Initial DASH Support #8

Conversation

gesinger commented Nov 7, 2017

Description

Requirements Checklist

forbesjo commented Nov 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gesinger Nov 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forbesjo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forbesjo commented Dec 13, 2017

Choose a reason for hiding this comment

mjneil Dec 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forbesjo commented Dec 14, 2017

gesinger Nov 20, 2017 •

edited

Loading

mjneil Dec 13, 2017 •

edited

Loading