Bug 225396 - [GPUP] Reduce MediaPlayer polling frequency when possible
Summary: [GPUP] Reduce MediaPlayer polling frequency when possible
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-05-05 10:07 PDT by Eric Carlson
Modified: 2023-03-24 02:09 PDT (History)
7 users (show)

See Also:


Attachments
Patch (23.45 KB, patch)
2021-05-05 10:31 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing (23.53 KB, patch)
2021-05-05 15:12 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing (24.24 KB, patch)
2021-05-06 11:18 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2021-05-05 10:07:30 PDT
When `currentMediaTimeDidChange` is supported by the media player, use it push currentTime updates instead of polling at a fixed frequency.
Comment 1 Radar WebKit Bug Importer 2021-05-05 10:07:39 PDT
<rdar://problem/77562643>
Comment 2 Eric Carlson 2021-05-05 10:31:44 PDT
Created attachment 427778 [details]
Patch
Comment 3 Peng Liu 2021-05-05 11:39:54 PDT
Comment on attachment 427778 [details]
Patch

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

> Source/WebCore/platform/graphics/MediaPlayerPrivate.h:109
> +    virtual bool setCurrentTimeDidChangeCallback(MediaPlayer::CurrentTimeDidChangeCallback&&) { return false; }

Nit. The return value indicates whether the player supports time change callback, right?
It works, but looks a little strange. Maybe adding an interface like "supportsTimeChangeCallback()" is better? I am not sure.

> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.messages.in:42
> +    CurrentTimeChanged(MediaTime mediaTime, MonotonicTime wallTime)

Nit. `wallTime` is interesting here. :-)
Maybe `queryTime` is better?
Comment 4 EWS 2021-05-05 13:10:45 PDT
Found 30 new test failures: compositing/geometry/fixed-position-composited-page-scale-smaller-than-viewport.html, compositing/iframes/border-uneven-radius-composited-frame.html, compositing/video/video-reflection.html, fast/canvas/webgl/texImage2D-video-flipY-true.html, http/tests/media/media-play-stream-chunked-icy.html, http/tests/media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-live-broadcast.html, http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html, http/tests/media/modern-media-controls/skip-back-support/skip-back-support-button-click.html, http/tests/media/modern-media-controls/skip-back-support/skip-back-support-live-broadcast.html, http/tests/media/modern-media-controls/status-support/status-support-live-broadcast.html ...
Comment 5 Eric Carlson 2021-05-05 15:12:01 PDT
Created attachment 427812 [details]
Patch for landing
Comment 6 Eric Carlson 2021-05-06 11:18:06 PDT
Created attachment 427908 [details]
Patch for landing
Comment 7 EWS 2021-05-06 13:49:37 PDT
Committed r277116 (237414@main): <https://commits.webkit.org/237414@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427908 [details].