<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][GL] switch to appsink"
href="https://bugs.webkit.org/show_bug.cgi?id=159466">bug 159466</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 #282884 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer][GL] switch to appsink"
href="https://bugs.webkit.org/show_bug.cgi?id=159466#c2">Comment # 2</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer][GL] switch to appsink"
href="https://bugs.webkit.org/show_bug.cgi?id=159466">bug 159466</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=282884&action=diff" name="attach_282884" title="patch">attachment 282884</a> <a href="attachment.cgi?id=282884&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=282884&action=review">https://bugs.webkit.org/attachment.cgi?id=282884&action=review</a>
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:592
> +static GstFlowReturn newSampleCallback(GstElement* sink, gpointer userData)
> +{
> + MediaPlayerPrivateGStreamerBase* player = reinterpret_cast<MediaPlayerPrivateGStreamerBase*>(userData);</span >
Since you are casting the callback in the g_signal_connect with G_CALLBACK, you can directly use MediaPlayerPrivateGStreamerBase* as parameter and avoid this cast.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:595
> + GstSample* sample = gst_app_sink_pull_sample(GST_APP_SINK(sink));
> +
> + player->triggerRepaint(sample);</span >
You don't need the local variable, unless triggerRepaint doesn't adopt the sample, in which case, you need the local variable but you are leaking the sample here, since gst_app_sink_pull_sample is transfer full
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:604
> + MediaPlayerPrivateGStreamerBase* player = reinterpret_cast<MediaPlayerPrivateGStreamerBase*>(userData);
> + GstSample* sample = gst_app_sink_pull_preroll(GST_APP_SINK(sink));
> +
> + player->triggerRepaint(sample);</span >
Same comments here, the sample is leaked
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:133
> + void triggerRepaint(GstSample*);
> +</span >
This shouldn't be public, make the callbacks private static members if you need to access private API instead of exposing it.</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>