[gstreamer-bugs] [Bug 596078] Playbin2 takes ref of audio-/video-sink parameter

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=596078
  GStreamer | gst-plugins-base | git

--- Comment #8 from Sebastian Dröge <slomo at circular-chaos.org> 2009-09-29 05:37:44 UTC ---
(In reply to comment #7)
> Am I missing something? gst_play_sink_set_sink/gst_play_sink_set_vis_plugin
> already add a ref, so gst_value_get_object is fine in the caller.
> 
> I think the problem is the one I started discussing with Wim some time ago,
> that gstplaysink calls gst_object_sink to sink the floating reference on the
> passed element. That seems to have been removed for the audio/video sinks when
> gst_play_sink_set_video_sink and gst_play_sink_set_audio_sink were unified into
> the gst_play_sink_set_sink() function, but it still seems to be true for the
> gst_play_sink_set_vis_plugin() function - causing some inconsistency.
> 
> I think playbin2 should not sink the ref on the passed objects, and make it
> uniformly the caller's responsibility.

The problem is, that _get_object is called (meaning, you don't own the
reference). This is then reffed and sinked later (refcount += 1 and the
refcount -= 1). So in the end playbin/playsink don't own any reference to the
passed object (only the GValue owns one reference, GLib owns another reference
internally and the caller owns the remaining reference).

So right, either playbin/playsink shouldn't sink the object and only ref it. Or
it should be as with my current patch ;)

I don't know if playbin/playsink should sink the object, it probably makes
sense because they "own" the object.

Jan, do you think we should fix that for 0.10.25 already?

(In reply to comment #6)
> Camerabin add refcounts. Its probably good to ref. It would be nice to cover
> that in our unit tests too.
> 
> The problem with the unref is that it causes a leak for apps that used custom
> elements so already. So I agree with Arnout that we need to point that out very
> clearly.

Yes, should definitely be in the release notes (although we don't give any API
guarantees for playbin2 yet).

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.