Allow for other media sources and destinations (i.e encapsulate MSE concerns into buffer controller only) #60

tchakabam · 2015-11-10T13:42:10Z

OK so this is my first PR on the project, it might not be a great fix or a big improvement, but its a start. Some things we need to get straight first from my perspective ..

There are several concerns:

It should not only be for video element but any HTML5 Media DOM, so also audio. That would work in principle, but I needed to change the name of functions and attributes still. Will add audio-only stream to tests asap.
All the MSE concerns should be encapsulated imho. There should be a sane seperation between what is HLS client (downloading, and buffering data) and what is further down the line: demuxing and playing the data "somehow" (MSE is only one option).
We would want to use something else maybe to process the data further, so optionally one could inject another BufferController than the default one. That could be for example something that works like a Node stream and represents some processing pipeline (which could be terminated or not by MSE or something else).

It would be great if hls.js could extend to these architectural ideas and not narrowing down to one API (MSE) but have "one input many outputs".

Then I hope in the future I can also contribute "real features" ;)

…cerns into MSE-specialized buffer-controller 3. Renames functions and attribute names from video to media (as it can also be audio DOM elements used)

tchakabam · 2015-11-10T13:48:58Z

@mangui Would be great if you could check this out whenever you find time :)

mangui · 2015-11-11T10:41:00Z

@tchakabam
regarding 1, I understand that we can use MSE for HTML audio and video, thus for sake of clarity we can rename all video related stuff to media
s/attachVideo/attachMedia .... in most cases s/video/media ...

regarding 2, that would bring a bunch of refactoring. could you elaborate with another use case ?
also in that case you should remove MSE_ATTACHED/MSE_DETACHED, and you should introduce MEDIA_ATTACHING/MEDIA_ATTACHED and MEDIA_DETACHING/MEDIA_DETACHED event
*ING events being fired by src/hls.js, *ED events being fired by *buffer_controller.js

tchakabam · 2015-11-11T12:18:59Z

@mangui On certain platforms, MSE might not be the (only / available) way to decode a stream. We might use WebAudio or other means as well. That's why I'd like to be able to get the HLS payload with a client independent of the media sink :)

That'll also enable to integrate it with open JS codecs implementations (where we will play stuff onto a canvas / web audio sink eventually but MSE is not involved).

Also you might just want to download HLS data without actually playing it, just for means of prefetching or offline synchronization.

It makes sense to decouple the concerns here.

Furthermore architecturally, I think it would make more sense to chain the elements in a stream-like way.
That said, the HLS client (it's TS or elementary stream output) should be a readable stream that would feed a writable stream that is the MP4 remuxer (but only if needed) and that could be then fed into any sink (MSE or software decoders + WebGL/WebAudio) depending on the nature of the stream and the capabilities of the device you d have to build a different pipeline.

HLS is a protocol that provides data to a player, lets not make the implementation dependent what technology the player is using to play that data :)

tchakabam · 2015-11-11T14:43:38Z

And not to be forgotten: custom encryption modes, custom integration with DRM services and/or EME (Encrypted Media Extensions), where you will want to take the media payload and write your own coupling around these APIs (EME & MSE) or something custom. It could even be fed into Flash - essentially the great thing here is to have a transport implementation of HLS in JavaScript and to be able to leverage it onto any existing or future playback technology.

…er simplified)

mangui · 2015-11-12T10:41:04Z

understood @tchakabam
it is true that MSE concern can easily be moved to buffer-controller.js, hence moving this file to a mse subfolder would make sense
also to keep events generic, we could

s/MSE_ATTACHED/MEDIA_ATTACHED
s/MSE_DETACHED/MEDIA_DETACHED
s/MSE_DETACHING/MEDIA_DETACHING
introduce a new event MEDIA_ATTACHING, that would be dispatched by src/hls.js on hls.attachMedia(). this new event would be listened by mse/buffer-controller. upon MEDIA_ATTACHING, mse/buffer-controller would instantiate the MSE related stuff (thus we can remove the MediaSource stuff from src/hls.js to mse/buffer-controller)

you can remove SOURCE_ATTACHED/SOURCE_DETACHED

the doc (API ...) should be updated as well to reflect those changes

…changed spec. Renamed video occurences to media.

tchakabam · 2015-11-16T23:59:36Z

OK cool 👍

I know its a bit of a refactoring but on the long run I think it makes a lot of sense. There was already the "StreamBufferController" on the branch I was working on but I removed it for now because it's still in development and it would make this PR a mess. But I will push this soon when we have merged this, and you'll see what the idea was :) Anyway its already possible to substitute the "standard" MSEBufferController by an own implementation by passing it in the config now. But I refrained from putting the latter class in an own directory for now since its the only actual implementation of this component at this day.

Ideally i think it also needs only one implementation, but it could be decoupled from MSE and simply push data to downstream components like the demuxer and decoder, eventually the sink will be MSE. That should also make it simpler to use different demuxers, like integrating with mux.js or other stuff for example if it would be necessary.

Looks good from your side with the latest changes now?

mangui · 2015-11-17T08:15:32Z

package.json

@@ -35,6 +35,7 @@
  },
  "dependencies": {
    "babelify": "^6.1.2",
+    "stream-browserify": "^2.0.1",


not needed ?

mangui · 2015-11-17T08:17:56Z

yes @tchakabam , looks good !
could you rebase your PR and remove the dependency on stream-browserify (any reason to include it?). so that i could merge

Conflicts: src/controller/mse-buffer-controller.js src/hls.js

tchakabam · 2015-11-17T19:07:29Z

@mangui OK done.

reason why i had stream-browserify in there is that this is what I use to implement another BufferController that will expose a Stream.Readable from which we can pipe the media data to wherever.

mangui · 2015-11-17T21:52:41Z

there are still some reference to attachMediaElement/detachMediaElement
it should be attachMedia/detachMedia

tchakabam · 2015-11-18T15:48:45Z

k should be good now ;)

mangui · 2015-11-18T15:51:01Z

src/hls.js

@@ -96,49 +97,21 @@ class Hls {
    this.bufferController.destroy();
    //this.fpsController.destroy();
    this.url = null;
-    this.detachVideo();
+    this.detachMediaElement();


are you sure ? 😄

mangui · 2015-11-18T17:01:56Z

perfect, will test and merge tomorrow.
just one thing, finally IMHO bufferController would be better named as MediaController ...
as it is reacting to MEDIA__TACH_ events
in that case mse/MediaController
you can rename the file if you want, or i will do it later on after merging this PR

tchakabam · 2015-11-18T17:11:22Z

@mangui I think that makes perfect sense as a name! but i'll let you do the renaming now ;)

tchakabam · 2015-11-18T17:25:32Z

🍻

mangui · 2015-11-19T18:05:15Z

will check tomorrow finally

related to https://github.com/dailymotion/hls.js/pull/60

Fix exception thrown from log when start pos is not immediately known

tchakabam added 2 commits November 10, 2015 14:36

1. Allows for injecting custom buffer-controller 2. Moves all MSE con…

f7aa8ad

…cerns into MSE-specialized buffer-controller 3. Renames functions and attribute names from video to media (as it can also be audio DOM elements used)

Replace occurences in test code of attachVideo by attachMediaElement

31e037c

Rename onVideo.. to onMedia.. functions

4452baa

Add StreamBufferController (proof-of-concept, could probably be furth…

2f3c563

…er simplified)

tchakabam added 4 commits November 17, 2015 00:41

Generalize events to MEDIA_{ATTACHED/ATTACHING/DETACH/DETACHING} and …

6151d5e

…changed spec. Renamed video occurences to media.

Rename attachMediaElement to attachMedia in tests

84c6207

replace attachVideo -> attachMedia in API.md and demo page

8ccaf6b

remove stream-buffer-controller for now

4d40e79

mangui reviewed Nov 17, 2015
View reviewed changes

package.json

@@ -35,6 +35,7 @@

},

"dependencies": {

"babelify": "^6.1.2",

"stream-browserify": "^2.0.1",

Copy link

Member

mangui Nov 17, 2015

Choose a reason for hiding this comment

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

not needed ?

tchakabam added 2 commits November 17, 2015 19:19

don't depend on stream-browserify

b75d3f5

Merge branch 'master' of https://github.com/dailymotion/hls.js

4677cb1

Conflicts: src/controller/mse-buffer-controller.js src/hls.js

replace occurence of attach/detachMedia in recovery func

056fff3

mangui reviewed Nov 18, 2015
View reviewed changes

rename detachMediaElement -> detachMedia in destroy()

a3ef212

mangui added the Enhancement label Nov 19, 2015

mangui merged commit a3ef212 into video-dev:master Nov 20, 2015

mangui referenced this pull request Nov 20, 2015

update dist

e5de47c

related to https://github.com/dailymotion/hls.js/pull/60

johnBartos pushed a commit that referenced this pull request Jun 27, 2018

Merge pull request #60 from jwplayer/bugfix/unknown-pos-log-exception

77cd168

Fix exception thrown from log when start pos is not immediately known

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow for other media sources and destinations (i.e encapsulate MSE concerns into buffer controller only) #60

Allow for other media sources and destinations (i.e encapsulate MSE concerns into buffer controller only) #60

tchakabam commented Nov 10, 2015

tchakabam commented Nov 10, 2015

mangui commented Nov 11, 2015

tchakabam commented Nov 11, 2015

tchakabam commented Nov 11, 2015

mangui commented Nov 12, 2015

tchakabam commented Nov 16, 2015

mangui Nov 17, 2015

mangui commented Nov 17, 2015

tchakabam commented Nov 17, 2015

mangui commented Nov 17, 2015

tchakabam commented Nov 18, 2015

mangui Nov 18, 2015

mangui commented Nov 18, 2015

tchakabam commented Nov 18, 2015

tchakabam commented Nov 18, 2015

mangui commented Nov 19, 2015

Allow for other media sources and destinations (i.e encapsulate MSE concerns into buffer controller only) #60

Allow for other media sources and destinations (i.e encapsulate MSE concerns into buffer controller only) #60

Conversation

tchakabam commented Nov 10, 2015

tchakabam commented Nov 10, 2015

mangui commented Nov 11, 2015

tchakabam commented Nov 11, 2015

tchakabam commented Nov 11, 2015

mangui commented Nov 12, 2015

tchakabam commented Nov 16, 2015

mangui Nov 17, 2015

Choose a reason for hiding this comment

mangui commented Nov 17, 2015

tchakabam commented Nov 17, 2015

mangui commented Nov 17, 2015

tchakabam commented Nov 18, 2015

mangui Nov 18, 2015

Choose a reason for hiding this comment

mangui commented Nov 18, 2015

tchakabam commented Nov 18, 2015

tchakabam commented Nov 18, 2015

mangui commented Nov 19, 2015