Bug 120170

Summary: MediaPlayerPrivateAVFoundationObjC is painting video frames under the video layer
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, glenn, jer.noble, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed patch none

Simon Fraser (smfr)
Reported 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.
Attachments
Proposed patch (3.10 KB, patch)
2013-08-28 13:08 PDT, Eric Carlson
no flags
Radar WebKit Bug Importer
Comment 1 2013-08-22 13:54:53 PDT
Simon Fraser (smfr)
Comment 2 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) {
Simon Fraser (smfr)
Comment 3 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.
Eric Carlson
Comment 4 2013-08-28 13:08:47 PDT
Created attachment 209917 [details] Proposed patch
Simon Fraser (smfr)
Comment 5 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.
Eric Carlson
Comment 6 2013-08-28 13:26:09 PDT
Note You need to log in before you can comment on or make changes to this bug.