[Webkit-unassigned] [Bug 159928] [GStreamer] Support a direct GPU-to-GPU copy of video textures to WebGL

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=159928

Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #289542|review?, commit-queue?      |review+, commit-queue-
              Flags|                            |

--- Comment #19 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 289542
  --> https://bugs.webkit.org/attachment.cgi?id=289542
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:701
> +GLContext* MediaPlayerPrivateGStreamerBase::prepareContextForCairoPaint(cairo_device_t*& device, GstVideoInfo& videoInfo, IntSize& size, IntSize& rotatedSize)

We don't need to return the device, you can get it from the context.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:710
> +    cairo_gl_device_set_thread_aware(device, FALSE);

You can do cairo_gl_device_set_thread_aware(glContext->cairoDevice(), FALSE); directly.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:773
> +    cairo_device_t* device;

So, this is not needed.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:778
> +    if (!prepareContextForCairoPaint(device, videoInfo, size, rotatedSize))
> +        return false;

Get the context here and use it below to pass the cairo device to paintToCairoSurface()

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:784
> +    // cairo_gl_surface_create_for_texture sets these parameters to GL_NEAREST, so we need to backup them

Nit: finish comments with '.'

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:806
> +    cairo_device_t* device;

Same here.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:813
> +    GLContext* context = prepareContextForCairoPaint(device, videoInfo, size, rotatedSize);
> +    if (!context)
> +        return nullptr;
> +    context->makeContextCurrent();

I still think that making the context current in prepareContextForCairoPaint wouldn't hurt even if it's not needed in the other case.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:819
> +    cairo_surface_t* rotatedSurface = cairo_gl_surface_create(device, CAIRO_CONTENT_COLOR_ALPHA, rotatedSize.width(), rotatedSize.height());
> +    if (!paintToCairoSurface(rotatedSurface, device, videoInfo, size, rotatedSize))
> +        return nullptr;
> +
> +    return adoptRef(rotatedSurface);

The smart pointer was correct here, now you are leaking the surface if paintToCairoSurface() returns false.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:136
> +    bool paintToCairoSurface(cairo_surface_t* outputSurface, cairo_device_t*, GstVideoInfo&, const IntSize&, const IntSize&);

outputSurface can be omitted in this case

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160922/955eeb9e/attachment.html>