Closed Bug 1144617 Opened 8 years ago Closed 8 years ago

Can't seek to end of video


(Core :: Audio/Video, defect)

Not set



Tracking Status
firefox37 --- wontfix
firefox38 --- fixed
firefox39 --- fixed


(Reporter: jya, Assigned: jya)


(Blocks 1 open bug)



(2 files, 1 obsolete file)

This may be a regression

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.
Blocks: MSE
From spec:

"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

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

Chrome has the proper behaviour.
IE never fire ended
(In reply to Jean-Yves Avenard [:jya] from comment #1)
> From spec:

Just FYI you should look at , not the w3c's slighty-buggy-and-out-of-date mirror of it.

But yes, this sounds like a bug.
Blocks: 1143971
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
Attachment #8580484 - Flags: review?(cpearce)
Assignee: nobody → jyavenard
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.
Attachment #8580498 - Flags: review?(matt.woodrow)
Attachment #8580498 - Flags: review?(matt.woodrow) → review+
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.
Attachment #8580484 - Flags: review?(cpearce) → review+
(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.
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.
Attachment #8581363 - Flags: review?(cpearce)
Attachment #8580484 - Attachment is obsolete: true
Attachment #8581363 - Flags: review?(cpearce) → review+
This only made us more spec compliant, but other piece of code relies on that behaviour to be correct to properly behave themselves.
Flags: needinfo?(giles)
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.
Flags: needinfo?(giles)
Attachment #8580498 - Flags: approval-mozilla-aurora?
Attachment #8581363 - Flags: approval-mozilla-aurora?
Attachment #8580498 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8581363 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.