Bug 120170 - MediaPlayerPrivateAVFoundationObjC is painting video frames under the video layer
Summary: MediaPlayerPrivateAVFoundationObjC is painting video frames under the video l...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-08-22 13:54 PDT by Simon Fraser (smfr)
Modified: 2013-08-28 13:26 PDT (History)
6 users (show)

See Also:


Attachments
Proposed patch (3.10 KB, patch)
2013-08-28 13:08 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2013-08-22 13:54:38 PDT
RenderVideo::paintReplaced() is called any time the render tree is being painted. In the non-flattening case, this calls into mediaPlayer->paint(), and MediaPlayerPrivateAVFoundationObjC: then calls down into MediaPlayerPrivateAVFoundationObjC::paintWithVideoOutput() which does CIContext stuff.

This is crazy because you're painting a video frame into a layer whose sublayer is showing video frames, and means that whenever the page paints, you're getting a CVPixelBufferRef which is expensive.
Comment 1 Radar WebKit Bug Importer 2013-08-22 13:54:53 PDT
<rdar://problem/14811861>
Comment 2 Simon Fraser (smfr) 2013-08-22 14:57:11 PDT
Maybe patch:

diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
index 1615340eec1152c3366639b41fcc5b4e85de9e2c..ecfa806819780e8dc434cc7065a05ce7b7c109c5 100644
--- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
+++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
@@ -787,14 +787,6 @@ void MediaPlayerPrivateAVFoundationObjC::paintCurrentFrameInContext(GraphicsCont
     if (!metaDataAvailable() || context->paintingDisabled())
         return;
 
-    paint(context, rect);
-}
-
-void MediaPlayerPrivateAVFoundationObjC::paint(GraphicsContext* context, const IntRect& rect)
-{
-    if (!metaDataAvailable() || context->paintingDisabled())
-        return;
-
     setDelayCallbacks(true);
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
 
@@ -810,6 +802,19 @@ void MediaPlayerPrivateAVFoundationObjC::paint(GraphicsContext* context, const I
     m_videoFrameHasDrawn = true;
 }
 
+void MediaPlayerPrivateAVFoundationObjC::paint(GraphicsContext* context, const IntRect& rect)
+{
+    LOG(Media, "MediaPlayerPrivateAVFoundationObjC::paint(%p) - metaDataAvailable = %d", this, metaDataAvailable());
+
+    if (!metaDataAvailable() || context->paintingDisabled())
+        return;
+
+    if (currentRenderingMode() == MediaRenderingToLayer)
+        return;
+
+    paintCurrentFrameInContext(context, rect);
+}
+
 #if __MAC_OS_X_VERSION_MIN_REQUIRED < 1080
 void MediaPlayerPrivateAVFoundationObjC::paintWithImageGenerator(GraphicsContext* context, const IntRect& rect)
 {
Comment 3 Simon Fraser (smfr) 2013-08-22 14:58:42 PDT
After some testing, this only happens if the <video> has style that requires it to have a layer (e.g. border, background color, shadow, outline etc), and the painting happens on things like window resize and selection.
Comment 4 Eric Carlson 2013-08-28 13:08:47 PDT
Created attachment 209917 [details]
Proposed patch
Comment 5 Simon Fraser (smfr) 2013-08-28 13:16:48 PDT
Comment on attachment 209917 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=209917&action=review

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:804
> +    
>  #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1080
>      paintWithVideoOutput(context, rect);
>  #else
>      paintWithImageGenerator(context, rect);
>  #endif
> -
> +    
>      END_BLOCK_OBJC_EXCEPTIONS;
>      setDelayCallbacks(false);
> -
> +    

Some non-empty lines here.
Comment 6 Eric Carlson 2013-08-28 13:26:09 PDT
Committed r154775: https://trac.webkit.org/r154775