[Webkit-unassigned] [Bug 159466] [GStreamer][GL] switch to appsink

[bugzilla-daemon at webkit.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #282884|review?                     |review-
              Flags|                            |

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

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:592
> +static GstFlowReturn newSampleCallback(GstElement* sink, gpointer userData)
> +{
> +    MediaPlayerPrivateGStreamerBase* player = reinterpret_cast<MediaPlayerPrivateGStreamerBase*>(userData);

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.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:595
> +    GstSample* sample = gst_app_sink_pull_sample(GST_APP_SINK(sink));
> +
> +    player->triggerRepaint(sample);

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

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

Same comments here, the sample is leaked

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:133
> +    void triggerRepaint(GstSample*);
> +

This shouldn't be public, make the callbacks private static members if you need to access private API instead of exposing it.

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