Bug 128731 - There is no rate change event when user agent modifies playback rate.
Summary: There is no rate change event when user agent modifies playback rate.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-13 03:11 PST by Piotr Grad
Modified: 2014-03-16 03:51 PDT (History)
15 users (show)

See Also:


Attachments
Patch (4.33 KB, patch)
2014-02-13 03:23 PST, Piotr Grad
jer.noble: review-
Details | Formatted Diff | Diff
Patch (4.33 KB, patch)
2014-02-14 02:49 PST, Piotr Grad
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (581.37 KB, application/zip)
2014-02-14 04:10 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (581.56 KB, application/zip)
2014-02-14 05:08 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (540.71 KB, application/zip)
2014-02-14 07:46 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Grad 2014-02-13 03:11:42 PST
When UA chages playback rate there is no ratechange event. playbackRate attribute is changed without event, this is not valid with current spec.
Comment 1 Piotr Grad 2014-02-13 03:23:42 PST
Created attachment 224056 [details]
Patch
Comment 2 Jer Noble 2014-02-13 08:30:05 PST
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?
Comment 3 Piotr Grad 2014-02-14 02:49:54 PST
Created attachment 224188 [details]
Patch
Comment 4 Piotr Grad 2014-02-14 02:52:51 PST
(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.
Comment 5 Build Bot 2014-02-14 04:09:56 PST
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
Comment 6 Build Bot 2014-02-14 04:10:02 PST
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
Comment 7 Build Bot 2014-02-14 05:08:48 PST
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
Comment 8 Build Bot 2014-02-14 05:08:51 PST
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
Comment 9 Build Bot 2014-02-14 07:46:52 PST
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
Comment 10 Build Bot 2014-02-14 07:46:54 PST
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
Comment 11 Eric Carlson 2014-02-14 08:22:46 PST
(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
Comment 12 Philippe Normand 2014-03-16 03:51:29 PDT
Comment on attachment 224188 [details]
Patch

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