[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

--- Comment #20 from Olivier Blin <olivier.blin at softathome.com> ---
(In reply to comment #19)
> Comment on attachment 289542 [details]
> 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.

Ok

> > 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.

Ok

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:773
> > +    cairo_device_t* device;
> 
> So, this is not needed.

Ok

> > 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()

Ok

> > 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 '.'

Ok

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

Ok

> > 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.

Done as well

> > 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.

That's right, I will fix for both cases.
Thanks for spotting this.

> > 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

Ok

-- 
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/fecf5255/attachment.html>