Bug 128731 – There is no rate change event when user agent modifies playback rate.

• Component: Media
• Status: RESOLVED INVALID

[Piotr Grad]

When UA chages playback rate there is no ratechange event. playbackRate attribute is changed without event, this is not valid with current spec.

[Piotr Grad]

Created attachment 224056 [details]
Patch

[Jer Noble]

Comment on attachment 224056 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=224056&action=review

> Source/WebCore/platform/graphics/MediaPlayer.cpp:1097
> +    if (!std::isnan(m_private->rate()))

This...

> Source/WebCore/platform/graphics/MediaPlayerPrivate.h:90
> +    virtual float rate() const { return std::numeric_limits<float>::infinity(); }

Does not match this. So for every platform which does not yet implement this method, rateChanged() notifications will result in the "rate" being set to infinity.

Additionally, this means that in the event of a buffer underrun, the video element will fire a ratechanged event in addition to a stalled event.  I do not believe this is correct.

Perhaps you could add more information about what problem you are attempting to solve, and we could come up with a better solution?

[Piotr Grad]

Created attachment 224188 [details]
Patch

[Piotr Grad]

(In reply to comment #2)
> (From update of attachment 224056 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=224056&action=review
> 
> > Source/WebCore/platform/graphics/MediaPlayer.cpp:1097
> > +    if (!std::isnan(m_private->rate()))
> 
> This...
> 
> > Source/WebCore/platform/graphics/MediaPlayerPrivate.h:90
> > +    virtual float rate() const { return std::numeric_limits<float>::infinity(); }
> 
> Does not match this. So for every platform which does not yet implement this method, rateChanged() notifications will result in the "rate" being set to infinity.
> 
> Additionally, this means that in the event of a buffer underrun, the video element will fire a ratechanged event in addition to a stalled event.  I do not believe this is correct.
> 
> Perhaps you could add more information about what problem you are attempting to solve, and we could come up with a better solution?

You have right about NaN, sorry for mistake.
The problem is when media player cannot handle playback rate which was set by HTMLMediaElement. In this case UA should update playbackRate in HTMLMediaElement and fire ratechange event.

[Build Bot]

Comment on attachment 224188 [details]
Patch

Attachment 224188 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5312193457291264

New failing tests:
media/media-fragments/TC0011.html
media/video-currentTime.html
media/track/track-cues-pause-on-exit.html
media/track/track-cues-enter-exit.html
media/media-captions.html
media/video-currentTime-set.html
media/audio-mpeg4-supported.html
media/media-fragments/TC0006.html
media/media-fragments/TC0004.html
media/media-ended.html
media/track/track-cues-sorted-before-dispatch.html
media/audio-mpeg-supported.html
media/track/track-cues-cuechange.html
media/track/track-disabled-addcue.html
media/track/track-cues-missed.html
media/media-fullscreen-inline.html
media/event-attributes.html
webgl/1.0.1/conformance/textures/texture-npot-video.html
webgl/1.0.2/conformance/extensions/oes-texture-float-with-video.html
media/media-load-event.html
media/media-fragments/TC0024.html
media/track/track-disabled.html
media/media-fragments/TC0015.html
media/media-fragments/TC0017.html
webgl/1.0.1/conformance/textures/tex-image-and-sub-image-2d-with-video.html
media/audio-concurrent-supported.html
media/media-fragments/TC0005.html
media/media-continues-playing-after-replace-source.html
media/video-currentTime-delay.html
media/media-fragments/TC0035.html

[Build Bot]

Created attachment 224197 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 224188 [details]
Patch

Attachment 224188 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4736021447049216

New failing tests:
media/media-fragments/TC0011.html
media/video-currentTime.html
media/track/track-cues-pause-on-exit.html
media/track/track-cues-enter-exit.html
media/media-captions.html
media/video-currentTime-set.html
media/audio-mpeg4-supported.html
media/media-fragments/TC0006.html
media/media-fragments/TC0004.html
media/media-ended.html
media/track/track-cues-sorted-before-dispatch.html
media/audio-mpeg-supported.html
media/track/track-cues-cuechange.html
media/track/track-disabled-addcue.html
media/track/track-cues-missed.html
media/media-fullscreen-inline.html
media/event-attributes.html
webgl/1.0.1/conformance/textures/texture-npot-video.html
webgl/1.0.2/conformance/extensions/oes-texture-float-with-video.html
media/media-load-event.html
media/media-fragments/TC0024.html
media/track/track-disabled.html
media/media-fragments/TC0015.html
media/media-fragments/TC0017.html
webgl/1.0.1/conformance/textures/tex-image-and-sub-image-2d-with-video.html
media/audio-concurrent-supported.html
media/media-fragments/TC0005.html
media/media-continues-playing-after-replace-source.html
media/video-currentTime-delay.html
media/media-fragments/TC0035.html

[Build Bot]

Created attachment 224203 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 224188 [details]
Patch

Attachment 224188 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4953993319022592

New failing tests:
media/media-fragments/TC0011.html
media/video-currentTime.html
media/track/track-cues-pause-on-exit.html
media/track/track-cues-enter-exit.html
media/media-captions.html
media/video-currentTime-set.html
media/audio-mpeg4-supported.html
media/media-fragments/TC0006.html
media/media-fragments/TC0004.html
media/media-ended.html
media/media-load-event.html
media/audio-mpeg-supported.html
media/track/track-cues-cuechange.html
media/track/track-disabled-addcue.html
media/track/track-cues-missed.html
fast/repaint/obscured-background-no-repaint.html
media/media-fullscreen-inline.html
webgl/1.0.1/conformance/textures/texture-npot-video.html
webgl/1.0.2/conformance/extensions/oes-texture-float-with-video.html
media/track/track-cues-sorted-before-dispatch.html
media/media-fragments/TC0024.html
media/track/track-disabled.html
media/media-fragments/TC0015.html
media/media-fragments/TC0017.html
webgl/1.0.1/conformance/textures/tex-image-and-sub-image-2d-with-video.html
media/audio-concurrent-supported.html
media/media-fragments/TC0005.html
media/video-pause-empty-events.html
media/media-continues-playing-after-replace-source.html
media/video-currentTime-delay.html

[Build Bot]

Created attachment 224212 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Eric Carlson]

(In reply to comment #4)
> The problem is when media player cannot handle playback rate which was set by HTMLMediaElement. In this case UA should update playbackRate in HTMLMediaElement and fire ratechange event.

According to the spec, this is not an issue. You are supposed to do whatever necessary to support an arbitrary rate, eg. pause and buffer data and then play it quickly, pause again, etc. The "playbackRate" attribute should always return whatever value was set [1]:

    The attribute is mutable: on getting it must return the last value it was set to, or 
    1.0 if it hasn't yet been set

This issue was discussed at *great* length on the mailing list a couple of years ago [2], a bug was filed [3], and in the end the working group decided on the current behavior [4][5], eg. when the user agent can't play at the requested rate:

  * playbackRate and defaultPlaybackRate reflect whatever value was set.
  * currentTime changes at a rate as close to playbackRate as possible.


[1] http://www.w3.org/html/wg/drafts/html/master/embedded-content-0.html#dom-media-playbackrate
[2] http://lists.w3.org/Archives/Public/public-html/2011Feb/0113.html
[3] https://www.w3.org/Bugs/Public/show_bug.cgi?id=10837
[4] http://lists.w3.org/Archives/Public/public-html/2011Mar/0682.html
[5] http://lists.w3.org/Archives/Public/public-html/2011Apr/0213.html

[Philippe Normand]

Comment on attachment 224188 [details]
Patch

Clearing review flag so the patch no longer appears in the review queue.