New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dash sources not loading on 5.10.2+ #3428

Closed
RavWar opened this Issue Jul 11, 2016 · 22 comments

Comments

Projects
None yet
9 participants
@RavWar

RavWar commented Jul 11, 2016

I've stumbled upon an issue in videojs 5.10.2+ and dash. The dash sources are not loading most of the times.

It actually depends on how fast the manifest is loaded i think. Small and quick manifests load up normally much frequently than the big or slow ones. Once network throttling is set to slow (3G for example) then the small start to fail as well.

Here is the test case: http://jsbin.com/yesexuxeqe/edit?html,console,output
Note: if videojs version 5.10.2 is replaced with 5.9.2, it works okay every time.

After some investigation i've managed to pinpoint the error to this pr: #3285
Here is a normal startup flow (i've added additional log entries for loadstart events and firstLoadStartListener, successiveLoadStartListener functions):

normal start

Here is a broken one:

bad start

Mentioned pr introduced a feature to dispose source handlers on repetitive loadstart events. And in case of slow dash manifest - loadstart is actually fired twice which causes source handler to be disposed before it can even initiate the playback.

I have not had time to dig up the cause of repetitive loadstart event yet. Maybe someone can shed some light on why's that happening?

@RavWar

This comment has been minimized.

Show comment
Hide comment
@RavWar

RavWar Jul 11, 2016

Also, i'm pretty sure this issue videojs/videojs-contrib-dash#90 has the same cause

RavWar commented Jul 11, 2016

Also, i'm pretty sure this issue videojs/videojs-contrib-dash#90 has the same cause

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Jul 11, 2016

Member

Interesting. Thanks for digging in and finding the root cause. It does sound like the repetitive loadstarts are the issue here. From what I understand, the loadstart event should only ever be fired once. Might be worth also opening up an issue against dashjs itself with this.

Member

gkatsev commented Jul 11, 2016

Interesting. Thanks for digging in and finding the root cause. It does sound like the repetitive loadstarts are the issue here. From what I understand, the loadstart event should only ever be fired once. Might be worth also opening up an issue against dashjs itself with this.

@richardbushell

This comment has been minimized.

Show comment
Hide comment
@richardbushell

richardbushell Jul 11, 2016

Ditto, we've been using videojs with videojs-contrib-dash for quite a while and also had to revert to vjs 5.9 after trying 5.10 at the weekend to avoid the Media Source error when trying to load the dash source.

Ditto, we've been using videojs with videojs-contrib-dash for quite a while and also had to revert to vjs 5.9 after trying 5.10 at the weekend to avoid the Media Source error when trying to load the dash source.

@richardbushell

This comment has been minimized.

Show comment
Hide comment
@richardbushell

richardbushell Jul 12, 2016

OK, here is the reply from dash.js:
Dash-Industry-Forum/dash.js#1491 (comment)
So, any thoughts on the optimal way to programmatically set dash as preferred source and say mp4 as fallback option via VideoJS (as we have been specifying the src children list within the video tag itself).
Video.JS seems to want 'source' tag children listed within the Video tag, does it have a programmatical way of accepting a list of possible sources in an object/array as source options?
Dash.JS seems to want NO sources in the video tag as it wants to attach the source programmatically.
Hence two loadstart events currently when physical source children are pre-stated in the video tag.
Ss, what is the best way to implement fallback?
Currently, VJS 5.10.2+ causes media error with this double loadstart.
Any ideas, we would welcome any initial thoughts before going back to review the coding options... Thanks.

richardbushell commented Jul 12, 2016

OK, here is the reply from dash.js:
Dash-Industry-Forum/dash.js#1491 (comment)
So, any thoughts on the optimal way to programmatically set dash as preferred source and say mp4 as fallback option via VideoJS (as we have been specifying the src children list within the video tag itself).
Video.JS seems to want 'source' tag children listed within the Video tag, does it have a programmatical way of accepting a list of possible sources in an object/array as source options?
Dash.JS seems to want NO sources in the video tag as it wants to attach the source programmatically.
Hence two loadstart events currently when physical source children are pre-stated in the video tag.
Ss, what is the best way to implement fallback?
Currently, VJS 5.10.2+ causes media error with this double loadstart.
Any ideas, we would welcome any initial thoughts before going back to review the coding options... Thanks.

@bbcrddave

This comment has been minimized.

Show comment
Hide comment
@bbcrddave

bbcrddave Jul 13, 2016

Just to be clear, dash.js doesn't really care about source tags. (not strictly true)

dash.js uses MSE which requires a MediaSource object be attached to the media element using media.src. Attaching this will always cause a loadstart, as specified by HTML5 (true but not really relevant)

bbcrddave commented Jul 13, 2016

Just to be clear, dash.js doesn't really care about source tags. (not strictly true)

dash.js uses MSE which requires a MediaSource object be attached to the media element using media.src. Attaching this will always cause a loadstart, as specified by HTML5 (true but not really relevant)

@richardbushell

This comment has been minimized.

Show comment
Hide comment
@richardbushell

richardbushell Jul 13, 2016

So does dash.js always create a SRC attribute within the video tag, rather than a source child of the video tag?

So does dash.js always create a SRC attribute within the video tag, rather than a source child of the video tag?

@bbcrddave

This comment has been minimized.

Show comment
Hide comment
@bbcrddave

bbcrddave Jul 13, 2016

I am eating my words and have partially retracted my previous comment, having been looking a bit deeper into how dash.js initialises when presented with a <source> tag containing type type='application/dash+xml', and how interacts with video.js. Some of the console logs in Dash-Industry-Forum/dash.js#1491 (comment) make it clear what is happening.

See my comments there for what is going on.

tl;dr there is a bug, but it's not the root cause of this issue.

bbcrddave commented Jul 13, 2016

I am eating my words and have partially retracted my previous comment, having been looking a bit deeper into how dash.js initialises when presented with a <source> tag containing type type='application/dash+xml', and how interacts with video.js. Some of the console logs in Dash-Industry-Forum/dash.js#1491 (comment) make it clear what is happening.

See my comments there for what is going on.

tl;dr there is a bug, but it's not the root cause of this issue.

@bbcrddave

This comment has been minimized.

Show comment
Hide comment
@bbcrddave

bbcrddave Jul 13, 2016

https://jsfiddle.net/jzk1sod9/ shows that, bug aside, loadstart is fired independently of dash.js and video.js if there is a source tag on the media element.

https://jsfiddle.net/jzk1sod9/ shows that, bug aside, loadstart is fired independently of dash.js and video.js if there is a source tag on the media element.

@bbcrddave

This comment has been minimized.

Show comment
Hide comment
@bbcrddave

bbcrddave Jul 13, 2016

So does dash.js always create a SRC attribute within the video tag, rather than a source child of the video tag?

Currently, dash.js uses the src attribute of the media element to attach the MediaSource object.

It is possible to attach that object to the src attribute of a source tag, but I don't think it would make any difference in this scenario.

Reference: https://w3c.github.io/media-source/#mediasource-attach

bbcrddave commented Jul 13, 2016

So does dash.js always create a SRC attribute within the video tag, rather than a source child of the video tag?

Currently, dash.js uses the src attribute of the media element to attach the MediaSource object.

It is possible to attach that object to the src attribute of a source tag, but I don't think it would make any difference in this scenario.

Reference: https://w3c.github.io/media-source/#mediasource-attach

@richardbushell

This comment has been minimized.

Show comment
Hide comment
@richardbushell

richardbushell Jul 13, 2016

Thanks Dave, yes, agree it wouldn't make any difference anyway.

Am tracking your skipAutoCreate post in dashjs issue:
Dash-Industry-Forum/dash.js#1496

skipAutoCreate before dash.js seems a bit hacky, but will await feedback.

I suppose the best solution for players in any event would possibly be to make the canplay decision from an array/object of possible sources (with no source tags pre-written, so using an empty video element), and then either use the dash source selection for dashjs attachment (through the player's sourcehandler), or fallback and create a non-dash source tag then. That should guarantee one loadstart.

Anyway, will wait on further thoughts.

richardbushell commented Jul 13, 2016

Thanks Dave, yes, agree it wouldn't make any difference anyway.

Am tracking your skipAutoCreate post in dashjs issue:
Dash-Industry-Forum/dash.js#1496

skipAutoCreate before dash.js seems a bit hacky, but will await feedback.

I suppose the best solution for players in any event would possibly be to make the canplay decision from an array/object of possible sources (with no source tags pre-written, so using an empty video element), and then either use the dash source selection for dashjs attachment (through the player's sourcehandler), or fallback and create a non-dash source tag then. That should guarantee one loadstart.

Anyway, will wait on further thoughts.

@richardbushell

This comment has been minimized.

Show comment
Hide comment
@richardbushell

richardbushell Jul 19, 2016

OK, have now eliminated dash.js itself as the root cause. Bottom line is that since VJS 5.10.x the sourcehandling thru contrib-dash now fails. Subsequently most browsers now fall back to default mp4 file, rather than dash. Works fine if I revert to VJS 5.9.0. Not sure if the commits mentioned above are indeed the issue but it's now broken until the culprit is identified and remedied.

OK, have now eliminated dash.js itself as the root cause. Bottom line is that since VJS 5.10.x the sourcehandling thru contrib-dash now fails. Subsequently most browsers now fall back to default mp4 file, rather than dash. Works fine if I revert to VJS 5.9.0. Not sure if the commits mentioned above are indeed the issue but it's now broken until the culprit is identified and remedied.

@richardbushell

This comment has been minimized.

Show comment
Hide comment
@richardbushell

richardbushell Jul 19, 2016

Just tested to establish break point.
VJS 5.9.2 is OK. VJS 5.10.1 breaks.
Culprit is definitely: #3285
Result is that some browsers now no longer select and play the dash source and now revert to fallback mp4 playback. Chrome & Edge still play dash, BUT Firefox and IE now revert to MP4 fallback.

Just tested to establish break point.
VJS 5.9.2 is OK. VJS 5.10.1 breaks.
Culprit is definitely: #3285
Result is that some browsers now no longer select and play the dash source and now revert to fallback mp4 playback. Chrome & Edge still play dash, BUT Firefox and IE now revert to MP4 fallback.

@richardbushell

This comment has been minimized.

Show comment
Hide comment
@richardbushell

richardbushell Jul 21, 2016

Using programmatic sources solved the problem. Currently I believe that anyone using html source tags will now break dash handling if using VJS 5.10.1+ due to: videojs/video.js#3285
I think the browser natively fires loadstart automatically if any source tag is included. Later a new source attribute is added after selection for dash, which again fires another loadstart. Broken by videojs/video.js#3285

Using programmatic sources solved the problem. Currently I believe that anyone using html source tags will now break dash handling if using VJS 5.10.1+ due to: videojs/video.js#3285
I think the browser natively fires loadstart automatically if any source tag is included. Later a new source attribute is added after selection for dash, which again fires another loadstart. Broken by videojs/video.js#3285

@BrandonOCasey

This comment has been minimized.

Show comment
Hide comment
@BrandonOCasey

BrandonOCasey Jul 21, 2016

Contributor

we saw what may be a similar issue in 5.10.1/5.10.2 and we tracked it down to that PR as well. As of 5.10.3 a fix #3343 was merged which may solve the issue

Contributor

BrandonOCasey commented Jul 21, 2016

we saw what may be a similar issue in 5.10.1/5.10.2 and we tracked it down to that PR as well. As of 5.10.3 a fix #3343 was merged which may solve the issue

@richardbushell

This comment has been minimized.

Show comment
Hide comment
@richardbushell

richardbushell Jul 21, 2016

The original error was in introduced in 5.10.1 but we still have the same problem as at 5.10.7. Using an array of source objects solves it (which we'll probably stick with), but it would be good to find a definitive fix for anyone still using source tags.

The original error was in introduced in 5.10.1 but we still have the same problem as at 5.10.7. Using an array of source objects solves it (which we'll probably stick with), but it would be good to find a definitive fix for anyone still using source tags.

@gkatsev gkatsev added the confirmed label Jul 25, 2016

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Jul 25, 2016

Member

I wonder if what we should do is check what the video's currentSrc is on the second loadstart and then if it's a blob do nothing but if it's anything else proceed with the current implementation?

Member

gkatsev commented Jul 25, 2016

I wonder if what we should do is check what the video's currentSrc is on the second loadstart and then if it's a blob do nothing but if it's anything else proceed with the current implementation?

@jemoreno

This comment has been minimized.

Show comment
Hide comment
@jemoreno

jemoreno Aug 23, 2016

Just wondering if anything has been done on this or if a temporary solution has been given?

Just wondering if anything has been done on this or if a temporary solution has been given?

@forbesjo

This comment has been minimized.

Show comment
Hide comment
@forbesjo

forbesjo Aug 23, 2016

Contributor

@jemoreno the temporary solution would be to load your dash source with player.src() as seen here https://github.com/videojs/videojs-contrib-dash#getting-started

Contributor

forbesjo commented Aug 23, 2016

@jemoreno the temporary solution would be to load your dash source with player.src() as seen here https://github.com/videojs/videojs-contrib-dash#getting-started

@jemoreno

This comment has been minimized.

Show comment
Hide comment
@jemoreno

jemoreno Aug 23, 2016

The video will load with this fix, however if you try and seek to a later time it locks up and will not play anymore.

The video will load with this fix, however if you try and seek to a later time it locks up and will not play anymore.

@ruslandinov

This comment has been minimized.

Show comment
Hide comment
@ruslandinov

ruslandinov Nov 1, 2016

Hey guys, we also encounter this problem and would appreciate a fix. Is there any chance for the PR to be merged soon?

Hey guys, we also encounter this problem and would appreciate a fix. Is there any chance for the PR to be merged soon?

@tf

This comment has been minimized.

Show comment
Hide comment
@tf

tf Aug 16, 2017

Is #3906 (which has been released as part of 6.0) a fix for this problem?

tf commented Aug 16, 2017

Is #3906 (which has been released as part of 6.0) a fix for this problem?

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Jan 3, 2018

Member

This is fixed as part of 6.0, since we removed the double loadstart code.

Member

gkatsev commented Jan 3, 2018

This is fixed as part of 6.0, since we removed the double loadstart code.

@gkatsev gkatsev closed this Jan 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment