Bug 99362

Summary: [GStreamer] GstCaps are leaked when building with gstreamer-1.0
Product: WebKit Reporter: arno. <a.renevier>
Component: MediaAssignee: arno. <a.renevier>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, eric, feature-media-reviews, gustavo, menard, mrobinson, pnormand, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch proposal: use GRefPtr<GstCaps> for webkitGstGetPadCaps; also use GRefPtr in webkitVideoSinkRender
none
Patch none

arno.
Reported 2012-10-15 14:00:31 PDT
Hi, when using gstreamer-1.0, webkitGstGetPadCaps gets a GstCaps* with gst_pad_get_current_caps or gst_pad_query_caps which both return an object with incremented ref-count. But those caps are not unrefed.
Attachments
patch proposal: use GRefPtr<GstCaps> for webkitGstGetPadCaps; also use GRefPtr in webkitVideoSinkRender (8.63 KB, patch)
2012-10-15 14:24 PDT, arno.
no flags
Patch (8.75 KB, patch)
2012-10-15 15:21 PDT, arno.
no flags
arno.
Comment 1 2012-10-15 14:24:46 PDT
Created attachment 168782 [details] patch proposal: use GRefPtr<GstCaps> for webkitGstGetPadCaps; also use GRefPtr in webkitVideoSinkRender patch proposal
Martin Robinson
Comment 2 2012-10-15 15:01:08 PDT
Comment on attachment 168782 [details] patch proposal: use GRefPtr<GstCaps> for webkitGstGetPadCaps; also use GRefPtr in webkitVideoSinkRender View in context: https://bugs.webkit.org/attachment.cgi?id=168782&action=review Looks nice, though I have a couple nits. > Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.cpp:57 > + if (!caps) > + return 0; > +#ifdef GST_API_VERSION_1 > + return adoptGRef(caps); > +#else > + return GRefPtr<GstCaps>(caps); > +#endif > } You don't actually need to check for null here or do an explicit call to GRefPtr. Is the problem that GST_PAD_CAPS doesn't return a new reference, but gst_pad_query_caps/gst_pad_get_current_caps does? This method can just be: #ifdef GST_API_VERSION_1 GStCaps* caps = gst_pad_get_current_caps(pad); if (!caps) caps = gst_pad_query_caps(pad, 0); return adoptGRef(caps); // gst_pad_query_caps and gst_pad_get_current_caps return a new reference. #else return GST_PAD_CAPS(pad); #endif. > Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.h:25 > + > #include <gst/gst.h> Nit: There should be no empty line here. > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:-187 > -#ifdef GST_API_VERSION_1 > - gst_caps_unref(caps); > -#endif Nice cleanup!
arno.
Comment 3 2012-10-15 15:21:29 PDT
Created attachment 168791 [details] Patch updated patch
WebKit Review Bot
Comment 4 2012-10-15 17:35:19 PDT
Comment on attachment 168791 [details] Patch Clearing flags on attachment: 168791 Committed r131387: <http://trac.webkit.org/changeset/131387>
WebKit Review Bot
Comment 5 2012-10-15 17:35:23 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.