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 641927 - [encodebin] refcount issue with the "request-pad" signal
[encodebin] refcount issue with the "request-pad" signal
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.33
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-02-09 13:27 UTC by Christian Fredrik Kalager Schaller
Modified: 2011-02-24 21:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
sample code showing bug (3.06 KB, text/x-python)
2011-02-09 13:27 UTC, Christian Fredrik Kalager Schaller
Details

Description Christian Fredrik Kalager Schaller 2011-02-09 13:27:57 UTC
Created attachment 180462 [details]
sample code showing bug

Trying to put another element between uridecodebin and encodebin seems to fail. Edward looked at it and as far as he could tell it seems to be some kind of recounting issue.

<bilboed-tp> Uraeus, looks like a weird refcounting issue
<bilboed-tp> Uraeus, works with get_request_pad("video_%d") instead of that signal
 Uraeus, beats me why

Attaching small testcase, code is ugly and not proper, but it should show the issue. If the if 'video' part of the code is changed to just have the same
text as the audio part, the code starts a transcoding.
Comment 1 Edward Hervey 2011-02-09 13:31:25 UTC
The refcount of the sinkpad (video_0) isn't increased when linking the pad, resulting in it being disposed in the end.

When we request the audio sink pad, it has a refcount of 1 (we are the creator), when we link it to the source pad it gets incrememnted to 2.

When we request the video sink pad, it has a refcount of 1 (we are the creator)... but when we link it to the videoflip source pad... its refcount isn't increased (and therefore goes away once we leave python block).
Comment 2 Sebastian Dröge (slomo) 2011-02-24 15:03:58 UTC
commit dc87e8698ed900c161312830ddf87ba436ba055e
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Feb 24 16:02:50 2011 +0100

    encodebin: Fix memory leaks related to request pads
    
    Request pads have to be released by the caller and must be
    unreffed after releasing them.

commit 8067bcc54f4f0565d82a1c47081fc59c42a42ba5
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Feb 24 15:55:00 2011 +0100

    encodebin: Return a new reference of the pad for the "request-pad" signal
    
    The GObject signal code assumes that the signal handlers return a
    new reference or copy. Fixes bug #641927.



This fixes the first problem but for me it's still crashing somewhere in oggmux.
Comment 3 Sebastian Dröge (slomo) 2011-02-24 15:23:34 UTC
commit 461d9f2f2c3eaeb17233d4f0d0d6677bba7a5a80
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Feb 24 16:22:53 2011 +0100

    oggmux: Don't handle GstCollectData as GstObject, use the pad instead
Comment 4 Tim-Philipp Müller 2011-02-24 19:48:56 UTC
I get crashes in the encodebin unit test now:

gstcheck.c:72:F:general:test_encodebin_sink_pads_multiple_dynamic:0: Unexpected critical/warning: gst_object_unref: assertion `((GObject *) object)->ref_count > 0' failed
Comment 5 Sebastian Dröge (slomo) 2011-02-24 20:00:56 UTC
Sorry, fixed now:


commit eb91fe7162c35d7298fb96fd15884ee38e886b15
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Feb 24 20:59:48 2011 +0100

    encodebin: Fix double unref in unit test
Comment 6 Tim-Philipp Müller 2011-02-24 20:34:35 UTC
> commit eb91fe7162c35d7298fb96fd15884ee38e886b15
> Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
> Date:   Thu Feb 24 20:59:48 2011 +0100
> 
>     encodebin: Fix double unref in unit test

This looks really wrong to me. gst_element_get_static_pad() returns a reference, which the caller should unref, but you're removing the unrefs here.
Comment 7 Sebastian Dröge (slomo) 2011-02-24 20:52:42 UTC
These are unrefs for the request pads and they are released and unreffed some lines below
Comment 8 Tim-Philipp Müller 2011-02-24 21:05:16 UTC
Ah, right, I was looking at the code of the (almost identical) test case above that, sorry.