1013933 - pause/resume html5 video resets speed (playbackRate)

Status: RESOLVED FIXED

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

Description

[Dennis Jakobsen]

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

[cajbir (:cajbir)]

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.

Comment 3

[Alastor Wu [:alwu]]

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

Comment 4

[Alastor Wu [:alwu]]

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!

Comment 5

[Paul Adenot (:padenot)]

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

Comment 6

[Alastor Wu [:alwu]]

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

Comment 7

[Alastor Wu [:alwu]]

Attachment: Don't change playbackrate in user interface

Hi, Paul, 
Could you help me review this patch?
Thanks!

Comment 8

[Paul Adenot (:padenot)]

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.

Comment 9

[Alastor Wu [:alwu]]

Attachment: MozReview Request: Bug 1013933 - preserve playbackrate across play/pause

Review commit: https://reviewboard.mozilla.org/r/46051/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/46051/

Comment 10

[Paul Adenot (:padenot)]

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.

Comment 11

[Alastor Wu [:alwu]]

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/

Comment 12

[Paul Adenot (:padenot)]

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.

Comment 13

[Alastor Wu [:alwu]]

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/

Comment 14

[Alastor Wu [:alwu]]

Paul, thanks for review!

And here is try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdf6428be532

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/765b22823c12

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/765b22823c12

Comment 17

[Kazi Nuzhat Tasnem]

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]