Closed
Bug 1013933
Opened 10 years ago
Closed 8 years ago
pause/resume html5 video resets speed (playbackRate)
Categories
(Core :: Audio/Video: Playback, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: brunis, Assigned: alwu)
References
Details
Attachments
(2 files)
1.12 KB,
patch
|
Details | Diff | Splinter Review | |
58 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release) Build ID: 20140506152807 Steps to reproduce: 1. I opened an html5 video 2. I set playback speed to 1.5x 3. I paused the video 4. I resumed playing the video Actual results: playbackRate was reset to 1.0x speed. Expected results: playbackRate should remain to whatever i set it to.
Comment 1•9 years ago
|
||
I confirm the behavior. I notice that the 'Play' option from the right click context menu does keep the playback rate. It's only the controls button that does not. This occurs due to videocontrols.xml explicitly setting playbackRate when pause is toggled: togglePause : function () { ... this.video.playbackRate = this.video.defaultPlaybackRate; ... } It is probably done this way due to the spec saying (https://html.spec.whatwg.org/multipage/embedded-content.html#expose-a-user-interface-to-the-user): "The "play" function in the user agent's interface must set the playbackRate attribute to the value of the defaultPlaybackRate attribute before invoking the play() method." I suspect this should really be done only if this is the first time the play button is pressed rather than transitioning from pause after playback is done. This seems to be what Chrome does.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•9 years ago
|
Component: Audio/Video → Audio/Video: Playback
Updated•9 years ago
|
Priority: -- → P2
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → alwu
Assignee | ||
Comment 3•8 years ago
|
||
From spec [1], > The "play" function in the user agent's interface must set the playbackRate > attribute to the value of the defaultPlaybackRate attribute before invoking the > play() method. it seems that we need to reset the playback rate when we press play button in user interface. To verify this point, I file an issue [2] on github and waiting for the answer. [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#user-interface [2] https://github.com/whatwg/html/issues/956
Assignee | ||
Comment 4•8 years ago
|
||
Hi, Paul, This question might be little annoying, but I still don't understand the spec and what you said on IRC, "we need to set the defaultplaybackRate in the controls I think". What I confused is if we set |playbackRate=defaultplaybackRate| that means we don't preserve the previous playbackRate, because the playbackRate doesn't equal to defaultplaybackRate at that time. I think this kind of situation is not we want, right? Thanks!
Flags: needinfo?(padenot)
Comment 5•8 years ago
|
||
Yes, our front-end code is wrong. The controls need to set the `defaultPlaybackRate` only, and never touch `playbackRate`. About what I said on irc, I was referring to [0], that says: > Note: The defaultPlaybackRate is used by the user agent when it exposes a user interface to the user. That way, the effective `playbackRate` will be preserved across play/pause. [0]: https://html.spec.whatwg.org/multipage/embedded-content.html#dom-media-defaultplaybackrate
Flags: needinfo?(padenot)
Assignee | ||
Comment 6•8 years ago
|
||
After the spec changed [1], I would remove the code which modifies playbackrate in video-control. [1] https://github.com/whatwg/html/pull/992/commits/0b321fb77a9ec6210e751488879acf6fb00d735f
Assignee | ||
Comment 7•8 years ago
|
||
Hi, Paul, Could you help me review this patch? Thanks!
Attachment #8738492 -
Flags: review?(padenot)
Comment 8•8 years ago
|
||
Comment on attachment 8738492 [details] [diff] [review] Don't change playbackrate in user interface Review of attachment 8738492 [details] [diff] [review]: ----------------------------------------------------------------- This is not enough to properly fix this. We should remove all occurrences of `previousPlaybackRate`. Additionally, there is some duplicated logic between the platform and `videocontrols.xml` here. For example, on in `togglePause`, the playbackRate is restored to the `defaultPlaybackRate`, but this is what the platform does. Same for `isDragging`, with the `previousPlaybackRate` bits.
Attachment #8738492 -
Flags: review?(padenot)
Assignee | ||
Comment 9•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46051/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46051/
Attachment #8740859 -
Flags: review?(padenot)
Comment 10•8 years ago
|
||
Comment on attachment 8740859 [details] MozReview Request: Bug 1013933 - preserve playbackrate across play/pause https://reviewboard.mozilla.org/r/46051/#review42555 This needs a test I think.
Attachment #8740859 -
Flags: review?(padenot)
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8740859 [details] MozReview Request: Bug 1013933 - preserve playbackrate across play/pause Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46051/diff/1-2/
Attachment #8740859 -
Flags: review?(padenot)
Comment 12•8 years ago
|
||
Comment on attachment 8740859 [details] MozReview Request: Bug 1013933 - preserve playbackrate across play/pause https://reviewboard.mozilla.org/r/46051/#review42955 r+ with comments addressed. ::: dom/media/test/mochitest.ini:37 (Diff revision 2) > 448636.ogv > 448636.ogv^headers^ > VID_0001.ogg > VID_0001.ogg^headers^ > allowed.sjs > + audio.ogg Can you use a file that is already present ? ::: dom/media/test/test_preserve_playbackrate_after_ui_play.html:32 (Diff revision 2) > +var expectedPlaybackRate = 0.5 > + > +function runTest() { > + var video = document.getElementById("video"); > + video.src = "audio.ogg"; > + video.playbackRate = expectedPlaybackRate; Maybe you should make it loop, because if automation is very slow, it can reach the end, sometimes. I've seen it happen and the cause of intermittents.
Attachment #8740859 -
Flags: review?(padenot) → review+
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8740859 [details] MozReview Request: Bug 1013933 - preserve playbackrate across play/pause Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46051/diff/2-3/
Assignee | ||
Comment 14•8 years ago
|
||
Paul, thanks for review! And here is try-server result. https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdf6428be532
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/765b22823c12
Keywords: checkin-needed
Assignee | ||
Updated•8 years ago
|
OS: Windows 7 → All
Hardware: x86_64 → Unspecified
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/765b22823c12
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•8 years ago
|
QA Whiteboard: [good first verify]
Comment 17•8 years ago
|
||
I have reproduced this bug with Firefox nightly 32.0a1(build id: 20140506152807)on windows 7(64 bit) I have verified this bug as fixed with Firefox beta 48.0b3(build id:20160623122823) User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 [bugday-20160713]
You need to log in
before you can comment on or make changes to this bug.
Description
•