Bug 172542

Summary: [GTK][WPE][GSTREAMER_GL] Do not use cairo to copy video textures to an accelerated canvas
Product: WebKit Reporter: Miguel Gomez <magomez>
Component: WebKitGTKAssignee: Miguel Gomez <magomez>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, buildbot, commit-queue, webkit-bug-importer, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews125 for ios-simulator-wk2
none
Patch none

Description Miguel Gomez 2017-05-24 09:40:05 PDT
Instead of using the current approach of copying the video frame using cairo, use VideoTextureCopierGStreamer, as it's done when copying frames to webgl. This will allow to perform color adjustments with the shader during the GPU, which will be needed as we're going to change the video frames format to BGRA.
Comment 1 Miguel Gomez 2017-05-24 10:01:49 PDT
Created attachment 311124 [details]
Patch
Comment 2 Build Bot 2017-05-24 11:26:43 PDT
Comment on attachment 311124 [details]
Patch

Attachment 311124 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3807875

New failing tests:
fast/events/before-unload-returnValue.html
Comment 3 Build Bot 2017-05-24 11:26:44 PDT
Created attachment 311138 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 4 Zan Dobersek 2017-05-29 06:58:43 PDT
Comment on attachment 311124 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:983
> +    unsigned textureID = *reinterpret_cast<unsigned*>(videoFrame.data[0]);

This variable can be set just before the copyVideoTextureToPlatformTexture() call.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:998
> +    RefPtr<cairo_surface_t> surface = adoptRef(cairo_gl_surface_create_for_texture(context->cairoDevice(), CAIRO_CONTENT_COLOR_ALPHA, m_videoTextureCopier->resultTexture(), size.width(), size.height()));
> +    return surface;

This can be in one line.

> Source/WebCore/platform/graphics/gstreamer/VideoTextureCopierGStreamer.cpp:146
> +    // Use our own output texture if we are not given one.
> +    if (!outputTexture)
> +        outputTexture = m_resultTexture;

Do we want to wait with the m_resultTexture creation until this point?
Comment 5 Zan Dobersek 2017-05-29 08:20:01 PDT
Comment on attachment 311124 [details]
Patch

Fine otherwise.
Comment 6 Miguel Gomez 2017-05-30 02:58:48 PDT
Created attachment 311497 [details]
Patch
Comment 7 WebKit Commit Bot 2017-05-30 06:02:50 PDT
Comment on attachment 311497 [details]
Patch

Clearing flags on attachment: 311497

Committed r217551: <http://trac.webkit.org/changeset/217551>
Comment 8 WebKit Commit Bot 2017-05-30 06:02:52 PDT
All reviewed patches have been landed.  Closing bug.