Attachment 438717 Details for Bug 230495 – Patch

[patch] Patch

bug-230495-20210920131341.patch (text/plain), 9.37 KB, created by Eric Carlson on 2021-09-20 13:13:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Eric Carlson

Created: 2021-09-20 13:13:42 PDT

Size: 9.37 KB

patch

obsolete

>Subversion Revision: 282405
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 03a79df43d0477219a519adcaa006516819d65e6..6fb635bb0a79b60726aa5181df35e8dd45fab6eb 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,45 @@
>+2021-09-20  Eric Carlson  <eric.carlson@apple.com>
>+
>+        [Cocoa] Videos sometimes don't render when loaded in the GPU process
>+        https://bugs.webkit.org/show_bug.cgi?id=230495
>+        rdar://83205407
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The media players choose what type of video output to create based on whether
>+        or not the renderer can support accelerated rendering. We were only pushing
>+        this state to the GPU process when the renderer changed to require backing, but
>+        that state was lost if that transition happened before the AVFoundation-backed
>+        media player was created. Change this to push the current state from the web process
>+        when a remote media player is created, and again whenever its readyState changes.
>+
>+        We haven't figured out how to reliably reproduce this, so https://webkit.org/b/230500
>+        tracks figuring that out and creating a test.
>+
>+        * GPUProcess/media/RemoteMediaPlayerProxy.cpp:
>+        (WebKit::RemoteMediaPlayerProxy::RemoteMediaPlayerProxy): Set m_renderingCanBeAccelerated
>+        from the configuration.
>+        (WebKit::RemoteMediaPlayerProxy::prepareToPlay): Add logging.
>+        (WebKit::RemoteMediaPlayerProxy::prepareForRendering): Ditto.
>+        (WebKit::RemoteMediaPlayerProxy::setPageIsVisible): Ditto.
>+        (WebKit::RemoteMediaPlayerProxy::acceleratedRenderingStateChanged): Ditto.
>+        (WebKit::RemoteMediaPlayerProxy::mediaPlayerReadyStateChanged): Ditto.
>+
>+        * GPUProcess/media/RemoteMediaPlayerProxyConfiguration.h: Add renderingCanBeAccelerated.
>+        (WebKit::RemoteMediaPlayerProxyConfiguration::encode const):
>+        (WebKit::RemoteMediaPlayerProxyConfiguration::decode): Decode it.
>+
>+        * WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:
>+        (WebKit::MediaPlayerPrivateRemote::readyStateChanged): Call acceleratedRenderingStateChanged
>+        if the state has changed.
>+        (WebKit::MediaPlayerPrivateRemote::acceleratedRenderingStateChanged): Remember
>+        the state.
>+        * WebProcess/GPU/media/MediaPlayerPrivateRemote.h:
>+
>+        * WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:
>+        (WebKit::RemoteMediaPlayerManager::createRemoteMediaPlayer): Pass the accelerated 
>+        rendering state with the configuration.
>+
> 2021-09-14  Chris Dumez  <cdumez@apple.com>
> 
>         [Hardening] Validate IDBValue's blob paths in WebIDBServer::putOrAdd()
>diff --git a/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp b/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp
>index bafd9265d516395cb5c05009f2f3d03c657e8fa7..1adba9e8b9dd04839c2251864d7b9a29efa07d2d 100644
>--- a/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp
>+++ b/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp
>@@ -84,6 +84,7 @@ RemoteMediaPlayerProxy::RemoteMediaPlayerProxy(RemoteMediaPlayerManagerProxy& ma
> #endif
> {
>     m_typesRequiringHardwareSupport = m_configuration.mediaContentTypesRequiringHardwareSupport;
>+    m_renderingCanBeAccelerated = m_configuration.renderingCanBeAccelerated;
>     m_player = MediaPlayer::create(*this, m_engineIdentifier);
> }
> 
>@@ -172,6 +173,7 @@ void RemoteMediaPlayerProxy::cancelLoad()
> 
> void RemoteMediaPlayerProxy::prepareToPlay()
> {
>+    ALWAYS_LOG(LOGIDENTIFIER);
>     m_player->prepareToPlay();
> }
> 
>@@ -233,11 +235,13 @@ void RemoteMediaPlayerProxy::setPitchCorrectionAlgorithm(WebCore::MediaPlayer::P
> 
> void RemoteMediaPlayerProxy::prepareForRendering()
> {
>+    ALWAYS_LOG(LOGIDENTIFIER);
>     m_player->prepareForRendering();
> }
> 
> void RemoteMediaPlayerProxy::setPageIsVisible(bool visible)
> {
>+    ALWAYS_LOG(LOGIDENTIFIER);
>     m_player->setPageIsVisible(visible);
> }
> 
>@@ -255,6 +259,7 @@ void RemoteMediaPlayerProxy::setVideoFullscreenGravity(WebCore::MediaPlayerEnums
> 
> void RemoteMediaPlayerProxy::acceleratedRenderingStateChanged(bool renderingCanBeAccelerated)
> {
>+    ALWAYS_LOG(LOGIDENTIFIER, renderingCanBeAccelerated);
>     m_renderingCanBeAccelerated = renderingCanBeAccelerated;
>     m_player->acceleratedRenderingStateChanged();
> }
>@@ -348,6 +353,7 @@ void RemoteMediaPlayerProxy::mediaPlayerNetworkStateChanged()
> 
> void RemoteMediaPlayerProxy::mediaPlayerReadyStateChanged()
> {
>+    ALWAYS_LOG(LOGIDENTIFIER);
>     updateCachedVideoMetrics();
>     updateCachedState(true);
>     m_cachedState.readyState = m_player->readyState();
>diff --git a/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxyConfiguration.h b/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxyConfiguration.h
>index ae1d99fd6613038ef98fa7b255b49a85d17e3f8b..4acb40e2c09ec75ce9eebb731e1dd1c9c632150f 100644
>--- a/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxyConfiguration.h
>+++ b/Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxyConfiguration.h
>@@ -47,7 +47,8 @@ struct RemoteMediaPlayerProxyConfiguration {
>     WebCore::SecurityOriginData documentSecurityOrigin;
>     uint64_t logIdentifier { 0 };
>     bool shouldUsePersistentCache { false };
>-    bool isVideo { 0 };
>+    bool isVideo { false };
>+    bool renderingCanBeAccelerated { false };
> 
>     template<class Encoder>
>     void encode(Encoder& encoder) const
>@@ -65,6 +66,7 @@ struct RemoteMediaPlayerProxyConfiguration {
>         encoder << logIdentifier;
>         encoder << shouldUsePersistentCache;
>         encoder << isVideo;
>+        encoder << renderingCanBeAccelerated;
>     }
> 
>     template <class Decoder>
>@@ -127,6 +129,11 @@ struct RemoteMediaPlayerProxyConfiguration {
>         if (!isVideo)
>             return std::nullopt;
> 
>+        std::optional<bool> renderingCanBeAccelerated;
>+        decoder >> renderingCanBeAccelerated;
>+        if (!renderingCanBeAccelerated)
>+            return std::nullopt;
>+
>         return {{
>             WTFMove(*referrer),
>             WTFMove(*userAgent),
>@@ -141,6 +148,7 @@ struct RemoteMediaPlayerProxyConfiguration {
>             *logIdentifier,
>             *shouldUsePersistentCache,
>             *isVideo,
>+            *renderingCanBeAccelerated,
>         }};
>     }
> };
>diff --git a/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp b/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp
>index dfe340730e060ff9f9cd468eb363fc891602deae..e06f4f7e3ac81f0d5cfa728296e33adce623c399 100644
>--- a/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp
>+++ b/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp
>@@ -351,8 +351,12 @@ void MediaPlayerPrivateRemote::setReadyState(MediaPlayer::ReadyState readyState)
> void MediaPlayerPrivateRemote::readyStateChanged(RemoteMediaPlayerState&& state)
> {
>     updateCachedState(WTFMove(state));
>-    if (auto player = makeRefPtr(m_player.get()))
>+    if (auto player = makeRefPtr(m_player.get())) {
>         player->readyStateChanged();
>+        bool renderingCanBeAccelerated = player->renderingCanBeAccelerated();
>+        if (m_renderingCanBeAccelerated != renderingCanBeAccelerated)
>+            acceleratedRenderingStateChanged();
>+    }
> }
> 
> void MediaPlayerPrivateRemote::volumeChanged(double volume)
>@@ -469,8 +473,10 @@ bool MediaPlayerPrivateRemote::supportsAcceleratedRendering() const
> 
> void MediaPlayerPrivateRemote::acceleratedRenderingStateChanged()
> {
>-    if (auto player = makeRefPtr(m_player.get()))
>-        connection().send(Messages::RemoteMediaPlayerProxy::AcceleratedRenderingStateChanged(player->renderingCanBeAccelerated()), m_id);
>+    if (auto player = makeRefPtr(m_player.get())) {
>+        m_renderingCanBeAccelerated = player->renderingCanBeAccelerated();
>+        connection().send(Messages::RemoteMediaPlayerProxy::AcceleratedRenderingStateChanged(m_renderingCanBeAccelerated), m_id);
>+    }
> }
> 
> #if ENABLE(WIRELESS_PLAYBACK_TARGET)
>diff --git a/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h b/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h
>index e704e69debf0eddef36602622d1273ab10019668..6200749d4d417629d0ac2bfb8ab38b19d308607e 100644
>--- a/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h
>+++ b/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h
>@@ -444,6 +444,7 @@ private:
>     bool m_invalid { false };
>     bool m_waitingForKey { false };
>     bool m_timeIsProgressing { false };
>+    bool m_renderingCanBeAccelerated { false };
> };
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp b/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp
>index db5f610b849ebf705ce32e0a197a8e6fe01c6544..8e9ec3a509ea2409a42411b78bd9b3ffdb73e187 100644
>--- a/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp
>+++ b/Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp
>@@ -150,6 +150,7 @@ std::unique_ptr<MediaPlayerPrivateInterface> RemoteMediaPlayerManager::createRem
>     proxyConfiguration.networkInterfaceName = player->mediaPlayerNetworkInterfaceName();
> #endif
>     proxyConfiguration.mediaContentTypesRequiringHardwareSupport = player->mediaContentTypesRequiringHardwareSupport();
>+    proxyConfiguration.renderingCanBeAccelerated = player->renderingCanBeAccelerated();
>     proxyConfiguration.preferredAudioCharacteristics = player->preferredAudioCharacteristics();
> #if !RELEASE_LOG_DISABLED
>     proxyConfiguration.logIdentifier = reinterpret_cast<uint64_t>(player->mediaPlayerLogIdentifier());

Actions: View | Formatted Diff | Diff

Attachments on bug 230495: 438717 | 438718 | 438721