<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer] Support a direct GPU-to-GPU copy of video textures to WebGL"
href="https://bugs.webkit.org/show_bug.cgi?id=159928">bug 159928</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #289542 Flags</td>
<td>review?, commit-queue?
</td>
<td>review+, commit-queue-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer] Support a direct GPU-to-GPU copy of video textures to WebGL"
href="https://bugs.webkit.org/show_bug.cgi?id=159928#c19">Comment # 19</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer] Support a direct GPU-to-GPU copy of video textures to WebGL"
href="https://bugs.webkit.org/show_bug.cgi?id=159928">bug 159928</a>
from <span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=289542&action=diff" name="attach_289542" title="Patch">attachment 289542</a> <a href="attachment.cgi?id=289542&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=289542&action=review">https://bugs.webkit.org/attachment.cgi?id=289542&action=review</a>
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:701
> +GLContext* MediaPlayerPrivateGStreamerBase::prepareContextForCairoPaint(cairo_device_t*& device, GstVideoInfo& videoInfo, IntSize& size, IntSize& rotatedSize)</span >
We don't need to return the device, you can get it from the context.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:710
> + cairo_gl_device_set_thread_aware(device, FALSE);</span >
You can do cairo_gl_device_set_thread_aware(glContext->cairoDevice(), FALSE); directly.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:773
> + cairo_device_t* device;</span >
So, this is not needed.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:778
> + if (!prepareContextForCairoPaint(device, videoInfo, size, rotatedSize))
> + return false;</span >
Get the context here and use it below to pass the cairo device to paintToCairoSurface()
<span class="quote">> 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</span >
Nit: finish comments with '.'
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:806
> + cairo_device_t* device;</span >
Same here.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:813
> + GLContext* context = prepareContextForCairoPaint(device, videoInfo, size, rotatedSize);
> + if (!context)
> + return nullptr;
> + context->makeContextCurrent();</span >
I still think that making the context current in prepareContextForCairoPaint wouldn't hurt even if it's not needed in the other case.
<span class="quote">> 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);</span >
The smart pointer was correct here, now you are leaking the surface if paintToCairoSurface() returns false.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:136
> + bool paintToCairoSurface(cairo_surface_t* outputSurface, cairo_device_t*, GstVideoInfo&, const IntSize&, const IntSize&);</span >
outputSurface can be omitted in this case</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>