Skip to content
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

Defer setting currentTIme until something is in buffer #101

Closed
nickdesaulniers opened this issue Jun 12, 2015 · 31 comments
Closed

Defer setting currentTIme until something is in buffer #101

nickdesaulniers opened this issue Jun 12, 2015 · 31 comments
Labels
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@nickdesaulniers
Copy link

STR:

  1. in FF Nightly 41 goto: http://shaka-player-demo.appspot.com/
  2. load manifest: http://www.bok.net/dash/tears_of_steel/cleartext/stream.mpd

expected: playback

actual: console error: Player Error InvalidStateError

will try posting more info with a downloaded, debug build

@nickdesaulniers
Copy link
Author

More info logged to the console from a local build of v1.3.0-debug:

EME not available. mediakeys.js:55:5
+ audio/mp4; codecs="mp4a.40.2" is supported player.js:178:3
+ video/mp4; codecs="avc1.42E01E" is supported player.js:178:3
+ audio/mp4; codecs="mp4a.40.2" is supported player.js:178:3
+ video/mp4; codecs="avc1.42E01E" is supported player.js:178:3
Initial seek range Array [ 0, 61.318 ] dash_video_source.js:159:1
Starting each stream from 0 stream_video_source.js:1112:3
 stream_video_source.js:863:1

Player error error { target: null, isTrusted: false, detail: InvalidStateError, eventPhase: 0, bubbles: true, cancelable: false, defaultPrevented: false, timeStamp: 1434139288126345, NONE: 0, CAPTURING_PHASE: 1, AT_TARGET: 2 } app.js:892:3

@nickdesaulniers
Copy link
Author

The call stack looks like:
app.onPlayerError_ app.js:892
shaka.util.FakeEventTarget.prototype.recursiveDispatch_ fake_event_target.js:145
shaka.util.FakeEventTarget.prototype.dispatchEvent fake_event_target.js:123
shaka.player.Player.prototype.load player.js:313

setting a breakpoint at app.onPlayerError_ app.js:892, stepping up the call stack to shaka.player.Player.prototype.load player.js:313:

error.message is "An attempt was made to use an object that is not, or is no longer, usable"
error.filename is "http://localhost:8002/lib/player/stream_video_source.js"
error.lineNumber is 1113

lib/player/stream_video_source.js:1113 is this.video.currentTime = streamStartTime;.

setting a break point there, streamStartTime is 0 and this.video.currentTime is also 0, and triggers the exception in Firefox.

This is equivalent to document.createElement('video').currentTime = 0 in Firefox (also triggers an invalid state error).

nickdesaulniers pushed a commit to nickdesaulniers/shaka-player that referenced this issue Jun 17, 2015
@joeyparrish joeyparrish added the type: bug Something isn't working correctly label Jun 17, 2015
@joeyparrish
Copy link
Member

Interesting. Thanks for investigating, Nick!

I'm confused about one point. Is the problem that we're setting currentTime too early (while the HTMLVideoElement is in the wrong state) or that we're setting currentTime to the exact same value that it already has?

I ask because in your PR, the subject is "defer assigning", but it seems to be checking the value instead of changing the timing.

@nickdesaulniers
Copy link
Author

Is the problem that we're setting currentTime too early (while the HTMLVideoElement is in the wrong state)

This seems to be the case, but I work in blissful ignorance a few levels up from the media stack so I'm not really sure why this is considered an error in Firefox. Maybe "defer" is not the right term, feel free to reword the commit message manually. Also, I'm not sure if this breaks seeking; I would guess not, since that conditional predicate should be true.

It would be better to have the predicate be if state is ready then assign time to zero, but I'm not sure how to check the state of a media element to see. There's probably some constant on the MediaElement prototype that VideoElements inherit from, but didn't see any likely candidates to check in my admittedly brief look through the attributes.

@joeyparrish
Copy link
Member

Yeah, I'm concerned #103 isn't the right fix. If it's a state issue, then some aspect of that state should be involved in the fix.

#103 only really fixes the problem if it's a matter of Firefox not permitting you to set currentTime to the same value it already has, which sounds like wacky behavior to me. I tend to think Firefox's implementation is smarter than that.

There are some streams which do not start at 0. I don't have a sample off-hand, but if you try something like this:

this.video.currentTime = streamStartTime + 1;

And it still throws an error, it's definitely a state problem and #103 isn't the right fix. If that does somehow fix it, we just need to reword the commit message in #103 and merge it.

@joeyparrish
Copy link
Member

Ah, I just ran across something that may be relevant. See the change in stream_video_source.js in blinkbox/shaka-player@79a80f0. It may be that Firefox and IE11 both dislike how/when we set currentTime. You may want to try the same tweak in stream_video_source.js that Blinkbox is using for IE.

@jonoward: Can you offer us any insight on this? Is IE's behavior similar to the Firefox behavior described in this issue?

@jonoward
Copy link
Contributor

Ah yes we had this exact issue in IE - it doesn't like certain operations being called before the video has metadata (calling msSetMediaKeys had the same issue).

Our change was to set currentTime within a handler for loadedmetadata. But to be really robust, you could first check if videoElement.readyState is > 0, if so, set current time immediately, else defer to loadedmetadata. Hope that helps!

@nickdesaulniers
Copy link
Author

if state is ready then assign time to zero, but I'm not sure how to check the state of a media element to see.

you could first check if videoElement.readyState is > 0

heh, funny I missed that.

I'll rewrite my patch.

@nickdesaulniers
Copy link
Author

Instead of > 0, there's const enums:

HTMLMediaElement.HAVE_NOTHING
HTMLMediaElement.HAVE_METADATA
HTMLMediaElement.HAVE_CURRENT_DATA
HTMLMediaElement.HAVE_FUTURE_DATA
HTMLMediaElement.HAVE_ENOUGH_DATA

so I think videoElement.readyState > HTMLMediaElement.HAVE_NOTHING is the way to go.

nickdesaulniers pushed a commit to nickdesaulniers/shaka-player that referenced this issue Jun 18, 2015
Fixes shaka-project#101
Required for Firefox's and IE's MediaElement implementations.
nickdesaulniers pushed a commit to nickdesaulniers/shaka-player that referenced this issue Jun 18, 2015
Fixes shaka-project#101
Required for Firefox's and IE's MediaElement implementations.
nickdesaulniers pushed a commit to nickdesaulniers/shaka-player that referenced this issue Jun 18, 2015
Fixes shaka-project#101
Required for Firefox's and IE's MediaElement implementations.
@joeyparrish joeyparrish added this to the v1.4.0 milestone Jun 19, 2015
joeyparrish added a commit that referenced this issue Jun 19, 2015
defer assigning to video.currentTime Fixes #101
@joeyparrish
Copy link
Member

PR had to be reverted, so reopening this issue.

@joeyparrish joeyparrish reopened this Jun 19, 2015
@joeyparrish
Copy link
Member

@nickdesaulniers, I just tested with the latest nightly of Firefox, but I'm having trouble reproducing on my Linux box. The stream you mentioned in the bug report is MP4, and Firefox 42 doesn't seem to support MP4 on Linux.

I tried the WebM "Angel One" test asset we have built-in, and it plays just fine on Firefox 42. Do you see this bug with all assets in general, or just with the "Tears of Steel" MPD you linked to?

@nickdesaulniers
Copy link
Author

We definitely don't have WebM support in our MSE implementation. Asking in irc.mozilla.org#media about mp4/h.264/Linux support now...

In 41.0a1 (2015-06-26), and 42.0a1 (2015-07-01) I continue to get the InvalidStateError, but not in 40.0a2 (2015-06-29) (where it works).

@joeyparrish
Copy link
Member

I flipped several MSE-related flags, including one about MSE WebM.

Interesting that it works in FF40. Could we perhaps raise a bug with someone at Mozilla about the behavior of FF41 and FF42? If the new behavior is more spec-correct, we could certainly revisit FF41 support in Shaka Player after confirming that this is, in fact, intended behavior for Firefox. But I'd hate to discover that we spent a lot of time and energy sweeping a bug under the rug with a workaround.

Has Mozilla decided when FF with ship with MSE enabled by default?

@joeyparrish joeyparrish modified the milestones: v1.5.0, v1.4.0 Jul 1, 2015
@cpeterso
Copy link

cpeterso commented Jul 2, 2015

@nickdesaulniers and I work at Mozilla. I've forwarded this bug to our MSE developer. He has been refactoring our MSE implementation in 41–42, so regressions are not unexpected. :-)

We plan to enable MSE by default in FF42 (in the Firefox Nightly channel now).

@joeyparrish
Copy link
Member

Awesome! Looking forward to FF42, then.

@joeyparrish joeyparrish removed this from the v1.5.0 milestone Jul 7, 2015
@cpearce
Copy link

cpearce commented Jul 17, 2015

I work on Firefox's EME and media stacks. We only support EME in MP4.

To get Linux MP4 support you need to have the appropriate ffmpeg packages installed and must ensure the following prefs have the following values in about:config:

media.fragmented-mp4.ffmpeg.enabled=true
media.fragmented-mp4.exposed=true
media.mediasource.whitelist=false

David Dorwin pointed out that this demo also fails in Firefox because MediaKeySystemAccess.prototype.getConfiguration does not exist. Currently we haven't implemented that, and I'm not sure when we'll get to it.

We support configuration discovery by providing a configuration list to navigator.requestMediaKeySystemAccess().

We support this WebIDL:
https://dxr.mozilla.org/mozilla-central/source/dom/webidl/MediaKeySystemAccess.webidl

Currently only the initDataType capability, audioType and videoType MediaKeySystemOptions fields are handled by our implementation. The optional fields are ignored, and we don't support any configurations that specify an audioCapability or videoCapability.

@jonoward
Copy link
Contributor

jonoward commented Sep 8, 2015

Just to add some additional info to this thread, it seems only IE11 raises an InvalidStateError when the current time is set when video.readyState is HTMLMediaElement.HAVE_NOTHING; Edge has the same behaviour as Chrome, and allows it (assume it defers it internally, as at this point the duration is not yet known by the video element, so it shouldn't be able to set it's currentTime).

Also, with the recent changes to add player.setPlaybackStartTime for initiating playback at a non zero currentTime, the invalid state error only happens if you use this method (in IE11, and I assume FF)

@joeyparrish
Copy link
Member

This same condition should be triggered without setPlaybackStartTime if you are playing live content. Live playback starting at t=0 would be vanishingly unlikely.

@joeyparrish joeyparrish added this to the Future milestone Oct 12, 2015
@nickdesaulniers
Copy link
Author

https://bugzilla.mozilla.org/show_bug.cgi?id=1217923

In the meantime, I've been successful in live streaming content using GPAC's DashCast, http-server, FF 44, and shaka-player (commenting out the line assigning to this.video.currentTime).

@joeyparrish joeyparrish added type: enhancement New feature or request and removed flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this labels Oct 23, 2015
@joeyparrish joeyparrish changed the title Player Error in FF 41 Defer setting currentTIme until something is in buffer Oct 23, 2015
@joeyparrish
Copy link
Member

We've found a way to fix it, but it will have to wait for v2.0. Experimentally, if we defer setting video.currentTime until after at least one init segment is in buffer, the latest stable FF seems to work just fine. However, since Stream.onUpdate_ uses video.currentTime to start fetching segments, we can't simply defer without making many more changes.

Shaka v2.0 will be structured differently and this should not be an issue.

@jyavenard
Copy link

Throwing an invalid state error when setting currentTime is per spec.
This is what I wrote in the Firefox bug created for this:

http://dev.w3.org/html5/spec-preview/media-elements.html#offsets-into-the-media-resource

Throw an InvalidStateError if there is no selected Media Resource.

When the video/media element is created, and no mediasource has been created and attached it doesn't have a selected media resource yet.

@joeyparrish
Copy link
Member

@jyavenard

"When the video/media element is created, and no mediasource has been created and attached it doesn't have a selected media resource yet."

That doesn't describe what happens in Shaka, though. MediaSource has been created and attached before setting currentTime. What we do (1.x) is set currentTime before appendBuffer().

What I am suggesting we do differently in 2.0 is set currentTime only after the first call to appendBuffer() completes.

@nickdesaulniers
Copy link
Author

I filed a bug with Chromium just in case.
https://code.google.com/p/chromium/issues/detail?id=547207

@jyavenard
Copy link

Ok... I went by the first post description that did "document.createElement('video').currentTime = 0".

Then you're hitting https://bugzilla.mozilla.org/show_bug.cgi?id=1188887

I had put it on a back burner, I'll bump it

@joeyparrish
Copy link
Member

Looks like the FF bug mentioned immediately above has been closed as fixed. I haven't tested this fix myself, but since IE11 suffers the same problem, this Shaka issue will have to stay open until we can defer currentTime.

@cpeterso
Copy link

cpeterso commented Nov 3, 2015

Firefox Nightly 45 now loads the mpd from the first comment, but it does not play automatically (like it does in Chrome).

Player error error { target: null, isTrusted: false, detail: TypeError, eventPhase: 0, bubbles: true, cancelable: false, defaultPrevented: false, timeStamp: 1446585931411407, NONE: 0, CAPTURING_PHASE: 1, AT_TARGET: 2 } app.js:962:3
app.onPlayerError_() app.js:962
shaka.util.FakeEventTarget.prototype.recursiveDispatch_() fake_event_target.js:143
shaka.util.FakeEventTarget.prototype.dispatchEvent() fake_event_target.js:121
shaka.player.Player.prototype.load/<() player.js:352
(Async: promise callback) shaka.player.Player.prototype.load() player.js:321
app.load_() app.js:758
app.loadDashStream()

If I press the play button, the video will start, but hiccups after a couple seconds and logs the following console error:

SBM video/mp4: multiple buffered ranges detected: Either the content has gaps in it, the content's segments are not aligned across bitrates, or the browser has evicted the middle of the buffer.
source_buffer_manager.js:455:1
shaka.media.SourceBufferManager.prototype.detectMultipleBufferedRanges() source_buffer_manager.js:455
shaka.media.Stream.prototype.onUpdate_() stream.js:515

@jyavenard: is this a problem with Firefox's MSE, the Shaka player, or the encoded media?

@cpeterso
Copy link

cpeterso commented Nov 3, 2015

I reported this autoplay and "multiple buffered ranges detected" issues in Firefox bug 1221329.

@joeyparrish joeyparrish modified the milestones: v2.0.0, v2.0-beta Jan 5, 2016
joeyparrish pushed a commit that referenced this issue Jan 16, 2016
Closes #101
Closes #186

Change-Id: I8d1a8d6c0f8cfb5abdd81a149318377282b2bea0
@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants