Changeset 257932 in webkit

Timestamp:

Mar 5, 2020 11:02:53 AM (4 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION: [ Mac ] fast/canvas/webgl/texImage2D-video-flipY-false.html is Timing out
​https://bugs.webkit.org/show_bug.cgi?id=205734

Source/WebCore:

Fix two bugs in MediaPlayerPrivateAVFoundationObjC causing the
preferred AVPlayerItemVideoOutput code path to not be taken, in
the situation where GPU-to-GPU copies are not possible because the
video's format doesn't allow them.

Implement currentTime, fixing longstanding bug in the
AVAssetImageGenerator fallback where only the first frame of the
video would be displayed.

Covered by existing layout test.

Patch by Kenneth Russell <​kbr@chromium.org> on 2020-03-05
Reviewed by Dean Jackson.

• platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
• platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:

(WebCore::MediaPlayerPrivateAVFoundationObjC::currentTime const):
(WebCore::MediaPlayerPrivateAVFoundationObjC::paintCurrentFrameInContext):
(WebCore::MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput):

LayoutTests:

Reenable layout test on macOS.

Patch by Kenneth Russell <​kbr@chromium.org> on 2020-03-05
Reviewed by Dean Jackson.

• platform/mac/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h (modified) (1 diff)
• Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-03-05  Kenneth Russell  <kbr@chromium.org>
+
+        REGRESSION: [ Mac ] fast/canvas/webgl/texImage2D-video-flipY-false.html is Timing out
+        https://bugs.webkit.org/show_bug.cgi?id=205734
+
+        Reenable layout test on macOS.
+
+        Reviewed by Dean Jackson.
+
+        * platform/mac/TestExpectations:
+
 2020-03-05  Jacob Uphoff  <jacob_uphoff@apple.com>
 

trunk/LayoutTests/platform/mac/TestExpectations

@@ -2030 +2030 @@
 webkit.org/b/207858 fast/canvas/webgl/simulated-vertexAttrib0-invalid-indicies.html [ Failure ]
 webkit.org/b/207858 fast/canvas/webgl/webgl2-texture-upload-enums.html [ Failure ]
-webkit.org/b/207858 fast/canvas/webgl/texImage2D-video-flipY-false.html [ Skip ]
 webkit.org/b/207858 webgl/1.0.3/conformance/programs/program-test.html [ Failure ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-03-05  Kenneth Russell  <kbr@chromium.org>
+
+        REGRESSION: [ Mac ] fast/canvas/webgl/texImage2D-video-flipY-false.html is Timing out
+        https://bugs.webkit.org/show_bug.cgi?id=205734
+
+        Fix two bugs in MediaPlayerPrivateAVFoundationObjC causing the
+        preferred AVPlayerItemVideoOutput code path to not be taken, in
+        the situation where GPU-to-GPU copies are not possible because the
+        video's format doesn't allow them.
+
+        Implement currentTime, fixing longstanding bug in the
+        AVAssetImageGenerator fallback where only the first frame of the
+        video would be displayed.
+
+        Covered by existing layout test.
+
+        Reviewed by Dean Jackson.
+
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::currentTime const):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::paintCurrentFrameInContext):
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput):
+
 2020-03-05  youenn fablet  <youenn@apple.com>
 

trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h

@@ -154 +154 @@
 #endif
 
+    float currentTime() const override;
     MediaTime currentMediaTime() const override;
 

trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm

@@ -1223 +1223 @@
 }
 
+float MediaPlayerPrivateAVFoundationObjC::currentTime() const
+{
+    return currentMediaTime().toFloat();
+}
+
 MediaTime MediaPlayerPrivateAVFoundationObjC::currentMediaTime() const
 {
@@ -1499 +1504 @@
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
 
-    if (videoOutputHasAvailableFrame())
+    // Callers of this will often call copyVideoTextureToPlatformTexture first,
+    // which calls updateLastPixelBuffer, which clears m_lastImage whenever the
+    // video delivers a new frame. This breaks videoOutputHasAvailableFrame's
+    // short-circuiting when m_lastImage is non-null, but the video often
+    // doesn't have a new frame to deliver since the last time
+    // hasNewPixelBufferForItemTime was called against m_videoOutput. To avoid
+    // changing the semantics of videoOutputHasAvailableFrame in ways that might
+    // break other callers, look for production of a recent pixel buffer from
+    // the video output, too.
+    if (videoOutputHasAvailableFrame() || (m_videoOutput && m_lastPixelBuffer))
         paintWithVideoOutput(context, rect);
     else
@@ -2221 +2235 @@
 void MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput(GraphicsContext& context, const FloatRect& outputRect)
 {
-    updateLastImage(UpdateType::UpdateSynchronously);
+    // It's crucial to not wait synchronously for the next image. Videos that
+    // come down this path are performing slow-case software uploads, and such
+    // videos may not return metadata in a timely fashion. Use the most recently
+    // available pixel buffer, if any.
+    updateLastImage();
     if (!m_lastImage)
         return;