AudioVideoRenderer TimeProgressEstimator incorrectly set m_effectiveRate

[jyavenard]

5a1e303

AudioVideoRenderer TimeProgressEstimator incorrectly set m_effectiveRate
https://bugs.webkit.org/show_bug.cgi?id=314795
rdar://177046564

Reviewed by Jer Noble and Eric Carlson.

311867@main ("Use a TimeProgressEstimator to interpolate
AudioVideoRendererRemote::currentTime() between GPU updates") introduced
two issues.

1. Every Play / Pause / SetRate IPC from WebContent caused the GPU-side
RemoteAudioVideoRendererProxyManager handler to synchronously build a
RemoteAudioVideoRendererState via stateFor() and push a StateUpdate IPC
back to the WebContent. stateFor() reads videoPlaybackQualityMetrics,
which on the protected-content AVSampleBufferDisplayLayer path (for
example Netflix) fans into four separate [renderer videoPerformanceMetrics]
synchronous XPCs into mediaserverd. The only consumer of that reply in the
WebContent is the client-side TimeProgressEstimator, which only needs the
MediaTimeUpdateData fields (currentTime, effectiveRate, wallTime) to
unfreeze and re-anchor after the local setRate / pause already froze it.
The full state payload and its metrics fetch were wasted work on every
transition.

2. AudioVideoRendererRemote::TimeProgressEstimator::setRate() gated the
assignment m_effectiveRate = rate on the same "if (currentRate)" guard
that controls the rebase of m_cachedTime. The guard is only meaningful
for the rebase (nothing to fold in when the old rate was already zero);
the new-rate assignment shouldn't ride on it. As a result, when setRate()
was called on an estimator whose current rate was 0 (for example
immediately after a stall()), m_effectiveRate stayed at 0 locally until
the next setTime() arrived. timeIsProgressing() and effectiveRate() would
keep reporting 0 in that round-trip window.

Fix 1: change Play / Pause / SetRate to asynchronous replies carrying only
WebCore::MediaTimeUpdateData. The GPU handlers now call renderer->play /
pause / setRate and invoke the completion with a fresh MediaTimeUpdateData
built by a new static helper timeUpdateDataFor(renderer). stateFor() is
refactored to reuse the same helper for the fields it shares. The new
payload contains no videoPlaybackQualityMetrics, so these transitions no
longer touch mediaserverd. The WebContent callers use
sendWithAsyncReplyOnDispatcher on queueSingleton(); the reply handler
applies m_timeEstimator.setTime(timeUpdateData) which is exactly the
unfreeze that the old StateUpdate reply performed via updateCacheState.

Fix 2: hoist m_effectiveRate = rate out of the rebase guard in
TimeProgressEstimator::setRate, and drop the now-redundant
m_effectiveRate = 0 inside the if (!rate) branch. All four (currentRate,
rate) cases now leave m_effectiveRate == rate on exit.

In a follow-up change we will extract the AudioVideoRendererRemote's TimeProgressEstimator
to its own class and combine with the one used in MediaPlayerPrivateRemote
this will allow for API tests and simplify the change. It will also allows
for using a SharedMemory timeBase removing the need for sending IPC messages.

* Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.messages.in:
Declare MediaTimeUpdateData async replies on Play, Pause, SetRate.

* Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.h:
(WebKit::RemoteAudioVideoRendererProxyManager::play):
(WebKit::RemoteAudioVideoRendererProxyManager::pause):
(WebKit::RemoteAudioVideoRendererProxyManager::setRate): Take a
CompletionHandler<void(WebCore::MediaTimeUpdateData&&)>&&.

* Source/WebKit/GPUProcess/media/RemoteAudioVideoRendererProxyManager.cpp:
(WebKit::timeUpdateDataFor): New file-local helper that returns a
MediaTimeUpdateData without fetching videoPlaybackQualityMetrics. Stamps
effectiveRate = timeIsProgressing() ? effectiveRate() : 0 and
wallTime = MonotonicTime::now(), matching what stateFor() used to do
inline.
(WebKit::RemoteAudioVideoRendererProxyManager::play):
(WebKit::RemoteAudioVideoRendererProxyManager::pause):
(WebKit::RemoteAudioVideoRendererProxyManager::setRate): Invoke the
completion with timeUpdateDataFor(*renderer) instead of sending a
StateUpdate(stateFor(...)). Null-renderer path still invokes the
completion with a default MediaTimeUpdateData so the IPC reply never
hangs.
(WebKit::RemoteAudioVideoRendererProxyManager::stateFor): Reuse
timeUpdateDataFor for the timeUpdateData sub-struct.

* Source/WebKit/WebProcess/GPU/media/AudioVideoRendererRemote.cpp:
(WebKit::AudioVideoRendererRemote::TimeProgressEstimator::setRate):
Always apply m_effectiveRate = rate; only the m_cachedTime / m_wallTime
rebase remains conditional on the prior rate being non-zero. Drop the
redundant m_effectiveRate = 0 inside the if (!rate) branch.
(WebKit::AudioVideoRendererRemote::play):
(WebKit::AudioVideoRendererRemote::pause):
(WebKit::AudioVideoRendererRemote::setRate): Use
sendWithAsyncReplyOnDispatcher on queueSingleton(); the reply handler
calls m_timeEstimator.setTime(timeUpdateData) so the estimator still gets
the same anchor it used to receive from the full StateUpdate reply.

Canonical link: https://commits.webkit.org/313249@main

f9ad1aa

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	🛠 win	⏳ 🛠 ios-apple
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	🧪 win-tests	⏳ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		⏳ 🛠 vision-apple
	✅ 🧪 ios-wk2-wpt	🧪 api-mac-debug	✅ 🛠 gtk3-libwebrtc
⏳ 🛠 🧪 jsc	❌ 🧪 api-ios		✅ 🛠 gtk
	✅ 🛠 ios-safer-cpp	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🛠 playstation
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on previous version of this PR (hash 73514ac)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	❌ 🛠 ios	🛠 mac	✅ 🛠 wpe	🛠 win	⏳ 🛠 ios-apple
	❌ 🛠 ios-sim	🛠 mac-AS-debug	🧪 wpe-wk2	🧪 win-tests	⏳ 🛠 mac-apple
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	🧪 api-mac	🧪 api-wpe		⏳ 🛠 vision-apple
	❌ 🧪 ios-wk2-wpt	🧪 api-mac-debug	❌ 🛠 gtk3-libwebrtc
⏳ 🛠 🧪 jsc	❌ 🧪 api-ios	🧪 mac-wk1	✅ 🛠 gtk
	🛠 ios-safer-cpp	🧪 mac-wk2	🧪 gtk-wk2
	❌ 🛠 vision	🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	🛠 vision-sim	🧪 mac-wk2-stress	✅ 🛠 playstation
	🧪 vision-wk2	🧪 mac-intel-wk2
	🛠 tv	🛠 mac-safer-cpp
	❌ 🛠 tv-sim
	❌ 🛠 watch
	❌ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on current version of this PR (hash f9ad1aa)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	🛠 win	⏳ 🛠 ios-apple
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	🧪 win-tests	⏳ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		⏳ 🛠 vision-apple
	✅ 🧪 ios-wk2-wpt	🧪 api-mac-debug	✅ 🛠 gtk3-libwebrtc
⏳ 🛠 🧪 jsc	❌ 🧪 api-ios		✅ 🛠 gtk
	✅ 🛠 ios-safer-cpp	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🛠 playstation
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

[webkit-commit-queue]

Committed 313249@main (5a1e303): https://commits.webkit.org/313249@main

Reviewed commits have been landed. Closing PR #64909 and removing active labels.