To avoid unref explicit calls that make Martin unhappy :)
Created attachment 114490 [details] proposed patch
Comment on attachment 114490 [details] proposed patch Attachment 114490 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10399353
Comment on attachment 114490 [details] proposed patch Attachment 114490 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10403311
Created attachment 114493 [details] proposed patch
Comment on attachment 114493 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=114493&action=review Look like you are forgetting to use adoptGRef. Other than that this patch looks good. Thank you! > Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:44 > + gst_object_ref(GST_OBJECT(ptr)); I think you should use gst_object_ref_sink here. > Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:57 > + gst_caps_ref(ptr); And here as well. > Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp:95 > + GRefPtr<GstElement> tee = gst_bin_get_by_name(GST_BIN(videoSink.get()), "videoTee"); You need to call adoptGRef here if you don't want to leak the tee. > Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp:107 > + GRefPtr<GstPad> srcPad = gst_element_get_request_pad(tee.get(), "src%d"); > + GRefPtr<GstPad> sinkPad = gst_element_get_static_pad(queue, "sink"); Ditto. > Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp:163 > + GRefPtr<GstElement> tee = gst_bin_get_by_name(GST_BIN(videoSink.get()), "videoTee"); The same for all of these as well.
(In reply to comment #5) > (From update of attachment 114493 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=114493&action=review > > Look like you are forgetting to use adoptGRef. Other than that this patch looks good. Thank you! > > > Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:44 > > + gst_object_ref(GST_OBJECT(ptr)); > > I think you should use gst_object_ref_sink here. > Yes, giving it a try. > > Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:57 > > + gst_caps_ref(ptr); > > And here as well. > GstCaps is not a GstObject. It's a lightweight refcounted structure, gst_caps_ref is enough here I think. > > Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp:95 > > + GRefPtr<GstElement> tee = gst_bin_get_by_name(GST_BIN(videoSink.get()), "videoTee"); > > You need to call adoptGRef here if you don't want to leak the tee. > Right! > > Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp:107 > > + GRefPtr<GstPad> srcPad = gst_element_get_request_pad(tee.get(), "src%d"); > > + GRefPtr<GstPad> sinkPad = gst_element_get_static_pad(queue, "sink"); > > Ditto. > > > Source/WebCore/platform/graphics/gstreamer/GStreamerGWorld.cpp:163 > > + GRefPtr<GstElement> tee = gst_bin_get_by_name(GST_BIN(videoSink.get()), "videoTee"); > > The same for all of these as well. Yes.
Created attachment 114697 [details] proposed patch
Comment on attachment 114697 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=114697&action=review Looks good, but please fix the minor issues below before landing. Thanks! > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1753 > + pad.clear(); > + pad = adoptGRef(gst_element_get_static_pad(m_webkitVideoSink, "sink")); You shouldn't need to clear the pad first, you can just reassign it. > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:234 > + GRefPtr<GstPad> targetPad = gst_element_get_static_pad(GST_ELEMENT(priv->appsrc), "src"); Missing adoptGRef here? > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:733 > + GRefPtr<GstCaps> caps = gst_caps_new_simple("application/x-icy", "metadata-interval", G_TYPE_INT, (gint) icyMetaInt, NULL); Also need to use adoptRef here as well.
Committed r99977: <http://trac.webkit.org/changeset/99977>