In gstreamer 1.0, gst_element_factory_make() now returns a floating reference [1]. MediaPlayerPrivateGStreamer calls gst_element_factory_make() to create the playbin object but does not take ownership of the object. As a consequence, the object keeps floating until it is unref'd in the MediaPlayerPrivateGStreamer destructor. We should call gst_object_ref_sink() on the playbin object returned by gst_element_factory_make() to effectively take ownership of it. [1] http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstElementFactory.html#gst-element-factory-make
Created attachment 183772 [details] Patch
How about using an adopted GRefPtr<GstElement>?
(In reply to comment #2) > How about using an adopted GRefPtr<GstElement>? Hmm. We cannot adopt since it is floating. Did I understand correctly that you want to make m_playBin a GRefPtr<GstElement> ? If so, then we need to adopt with gstreamer 0.10 and *not* adopt with gstreamer 1.0. I'm fine with that. However, I will likely need to replace uses to m_playBin everywhere by m_playBin.get(), right?
Comment on attachment 183772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183772&action=review Right, Christophe... Well, Martin are you OK with this patch? > Source/WebCore/ChangeLog:17 > +We should call gst_object_ref_sink() on the playbin object returned by gst_element_factory_make() to effectively take ownership of it. Left over?
Comment on attachment 183772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183772&action=review >> Source/WebCore/ChangeLog:17 >> +We should call gst_object_ref_sink() on the playbin object returned by gst_element_factory_make() to effectively take ownership of it. > > Left over? Oops.
(In reply to comment #2) > How about using an adopted GRefPtr<GstElement>? Well, I do not mind using a smart pointer if you prefer. However, keep in mind this involves more changes. Also note that we are already doing the following a few lines below: // Take ownership. gst_object_ref_sink(m_videoSinkBin);
Created attachment 183778 [details] Patch Fix changelog (not using smart pointer yet).
Created attachment 183949 [details] Patch using a smart pointer (alternative)
Comment on attachment 183949 [details] Patch using a smart pointer (alternative) LGTM!
Comment on attachment 183949 [details] Patch using a smart pointer (alternative) Clearing flags on attachment: 183949 Committed r140414: <http://trac.webkit.org/changeset/140414>
All reviewed patches have been landed. Closing bug.
Comment on attachment 183949 [details] Patch using a smart pointer (alternative) View in context: https://bugs.webkit.org/attachment.cgi?id=183949&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1797 > + // In gstreamer 1.0, gst_element_factory_make returns a floating Based on the results on the Qt bots, it seems like the gstreamer 0.10 doc may be wrong and gst_element_factory_make() is returning a floating reference there as well :( I'll upload a fix soon.