Closed Bug 1130839 Opened 5 years ago Closed 5 years ago

Changing media element duration should update playback position when needed

Categories

(Core :: Audio/Video, defect, P2)

x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

Per spec:
http://www.w3.org/TR/html5/embedded-content-0.html#durationChange

"If the duration is changed such that the current playback position ends up being greater than the time of the end of the media resource, then the user agent must also seek to the time of the end of the media resource."

Currently, nothing related to playback position is done.

Also, should the state machine has pending decoded frames, those would get played and MediaDecoderStateMachine::UpdatePlaybackPositionInternal would extend the duration of the media element.
Here is a test sample:
http://people.mozilla.org/~jyavenard/tests/mse_mp4/1130839.html

It loads 10s worth of video, then seek to 7s, then change mediasource duration to 5.

Immediately after changing the mediasource duration to 5, video.seeking should be true and video.currentTime should be 7.
Summary: Removal of data should update playback position when needed → Changing media element duration should update playback position when needed
"Immediately after changing the mediasource duration to 5, video.seeking should be true and video.currentTime should be 7."
should read:
"Immediately after changing the mediasource duration to 5, video.seeking should be true and video.currentTime should be 5."
Seek when duration is changed as per spec.
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Attachment #8561168 - Flags: review?(cpearce)
mochitest to go with it.. I added tests to verify bug 1130826 while at it
Attachment #8561212 - Flags: review?(cajbir.bugzilla)
Depends on: 1130948
Attachment #8561168 - Attachment is obsolete: true
Attachment #8561168 - Flags: review?(cpearce)
Attachment #8561287 - Flags: review?(cpearce)
remove garbage from another patch
Attachment #8561334 - Flags: review?(cajbir.bugzilla)
Attachment #8561212 - Attachment is obsolete: true
Attachment #8561212 - Flags: review?(cajbir.bugzilla)
Priority: -- → P2
Comment on attachment 8561334 [details] [diff] [review]
mochitest ensuring element seeks to end of media

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

::: dom/media/mediasource/test/test_TruncatedDuration.html
@@ +1,4 @@
> +<!DOCTYPE HTML>
> +<html>
> +<head>
> +  <title>MSE: truncated seeks to end and buffered range updated</title>

I don't understand this title. Can you make it clearer? Can you add a comment in the test explaining what the test is testing? This makes it easier to verify the code matches the intent.
Attachment #8561334 - Flags: review?(cajbir.bugzilla) → review+
Comment on attachment 8561287 [details] [diff] [review]
Seek to end of media if duration changed

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

::: dom/media/MediaDecoderStateMachine.cpp
@@ +1250,5 @@
>    NS_ASSERTION(mCurrentFrameTime >= 0, "CurrentTime should be positive!");
>    if (aTime > mEndTime) {
>      NS_ASSERTION(mCurrentFrameTime > GetDuration(),
>                   "CurrentTime must be after duration if aTime > endTime!");
> +    DECODER_LOG("Setting new time to %lld", aTime);

"Setting new end time"...
Attachment #8561287 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/07b6be460567
https://hg.mozilla.org/mozilla-central/rev/f310a80db30d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.