REGRESSION(264795@main): Some HLS videos have a frame half as wide as…

… it should be https://bugs.webkit.org/show_bug.cgi?id=260825 rdar://110467872 Reviewed by Jer Noble. In 264795@main, the source of truth for videoLayerSize was moved from MediaPlayerPrivate to HTMLMediaElement, but a cached value remained on MediaPlayerPrivateRemote (in m_videoLayerSize). When the GPU process would send the LayerHostingContextIdChanged message to the WebContent process with a new presentation size, MediaPlayerPrivateRemote's cached value would be updated but HTMLMediaElement's would not. This could ultimately lead to WebAVPlayerLayer laying out with a size that didn't match the presentation size of the styled video element. Fixed this by removing m_videoLayerSize from MediaPlayerPrivateRemote and delegating to HTMLMediaElement when setting the presentation size specified in the LayerHostingContextIdChanged message. No new tests as reproducing this requires content that isn't licenced for inclusion in WebKit. * Source/WebCore/html/HTMLMediaElement.h: * Source/WebCore/platform/graphics/MediaPlayer.cpp: (WebCore::MediaPlayer::videoLayerSizeDidChange): * Source/WebCore/platform/graphics/MediaPlayer.h: (WebCore::MediaPlayerClient::mediaPlayerVideoLayerSizeDidChange): * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm: (WebCore::MediaPlayerPrivateAVFoundationObjC::createAVPlayerLayer): * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm: (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::ensureLayer): * Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp: (WebKit::MediaPlayerPrivateRemote::platformLayer const): (WebKit::MediaPlayerPrivateRemote::setVideoLayerSizeFenced): Deleted. * Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h: * Source/WebKit/WebProcess/GPU/media/cocoa/MediaPlayerPrivateRemoteCocoa.mm: (WebKit::MediaPlayerPrivateRemote::layerHostingContextIdChanged): (WebKit::MediaPlayerPrivateRemote::videoLayerSize const): (WebKit::MediaPlayerPrivateRemote::setVideoLayerSizeFenced): Canonical link: https://commits.webkit.org/267389@main
WebKit · Aug 29, 2023 · a517dfc · a517dfc
1 parent 232c6fa
commit a517dfc
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 16 deletions.
diff --git a/Source/WebCore/html/HTMLMediaElement.h b/Source/WebCore/html/HTMLMediaElement.h
@@ -647,7 +647,6 @@ class HTMLMediaElement
     WEBCORE_EXPORT LayerHostingContextID layerHostingContextID();
     WEBCORE_EXPORT WebCore::FloatSize naturalSize();
 
-    FloatSize mediaPlayerVideoLayerSize() const override { return videoLayerSize(); }
     WEBCORE_EXPORT WebCore::FloatSize videoLayerSize() const;
     void setVideoLayerSizeFenced(const FloatSize&, WTF::MachSendRight&&);
     void updateMediaState();
@@ -827,6 +826,9 @@ class HTMLMediaElement
 
     bool mediaPlayerShouldDisableHDR() const final { return shouldDisableHDR(); }
 
+    FloatSize mediaPlayerVideoLayerSize() const final { return videoLayerSize(); }
+    void mediaPlayerVideoLayerSizeDidChange(const FloatSize& size) final { m_videoLayerSize = size; }
+
     void pendingActionTimerFired();
     void progressEventTimerFired();
     void playbackProgressTimerFired();

diff --git a/Source/WebCore/platform/graphics/MediaPlayer.cpp b/Source/WebCore/platform/graphics/MediaPlayer.cpp
@@ -923,6 +923,11 @@ FloatSize MediaPlayer::videoLayerSize() const
     return client().mediaPlayerVideoLayerSize();
 }
 
+void MediaPlayer::videoLayerSizeDidChange(const FloatSize& size)
+{
+    client().mediaPlayerVideoLayerSizeDidChange(size);
+}
+
 void MediaPlayer::setVideoLayerSizeFenced(const FloatSize& size, WTF::MachSendRight&& fence)
 {
     m_private->setVideoLayerSizeFenced(size, WTFMove(fence));

diff --git a/Source/WebCore/platform/graphics/MediaPlayer.h b/Source/WebCore/platform/graphics/MediaPlayer.h
@@ -318,6 +318,7 @@ class MediaPlayerClient {
     virtual bool mediaPlayerShouldDisableHDR() const { return false; }
 
     virtual FloatSize mediaPlayerVideoLayerSize() const { return { }; }
+    virtual void mediaPlayerVideoLayerSizeDidChange(const FloatSize&) { }
 
 #if !RELEASE_LOG_DISABLED
     virtual const void* mediaPlayerLogIdentifier() { return nullptr; }
@@ -373,6 +374,7 @@ class WEBCORE_EXPORT MediaPlayer : public MediaPlayerEnums, public ThreadSafeRef
     void requestHostingContextID(LayerHostingContextIDCallback&&);
     LayerHostingContextID hostingContextID() const;
     FloatSize videoLayerSize() const;
+    void videoLayerSizeDidChange(const FloatSize&);
     void setVideoLayerSizeFenced(const FloatSize&, WTF::MachSendRight&&);
 
 #if PLATFORM(IOS_FAMILY)

diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
@@ -647,7 +647,7 @@ static WallTime toSystemClockTime(NSDate *date)
     [m_videoLayer addObserver:m_objcObserver.get() forKeyPath:@"readyForDisplay" options:NSKeyValueObservingOptionNew context:(void *)MediaPlayerAVFoundationObservationContextAVPlayerLayer];
     updateVideoLayerGravity();
     [m_videoLayer setContentsScale:player->playerContentsScale()];
-    m_videoLayerManager->setVideoLayer(m_videoLayer.get(), player->videoLayerSize());
+    m_videoLayerManager->setVideoLayer(m_videoLayer.get(), player->presentationSize());
 
 #if PLATFORM(IOS_FAMILY) && !PLATFORM(WATCHOS) && !PLATFORM(APPLETV)
     if ([m_videoLayer respondsToSelector:@selector(setPIPModeEnabled:)])

diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm
@@ -929,7 +929,7 @@ void getSupportedTypes(HashSet<String>& types) const final
     if (m_mediaSourcePrivate)
         m_mediaSourcePrivate->setVideoLayer(m_sampleBufferDisplayLayer.get());
     if (player) {
-        m_videoLayerManager->setVideoLayer(m_sampleBufferDisplayLayer.get(), player->videoLayerSize());
+        m_videoLayerManager->setVideoLayer(m_sampleBufferDisplayLayer.get(), player->presentationSize());
         player->renderingModeChanged();
     }
 }

diff --git a/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp b/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp
@@ -840,8 +840,9 @@ PlatformLayer* MediaPlayerPrivateRemote::platformLayer() const
 {
 #if PLATFORM(COCOA)
     if (!m_videoLayer && m_layerHostingContextID) {
-        m_videoLayer = createVideoLayerRemote(const_cast<MediaPlayerPrivateRemote*>(this), m_layerHostingContextID, m_videoFullscreenGravity, expandedIntSize(m_videoLayerSize));
-        m_videoLayerManager->setVideoLayer(m_videoLayer.get(), expandedIntSize(m_videoLayerSize));
+        auto expandedVideoLayerSize = expandedIntSize(videoLayerSize());
+        m_videoLayer = createVideoLayerRemote(const_cast<MediaPlayerPrivateRemote*>(this), m_layerHostingContextID, m_videoFullscreenGravity, expandedVideoLayerSize);
+        m_videoLayerManager->setVideoLayer(m_videoLayer.get(), expandedVideoLayerSize);
     }
     return m_videoLayerManager->videoInlineLayer();
 #else
@@ -987,14 +988,6 @@ void MediaPlayerPrivateRemote::setPresentationSize(const IntSize& size)
     connection().send(Messages::RemoteMediaPlayerProxy::SetPresentationSize(size), m_id);
 }
 
-#if PLATFORM(COCOA)
-void MediaPlayerPrivateRemote::setVideoLayerSizeFenced(const FloatSize& size, WTF::MachSendRight&& machSendRight)
-{
-    connection().send(Messages::RemoteMediaPlayerProxy::SetVideoLayerSizeFenced(size, WTFMove(machSendRight)), m_id);
-    m_videoLayerSize = size;
-}
-#endif
-
 void MediaPlayerPrivateRemote::paint(GraphicsContext& context, const FloatRect& rect)
 {
     paintCurrentFrameInContext(context, rect);

diff --git a/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h b/Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h
@@ -115,7 +115,7 @@ class MediaPlayerPrivateRemote final
     void renderingModeChanged();
 #if PLATFORM(COCOA)
     void layerHostingContextIdChanged(std::optional<WebKit::LayerHostingContextID>&&, const WebCore::IntSize&);
-    WebCore::FloatSize videoLayerSize() const final { return m_videoLayerSize; }
+    WebCore::FloatSize videoLayerSize() const final;
     void setVideoLayerSizeFenced(const WebCore::FloatSize&, WTF::MachSendRight&&) final;
 #endif
 
@@ -482,7 +482,6 @@ class MediaPlayerPrivateRemote final
 
     Vector<LayerHostingContextIDCallback> m_layerHostingContextIDRequests;
     LayerHostingContextID m_layerHostingContextID { 0 };
-    WebCore::FloatSize m_videoLayerSize;
     std::optional<WebCore::VideoFrameMetadata> m_videoFrameMetadata;
     bool m_isGatheringVideoFrameMetadata { false };
 #if PLATFORM(COCOA) && !HAVE(AVSAMPLEBUFFERDISPLAYLAYER_COPYDISPLAYEDPIXELBUFFER)

diff --git a/Source/WebKit/WebProcess/GPU/media/cocoa/MediaPlayerPrivateRemoteCocoa.mm b/Source/WebKit/WebProcess/GPU/media/cocoa/MediaPlayerPrivateRemoteCocoa.mm
@@ -92,7 +92,19 @@
         return;
     }
     setLayerHostingContextID(inlineLayerHostingContextId.value());
-    m_videoLayerSize = presentationSize;
+    player->videoLayerSizeDidChange(presentationSize);
+}
+
+WebCore::FloatSize MediaPlayerPrivateRemote::videoLayerSize() const
+{
+    if (RefPtr player = m_player.get())
+        return player->videoLayerSize();
+    return { };
+}
+
+void MediaPlayerPrivateRemote::setVideoLayerSizeFenced(const FloatSize& size, WTF::MachSendRight&& machSendRight)
+{
+    connection().send(Messages::RemoteMediaPlayerProxy::SetVideoLayerSizeFenced(size, WTFMove(machSendRight)), m_id);
 }
 
 } // namespace WebKit