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 796814 - gst_rtsp_media_set_clock() leaks and is not listed in the official documentation
gst_rtsp_media_set_clock() leaks and is not listed in the official documentation
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Linux
: Normal normal
: 1.14.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-07-15 09:44 UTC by Carlos Rafael Giani
Modified: 2018-07-16 23:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gst_rtsp_media_set_clock docs patch (819 bytes, patch)
2018-07-16 19:57 UTC, Carlos Rafael Giani
committed Details | Review
rtsp-media clock cleanup patch (847 bytes, patch)
2018-07-16 19:59 UTC, Carlos Rafael Giani
committed Details | Review

Description Carlos Rafael Giani 2018-07-15 09:44:17 UTC
First, I cannot find gst_rtsp_media_set_clock() here, even though it s part of the official API: https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-rtsp-server/html/GstRTSPMedia.html

Second, I think this function causes a leak. Inside, I see that it first unref's any previously set clock, and then refs the new clock. However, I don't see anything in gst_rtsp_media_finalize() that unrefs that clock.

This might explain why I haven't seen this leak before - if no clock is explicitely set, then automatic clock negotiation is performed in the media's pipeline, which does not touch the gst_object_ref() call inside gst_rtsp_media_set_clock().

So, the fix would be to unref priv->clock if it isn't NULL. Or am I missing something?
Comment 1 Sebastian Dröge (slomo) 2018-07-16 06:57:17 UTC
Yes, it should be finalized by unreffing. The same bug in GstRTSPMediaFactory

It does not appear in the docs because it was not added to docs/libs/gst-rtsp-server-sections.txt

Do you want to provide a patch for both?
Comment 2 Carlos Rafael Giani 2018-07-16 19:57:58 UTC
Created attachment 373067 [details] [review]
gst_rtsp_media_set_clock docs patch
Comment 3 Carlos Rafael Giani 2018-07-16 19:59:11 UTC
Created attachment 373068 [details] [review]
rtsp-media clock cleanup patch

Here are both patches. These should take care of the documented issues. Also note that the rtsp media factory class _does_ unref the clock, so it does not need fixing.
Comment 4 Tim-Philipp Müller 2018-07-16 23:02:08 UTC
Thanks, pushed. Took liberty to add the media_factory funcs to API docs too.

commit 12c2dd6e1c725b6072b8b9fc06ddda0e2057b4b5
Author: Carlos Rafael Giani <dv@pseudoterminal.org>
Date:   Mon Jul 16 21:57:08 2018 +0200

    rtsp-media: unref clock (if set) when finalizing
    
    https://bugzilla.gnome.org/show_bug.cgi?id=796814

commit e99471a56b7b122cba2ca54298c054c936500ad7
Author: Carlos Rafael Giani <dv@pseudoterminal.org>
Date:   Mon Jul 16 21:56:44 2018 +0200

    rtsp-media: add gst_rtsp_media_*_set_clock to docs
    
    https://bugzilla.gnome.org/show_bug.cgi?id=796814