Video spills over PiP screen a little when using Picture in Picture
authorpeng.liu6@apple.com <peng.liu6@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Jun 2020 23:32:18 +0000 (23:32 +0000)
committerpeng.liu6@apple.com <peng.liu6@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Jun 2020 23:32:18 +0000 (23:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=213658

Reviewed by Eric Carlson.

Source/WebCore:

We need to provide video content dimensions instead of video element sizes to
AVPlayerController to make sure that the Picture-in-Picture window will have
the correct aspect ratio.

Test: media/picture-in-picture/picture-in-picture-window-aspect-ratio.html

* platform/ios/VideoFullscreenInterfaceAVKit.h:
* platform/ios/VideoFullscreenInterfaceAVKit.mm:
(VideoFullscreenInterfaceAVKit::setupFullscreen):
* platform/ios/WebVideoFullscreenControllerAVKit.mm:
(VideoFullscreenControllerContext::setUpFullscreen):

Source/WebKit:

Add the video content dimensions to the IPC message VideoFullscreenManagerProxy::SetupFullscreenWithID.

* UIProcess/Cocoa/VideoFullscreenManagerProxy.h:
* UIProcess/Cocoa/VideoFullscreenManagerProxy.messages.in:
* UIProcess/Cocoa/VideoFullscreenManagerProxy.mm:
(WebKit::VideoFullscreenManagerProxy::setupFullscreenWithID):
(WebKit::VideoFullscreenManagerProxy::setVideoDimensions):
(WebKit::VideoFullscreenManagerProxy::enterFullscreen):
* WebProcess/cocoa/VideoFullscreenManager.mm:
(WebKit::VideoFullscreenManager::enterVideoFullscreenForVideoElement):

LayoutTests:

* media/picture-in-picture/picture-in-picture-window-aspect-ratio-expected.txt: Added.
* media/picture-in-picture/picture-in-picture-window-aspect-ratio.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@263707 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/media/picture-in-picture/picture-in-picture-window-aspect-ratio-expected.txt [new file with mode: 0644]
LayoutTests/media/picture-in-picture/picture-in-picture-window-aspect-ratio.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.h
Source/WebCore/platform/ios/VideoFullscreenInterfaceAVKit.mm
Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/Cocoa/VideoFullscreenManagerProxy.h
Source/WebKit/UIProcess/Cocoa/VideoFullscreenManagerProxy.messages.in
Source/WebKit/UIProcess/Cocoa/VideoFullscreenManagerProxy.mm
Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm

index 82ce0a5..767deea 100644 (file)
@@ -1,3 +1,13 @@
+2020-06-29  Peng Liu  <peng.liu6@apple.com>
+
+        Video spills over PiP screen a little when using Picture in Picture
+        https://bugs.webkit.org/show_bug.cgi?id=213658
+
+        Reviewed by Eric Carlson.
+
+        * media/picture-in-picture/picture-in-picture-window-aspect-ratio-expected.txt: Added.
+        * media/picture-in-picture/picture-in-picture-window-aspect-ratio.html: Added.
+
 2020-06-29  Karl Rackler  <rackler@apple.com>
 
         Remove expectation for svg/W3C-SVG-1.1/fonts-elem-01-t.svg and svg/W3C-SVG-1.1/fonts-elem-02-t.svg and svg/W3C-SVG-1.1/fonts-elem-03-b.svg and svg/W3C-SVG-1.1/fonts-elem-07-b.svg as they are passing. 
diff --git a/LayoutTests/media/picture-in-picture/picture-in-picture-window-aspect-ratio-expected.txt b/LayoutTests/media/picture-in-picture/picture-in-picture-window-aspect-ratio-expected.txt
new file mode 100644 (file)
index 0000000..7703f64
--- /dev/null
@@ -0,0 +1,9 @@
+Tests that a pip window has the same aspect ratio as the video content.
+
+RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
+RUN(internals.setMockVideoPresentationModeEnabled(true))
+RUN(video.src = findMediaFile("video", "../content/test"))
+EVENT(canplaythrough)
+EXPECTED (Math.abs(pipWindow.height / pipWindow.width - 240 / 320) < '0.01') OK
+END OF TEST
+
diff --git a/LayoutTests/media/picture-in-picture/picture-in-picture-window-aspect-ratio.html b/LayoutTests/media/picture-in-picture/picture-in-picture-window-aspect-ratio.html
new file mode 100644 (file)
index 0000000..aa5b427
--- /dev/null
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="../video-test.js"></script>
+    <script src="../media-file.js"></script>
+    <script>
+        window.addEventListener('load', async event => {
+            if (!window.internals) {
+                failTest('This test requires window.internals.');
+                return;
+            }
+
+            findMediaElement();
+
+            run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
+            run('internals.setMockVideoPresentationModeEnabled(true)');
+
+            run('video.src = findMediaFile("video", "../content/test")');
+            await waitFor(video, 'canplaythrough');
+
+            runWithKeyDown(() => {
+                video.requestPictureInPicture()
+                .then(pipWindow => {
+                    window.pipWindow = pipWindow;
+                    // Compare floating point numbers with the precision 0.01
+                    testExpected('Math.abs(pipWindow.height / pipWindow.width - 240 / 320)', 0.01, '<');
+
+                    document.exitPictureInPicture().then(endTest).catch(() => {
+                        failTest('Failed to exit the Picture-in-Picture mode.');
+                    });
+                })
+                .catch(error => {
+                    failTest("Failed to enter the Picture-in-Picture mode.");
+                });
+            });
+        });
+    </script>
+</head>
+<body>
+    <div>Tests that a pip window has the same aspect ratio as the video content.</div>
+    <video controls width="2" height="2"></video>
+</body>
+</html>
index 28b165f..2c40c94 100644 (file)
@@ -1,3 +1,22 @@
+2020-06-29  Peng Liu  <peng.liu6@apple.com>
+
+        Video spills over PiP screen a little when using Picture in Picture
+        https://bugs.webkit.org/show_bug.cgi?id=213658
+
+        Reviewed by Eric Carlson.
+
+        We need to provide video content dimensions instead of video element sizes to
+        AVPlayerController to make sure that the Picture-in-Picture window will have
+        the correct aspect ratio.
+
+        Test: media/picture-in-picture/picture-in-picture-window-aspect-ratio.html
+
+        * platform/ios/VideoFullscreenInterfaceAVKit.h:
+        * platform/ios/VideoFullscreenInterfaceAVKit.mm:
+        (VideoFullscreenInterfaceAVKit::setupFullscreen):
+        * platform/ios/WebVideoFullscreenControllerAVKit.mm:
+        (VideoFullscreenControllerContext::setUpFullscreen):
+
 2020-06-29  Tetsuharu Ohzeki  <tetsuharu.ohzeki@gmail.com>
 
         Remove ENABLE_STREAMS_API compilation flag
index 4390cd6..357d9ae 100644 (file)
@@ -79,7 +79,7 @@ public:
     // PlaybackSessionModelClient
     WEBCORE_EXPORT void externalPlaybackChanged(bool enabled, PlaybackSessionModel::ExternalPlaybackTargetType, const String& localizedDeviceName) final;
 
-    WEBCORE_EXPORT void setupFullscreen(UIView&, const IntRect& initialRect, UIView *, HTMLMediaElementEnums::VideoFullscreenMode, bool allowsPictureInPicturePlayback, bool standby);
+    WEBCORE_EXPORT void setupFullscreen(UIView& videoView, const IntRect& initialRect, const FloatSize& videoDimensions, UIView* parentView, HTMLMediaElementEnums::VideoFullscreenMode, bool allowsPictureInPicturePlayback, bool standby);
     WEBCORE_EXPORT void enterFullscreen();
     WEBCORE_EXPORT void exitFullscreen(const IntRect& finalRect);
     WEBCORE_EXPORT void cleanupFullscreen();
index df136c0..4909e94 100644 (file)
@@ -889,14 +889,14 @@ void VideoFullscreenInterfaceAVKit::applicationDidBecomeActive()
     LOG(Fullscreen, "VideoFullscreenInterfaceAVKit::applicationDidBecomeActive(%p)", this);
 }
 
-void VideoFullscreenInterfaceAVKit::setupFullscreen(UIView& videoView, const IntRect& initialRect, UIView* parentView, HTMLMediaElementEnums::VideoFullscreenMode mode, bool allowsPictureInPicturePlayback, bool standby)
+void VideoFullscreenInterfaceAVKit::setupFullscreen(UIView& videoView, const IntRect& initialRect, const FloatSize& videoDimensions, UIView* parentView, HTMLMediaElementEnums::VideoFullscreenMode mode, bool allowsPictureInPicturePlayback, bool standby)
 {
     ASSERT(standby || mode != HTMLMediaElementEnums::VideoFullscreenModeNone);
     LOG(Fullscreen, "VideoFullscreenInterfaceAVKit::setupFullscreen(%p)", this);
 
     [playerController() setHasEnabledVideo:true];
     [playerController() setHasVideo:true];
-    [playerController() setContentDimensions:initialRect.size()];
+    [playerController() setContentDimensions:videoDimensions];
 
     m_allowsPictureInPicturePlayback = allowsPictureInPicturePlayback;
     m_videoView = &videoView;
index 7a9120f..c8f9bce 100644 (file)
@@ -981,7 +981,9 @@ void VideoFullscreenControllerContext::setUpFullscreen(HTMLVideoElement& videoEl
     FloatRect videoLayerFrame = FloatRect(FloatPoint(), videoElementClientRect.size());
     m_fullscreenModel->setVideoLayerFrame(videoLayerFrame);
 
-    dispatch_async(dispatch_get_main_queue(), [protectedThis = makeRefPtr(this), this, videoElementClientRect, viewRef, mode, allowsPictureInPicture] {
+    FloatSize videoDimensions = { (float)videoElement.videoWidth(), (float)videoElement.videoHeight() };
+
+    dispatch_async(dispatch_get_main_queue(), [protectedThis = makeRefPtr(this), this, videoElementClientRect, videoDimensions, viewRef, mode, allowsPictureInPicture] {
         ASSERT(isUIThread());
         WebThreadLock();
 
@@ -993,7 +995,7 @@ void VideoFullscreenControllerContext::setUpFullscreen(HTMLVideoElement& videoEl
 
         m_videoFullscreenView = adoptNS([PAL::allocUIViewInstance() init]);
 
-        m_interface->setupFullscreen(*m_videoFullscreenView.get(), videoElementClientRect, viewRef.get(), mode, allowsPictureInPicture, false);
+        m_interface->setupFullscreen(*m_videoFullscreenView.get(), videoElementClientRect, videoDimensions, viewRef.get(), mode, allowsPictureInPicture, false);
     });
 }
 
index 80d6940..fcc07f1 100644 (file)
@@ -1,3 +1,21 @@
+2020-06-29  Peng Liu  <peng.liu6@apple.com>
+
+        Video spills over PiP screen a little when using Picture in Picture
+        https://bugs.webkit.org/show_bug.cgi?id=213658
+
+        Reviewed by Eric Carlson.
+
+        Add the video content dimensions to the IPC message VideoFullscreenManagerProxy::SetupFullscreenWithID.
+
+        * UIProcess/Cocoa/VideoFullscreenManagerProxy.h:
+        * UIProcess/Cocoa/VideoFullscreenManagerProxy.messages.in:
+        * UIProcess/Cocoa/VideoFullscreenManagerProxy.mm:
+        (WebKit::VideoFullscreenManagerProxy::setupFullscreenWithID):
+        (WebKit::VideoFullscreenManagerProxy::setVideoDimensions):
+        (WebKit::VideoFullscreenManagerProxy::enterFullscreen):
+        * WebProcess/cocoa/VideoFullscreenManager.mm:
+        (WebKit::VideoFullscreenManager::enterVideoFullscreenForVideoElement):
+
 2020-06-29  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, reverting r262004.
index aef71e0..e8d3fe0 100644 (file)
@@ -55,6 +55,9 @@ typedef WebCore::VideoFullscreenInterfaceMac PlatformVideoFullscreenInterface;
 
 namespace WebKit {
 
+constexpr size_t DefaultMockPictureInPictureWindowWidth = 100;
+constexpr size_t DefaultMockPictureInPictureWindowHeight = 100;
+
 class WebPageProxy;
 class PlaybackSessionManagerProxy;
 class PlaybackSessionModelContext;
@@ -160,7 +163,7 @@ private:
     void hasVideoInPictureInPictureDidChange(bool);
 
     // Messages from VideoFullscreenManager
-    void setupFullscreenWithID(PlaybackSessionContextIdentifier, WebKit::LayerHostingContextID videoLayerID, const WebCore::IntRect& initialRect, float hostingScaleFactor, WebCore::HTMLMediaElementEnums::VideoFullscreenMode, bool allowsPictureInPicture, bool standby);
+    void setupFullscreenWithID(PlaybackSessionContextIdentifier, WebKit::LayerHostingContextID videoLayerID, const WebCore::IntRect& initialRect, const WebCore::FloatSize& videoDimensions, float hostingScaleFactor, WebCore::HTMLMediaElementEnums::VideoFullscreenMode, bool allowsPictureInPicture, bool standby);
     void setInlineRect(PlaybackSessionContextIdentifier, const WebCore::IntRect& inlineRect, bool visible);
     void setHasVideoContentLayer(PlaybackSessionContextIdentifier, bool value);
     void setHasVideo(PlaybackSessionContextIdentifier, bool);
@@ -188,6 +191,7 @@ private:
     void fullscreenMayReturnToInline(PlaybackSessionContextIdentifier);
 
     bool m_mockVideoPresentationModeEnabled { false };
+    WebCore::FloatSize m_mockPictureInPictureWindowSize { DefaultMockPictureInPictureWindowWidth, DefaultMockPictureInPictureWindowHeight };
 
     WebPageProxy* m_page;
     Ref<PlaybackSessionManagerProxy> m_playbackSessionManagerProxy;
index aa4b232..41f06cc 100644 (file)
@@ -24,7 +24,7 @@
 messages -> VideoFullscreenManagerProxy {
     SetHasVideo(WebKit::PlaybackSessionContextIdentifier contextId, bool hasVideo)
     SetVideoDimensions(WebKit::PlaybackSessionContextIdentifier contextId, WebCore::FloatSize videoDimensions)
-    SetupFullscreenWithID(WebKit::PlaybackSessionContextIdentifier contextId, WebKit::LayerHostingContextID videoLayerID, WebCore::IntRect initialRect, float hostingScaleFactor, WebCore::HTMLMediaElementEnums::VideoFullscreenMode videoFullscreenMode, bool allowsPictureInPicture, bool standby)
+    SetupFullscreenWithID(WebKit::PlaybackSessionContextIdentifier contextId, WebKit::LayerHostingContextID videoLayerID, WebCore::IntRect initialRect, WebCore::FloatSize videoDimensions, float hostingScaleFactor, WebCore::HTMLMediaElementEnums::VideoFullscreenMode videoFullscreenMode, bool allowsPictureInPicture, bool standby)
     EnterFullscreen(WebKit::PlaybackSessionContextIdentifier contextId)
     ExitFullscreen(WebKit::PlaybackSessionContextIdentifier contextId, WebCore::IntRect finalRect)
     SetInlineRect(WebKit::PlaybackSessionContextIdentifier contextId, WebCore::IntRect inlineRect, bool visible)
index 07f133d..376e106 100644 (file)
@@ -506,7 +506,7 @@ void VideoFullscreenManagerProxy::hasVideoInPictureInPictureDidChange(bool value
 
 #pragma mark Messages from VideoFullscreenManager
 
-void VideoFullscreenManagerProxy::setupFullscreenWithID(PlaybackSessionContextIdentifier contextId, WebKit::LayerHostingContextID videoLayerID, const WebCore::IntRect& initialRect, float hostingDeviceScaleFactor, HTMLMediaElementEnums::VideoFullscreenMode videoFullscreenMode, bool allowsPictureInPicture, bool standby)
+void VideoFullscreenManagerProxy::setupFullscreenWithID(PlaybackSessionContextIdentifier contextId, WebKit::LayerHostingContextID videoLayerID, const WebCore::IntRect& initialRect, const WebCore::FloatSize& videoDimensions, float hostingDeviceScaleFactor, HTMLMediaElementEnums::VideoFullscreenMode videoFullscreenMode, bool allowsPictureInPicture, bool standby)
 {
     MESSAGE_CHECK(videoLayerID);
 
@@ -514,6 +514,8 @@ void VideoFullscreenManagerProxy::setupFullscreenWithID(PlaybackSessionContextId
     addClientForContext(contextId);
 
     if (m_mockVideoPresentationModeEnabled) {
+        if (!videoDimensions.isEmpty())
+            m_mockPictureInPictureWindowSize.setHeight(DefaultMockPictureInPictureWindowWidth /  videoDimensions.aspectRatio());
 #if PLATFORM(IOS_FAMILY)
         requestVideoContentLayer(contextId);
 #else
@@ -540,8 +542,9 @@ void VideoFullscreenManagerProxy::setupFullscreenWithID(PlaybackSessionContextId
 #if PLATFORM(IOS_FAMILY)
     auto* rootNode = downcast<RemoteLayerTreeDrawingAreaProxy>(*m_page->drawingArea()).remoteLayerTreeHost().rootNode();
     UIView *parentView = rootNode ? rootNode->uiView() : nil;
-    interface->setupFullscreen(*model->layerHostView(), initialRect, parentView, videoFullscreenMode, allowsPictureInPicture, standby);
+    interface->setupFullscreen(*model->layerHostView(), initialRect, videoDimensions, parentView, videoFullscreenMode, allowsPictureInPicture, standby);
 #else
+    UNUSED_PARAM(videoDimensions);
     IntRect initialWindowRect;
     m_page->rootViewToWindow(initialRect, initialWindowRect);
     interface->setupFullscreen(*model->layerHostView(), initialWindowRect, m_page->platformWindow(), videoFullscreenMode, allowsPictureInPicture);
@@ -559,18 +562,26 @@ void VideoFullscreenManagerProxy::setHasVideo(PlaybackSessionContextIdentifier c
 
 void VideoFullscreenManagerProxy::setVideoDimensions(PlaybackSessionContextIdentifier contextId, const FloatSize& videoDimensions)
 {
-    if (m_mockVideoPresentationModeEnabled)
+    auto* interface = findInterface(contextId);
+    if (!interface)
         return;
 
-    if (auto* interface = findInterface(contextId))
-        interface->videoDimensionsChanged(videoDimensions);
+    if (m_mockVideoPresentationModeEnabled) {
+        if (videoDimensions.isEmpty())
+            return;
+
+        m_mockPictureInPictureWindowSize.setHeight(DefaultMockPictureInPictureWindowWidth / videoDimensions.aspectRatio());
+        return;
+    }
+
+    interface->videoDimensionsChanged(videoDimensions);
 }
 
 void VideoFullscreenManagerProxy::enterFullscreen(PlaybackSessionContextIdentifier contextId)
 {
     if (m_mockVideoPresentationModeEnabled) {
         didEnterFullscreen(contextId);
-        setVideoLayerFrame(contextId, {0, 0, 200, 150});
+        setVideoLayerFrame(contextId, {0, 0, m_mockPictureInPictureWindowSize.width(), m_mockPictureInPictureWindowSize.height()});
         return;
     }
 
index a95b7b4..0313d5b 100644 (file)
@@ -285,7 +285,7 @@ void VideoFullscreenManager::enterVideoFullscreenForVideoElement(HTMLVideoElemen
         interface->layerHostingContext()->setRootLayer(videoLayer.get());
     }
 
-    m_page->send(Messages::VideoFullscreenManagerProxy::SetupFullscreenWithID(contextId, interface->layerHostingContext()->contextID(), videoRect, m_page->deviceScaleFactor(), interface->fullscreenMode(), allowsPictureInPicture, standby));
+    m_page->send(Messages::VideoFullscreenManagerProxy::SetupFullscreenWithID(contextId, interface->layerHostingContext()->contextID(), videoRect, FloatSize(videoElement.videoWidth(), videoElement.videoHeight()), m_page->deviceScaleFactor(), interface->fullscreenMode(), allowsPictureInPicture, standby));
 }
 
 void VideoFullscreenManager::exitVideoFullscreenForVideoElement(WebCore::HTMLVideoElement& videoElement)