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 796860 - decodebin3: eos event leaks with short mp4 file
decodebin3: eos event leaks with short mp4 file
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 1.14.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 796859 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2018-07-24 01:17 UTC by rland
Modified: 2018-10-18 13:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
1s duration test file (25.01 KB, video/mp4)
2018-07-24 01:17 UTC, rland
  Details
proposed patch (729 bytes, patch)
2018-07-24 03:42 UTC, rland
reviewed Details | Review
proposed patch2 (777 bytes, patch)
2018-07-24 11:26 UTC, rland
none Details | Review

Description rland 2018-07-24 01:17:49 UTC
Created attachment 373131 [details]
1s duration test file

ubuntu 16.04 + 1.14.2

GST_TRACERS="leaks" GST_DEBUG="GST_TRACER:7" gst-launch-1.0 playbin3 uri=http://10.9.44.116/vod/mp4/shorttime/girls_time_360p_with_timecode_1s.mp4
...
race=(string)%s;
0:00:00.028976223  7436       0xc8e290 TRACE             GST_TRACER gsttracerrecord.c:111:gst_tracer_record_build_format: object-refings.class, ts=(structure)"value\,\ type\=\(type\)guint64\,\ related-to\=\(GstTracerValueScope\)GST_TRACER_VALUE_SCOPE_PROCESS\;", type-name=(structure)"value\,\ type\=\(type\)gchararray\,\ related-to\=\(GstTracerValueScope\)GST_TRACER_VALUE_SCOPE_PROCESS\;", address=(structure)"value\,\ type\=\(type\)gpointer\,\ related-to\=\(GstTracerValueScope\)GST_TRACER_VALUE_SCOPE_PROCESS\;", description=(structure)"value\,\ type\=\(type\)gchararray\,\ related-to\=\(GstTracerValueScope\)GST_TRACER_VALUE_SCOPE_PROCESS\;", ref-count=(structure)"value\,\ type\=\(type\)guint\,\ related-to\=\(GstTracerValueScope\)GST_TRACER_VALUE_SCOPE_PROCESS\;", trace=(structure)"value\,\ type\=\(type\)gchararray\,\ related-to\=\(GstTracerValueScope\)GST_TRACER_VALUE_SCOPE_PROCESS\;";
0:00:00.028992260  7436       0xc8e290 DEBUG             GST_TRACER gsttracerrecord.c:125:gst_tracer_record_build_format: new format string: object-refings, ts=(guint64)%lu, type-name=(string)%s, address=(gpointer)%p, description=(string)%s, ref-count=(uint)%u, trace=(string)%s;
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Redistribute latency...
Redistribute latency...
Redistribute latency...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstPulseSinkClock
Got EOS from element "playbin3-0".
Execution ended after 0:00:01.068662349
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...
0:00:01.415559755  7436       0xc8e290 TRACE             GST_TRACER :0:: object-alive, type-name=(string)GstEvent, address=(gpointer)0x7f25dc003a70, description=(string)eos event: 0x7f25dc003a70, time 99:99:99.999999999, seq-num 22, (NULL), ref-count=(uint)1, trace=(string);

** (gst-launch-1.0:7436): WARNING **: Leaks detected
Comment 1 rland 2018-07-24 01:25:49 UTC
In addition, playbin2 is OK.
Comment 2 Nicolas Dufresne (ndufresne) 2018-07-24 01:28:14 UTC
*** Bug 796859 has been marked as a duplicate of this bug. ***
Comment 3 rland 2018-07-24 03:42:18 UTC
Created attachment 373132 [details] [review]
proposed patch
Comment 4 Tim-Philipp Müller 2018-07-24 07:41:39 UTC
Comment on attachment 373132 [details] [review]
proposed patch

Thanks for the patch!

Not sure this is entirely right.

I think we only need to unref the event if the return value is GST_PAD_PROBE_HANDLED, but there seems to be at least one code path where it's going to be GST_PAD_PROBE_REMOVE and I think if we return REMOVE we mustn't unref the event?
Comment 5 rland 2018-07-24 09:59:22 UTC
>I think we only need to unref the event if the return value is >GST_PAD_PROBE_HANDLED, but there seems to be at least one code path where it's >going to be GST_PAD_PROBE_REMOVE

Yeah,but at least before L1829(https://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst/playback/gstdecodebin3.c#n1829) break, we didn't deal with the eos event, so if we want to break at L1829, we have to unref in any case. :)
Comment 6 rland 2018-07-24 11:26:34 UTC
Created attachment 373139 [details] [review]
proposed patch2

I thought about it again, the way you suggest it may be better and safer, or it is better to re-submit a patch. :)
Comment 7 Tim-Philipp Müller 2018-10-18 13:18:04 UTC
Sorry for the delay. Pushed to master and 1.14 branch for 1.14.5 now, many thanks for the patch!

commit a15baf797603c6c65e442f488f20993d800bbbc1
Author: Roland Jon <rlandjon@gmail.com>
Date:   Tue Jul 24 18:40:36 2018 +0800

    decodebin3: fix eos event leak
    
    https://bugzilla.gnome.org/show_bug.cgi?id=796860