After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 773515 - rtph263ppay: Fix caps leak
rtph263ppay: Fix caps leak
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: 1.10.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-10-26 07:59 UTC by Stian Selnes (stianse)
Modified: 2016-11-04 17:39 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
rtph263ppay: Fix caps leak (2.36 KB, patch)
2016-10-26 07:59 UTC, Stian Selnes (stianse) 		committed Details | Review

Description Stian Selnes (stianse) 2016-10-26 07:59:15 UTC 
Fix leaking caps when downstream has not-fixed caps.
Comment 1 Stian Selnes (stianse) 2016-10-26 07:59:20 UTC 
Created attachment 338480 [details] [review]
rtph263ppay: Fix caps leak
Comment 2 Sebastian Dröge (slomo) 2016-10-26 08:08:46 UTC 
Review of attachment 338480 [details] [review]:

::: gst/rtp/gstrtph263ppay.c
@@ +270,2 @@
     caps =
         gst_pad_get_pad_template_caps (GST_RTP_BASE_PAYLOAD_SINKPAD (payload));

Shouldn't this return the intersection between the two then?
Comment 3 Stian Selnes (stianse) 2016-10-26 08:32:01 UTC 
(In reply to Sebastian Dröge (slomo) from comment #2)

> Shouldn't this return the intersection between the two then?

Agree. The patch only plugs a leak, but I can craft a test case and update the patch fixing that behavior as well.
Comment 4 Stian Selnes (stianse) 2016-10-26 09:07:20 UTC 
Taking a closer look at the code I think it works as intended. Intersect is wrong since we're checking the peercaps downstream (rtp-caps) and returning a h263-caps.

If the rtp-caps is fixed there is logic for checking whether there are any fields of the rtp-caps that should be transferred to the returned h263-caps. There could potentially be a use case for checking this also when the rtp-caps is not fixed (and we enter the reviewed code path), but I propose we keep the logic as it's been for a long time instead of complicate things to support a made up use case.

I propose to apply the patch as it is to fix the leak.
Comment 5 Sebastian Dröge (slomo) 2016-11-01 18:21:02 UTC 
Attachment 338480 [details] pushed as 78ab8cb - rtph263ppay: Fix caps leak