Closed
Bug 1130839
Opened 11 years ago
Closed 11 years ago
Changing media element duration should update playback position when needed
Categories
(Core :: Audio/Video, defect, P2)
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)
3.33 KB,
patch
|
cpearce
:
review+
|
Details | Diff | Splinter Review |
3.42 KB,
patch
|
cajbir
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
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.
Assignee | ||
Updated•11 years ago
|
Summary: Removal of data should update playback position when needed → Changing media element duration should update playback position when needed
Assignee | ||
Comment 2•11 years ago
|
||
"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."
Assignee | ||
Comment 3•11 years ago
|
||
Seek when duration is changed as per spec.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Assignee | ||
Updated•11 years ago
|
Attachment #8561168 -
Flags: review?(cpearce)
Assignee | ||
Comment 4•11 years ago
|
||
mochitest to go with it.. I added tests to verify bug 1130826 while at it
Attachment #8561212 -
Flags: review?(cajbir.bugzilla)
Assignee | ||
Comment 5•11 years ago
|
||
:cpearce
Assignee | ||
Updated•11 years ago
|
Attachment #8561168 -
Attachment is obsolete: true
Attachment #8561168 -
Flags: review?(cpearce)
Assignee | ||
Updated•11 years ago
|
Attachment #8561287 -
Flags: review?(cpearce)
Assignee | ||
Comment 6•11 years ago
|
||
remove garbage from another patch
Attachment #8561334 -
Flags: review?(cajbir.bugzilla)
Assignee | ||
Updated•11 years ago
|
Attachment #8561212 -
Attachment is obsolete: true
Attachment #8561212 -
Flags: review?(cajbir.bugzilla)
Updated•11 years ago
|
Priority: -- → P2
Comment 7•11 years ago
|
||
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 8•11 years ago
|
||
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+
Assignee | ||
Comment 9•11 years ago
|
||
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/07b6be460567
https://hg.mozilla.org/mozilla-central/rev/f310a80db30d
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•