1144617 - Can't seek to end of video

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jean-Yves Avenard [:jya]]

This may be a regression

http://people.mozilla.org/~jyavenard/tests/mse_mp4/seektoend.html?eos=1&eosat=-1&duration=-1&init=0

Attempting to seek to the end of the video element using:
video.currentTime = video.duration

will never complete.
the seeking event is fired.

However seeked event is never fired, (nor will "ended")

Expected behaviour:
"seeked" and "ended" event to be fired.

Comment 1

[Jean-Yves Avenard [:jya]]

From spec:
http://www.w3.org/html/wg/drafts/html/master/semantics.html#dom-media-seek

"Set the current playback position to the new playback position.

Note. If the media element was potentially playing immediately before it started seeking, but seeking caused its readyState attribute to change to a value lower than HAVE_FUTURE_DATA, then a waiting event will be fired at the element.

Note. This step sets the current playback position, and thus can immediately trigger other conditions, such as the rules regarding when playback "reaches the end of the media resource" (part of the logic that handles looping), even before the user agent is actually able to render the media data for that position (as determined in the next step)."

The second note describes what to do should we reach the end of the media resource
http://www.w3.org/html/wg/drafts/html/master/semantics.html#reaches-the-end

So we should assume that the seek has completed and we've reached the end.

Chrome has the proper behaviour.
IE never fire ended

Comment 2

[Bobby Holley (:bholley)]

(In reply to Jean-Yves Avenard [:jya] from comment #1)
> From spec:
> http://www.w3.org/html/wg/drafts/html/master/semantics.html#dom-media-seek

Just FYI you should look at https://html.spec.whatwg.org/multipage/ , not the w3c's slighty-buggy-and-out-of-date mirror of it.

But yes, this sounds like a bug.

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Part1. Add MediaDecoder::IsLiveStream() API

Add MediaDecoder::IsLiveStream() method. MediaSourceResource returns -1 which always make the MDSM considers that it's a live stream and never issue the ended event after a seek

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Part2. Allow seek to end of mediasource.duration

When seeking to the end of the mediasource duration, we would have stalled as the mPendingSeekTime would never be in the buffered range. So instead, we seek to the last of the audio sample and the last video sample. The MDSM seek logic will hit an end of stream and 
automatically complete the seek as required.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8580484 [details] [diff] [review]
Part1. Add MediaDecoder::IsLiveStream() API

Review of attachment 8580484 [details] [diff] [review]:
-----------------------------------------------------------------

It's a shame we can't stick a IsLiveStream() function on the MediaResource instead.

Comment 6

[Jean-Yves Avenard [:jya]]

(In reply to Chris Pearce (:cpearce) from comment #5)
> Comment on attachment 8580484 [details] [diff] [review]
> Part1. Add MediaDecoder::IsLiveStream() API
> 
> Review of attachment 8580484 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It's a shame we can't stick a IsLiveStream() function on the MediaResource
> instead.

We could. A tad more work however, but nothing significant I don't think.

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: Part1. Add MediaResource::IsLiveStream() API

v2: Add MediaResource::IsLiveStream(). I added a monitor to set it. In practice, no race can occur as we won't complete the seek until the stream is ended and if a race does occur it's nothing serious. Also, setting and reading a boolean is atomic with most platforms. But just in case this method is used elsewhere. But just for the clarity of things. as MediaSourceResource::mEnded is accessed from two different threads.

Comment 8

[Jean-Yves Avenard [:jya]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/5a6798098985 
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6604e8d39fc6

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5a6798098985
https://hg.mozilla.org/mozilla-central/rev/6604e8d39fc6

Comment 10

[Jean-Yves Avenard [:jya]]

This only made us more spec compliant, but other piece of code relies on that behaviour to be correct to properly behave themselves.

Comment 11

[Ralph Giles (:rillian)]

Comment on attachment 8580498 [details] [diff] [review]
Part2. Allow seek to end of mediasource.duration

Requesting 38 uplift for both patches on this bug.

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Possible stalls after seeking, spec compliance.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: Change is isolated to MSE code.
[String/UUID change made/needed]: None.

Comment 12

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/efd5a0a9cf83
https://hg.mozilla.org/releases/mozilla-aurora/rev/00cbbfb86628