GNOME Bugzilla – Bug 585551
AppSrc bindings do not handle buffer ownership correctly
Last modified: 2009-09-11 08:51:58 UTC
There are two problems: - An error about an assertion about the refcount of a miniobject. - An System.AccessViolationException thrown for the buffer data. The first one occurs when using AppSrc.PushBuffer(buffer). The second results from using new Gst.Buffer(byte[] data); to construct the buffer to push to the appsrc.
Created attachment 136416 [details] Testcase for AppSrc.PushBuffer This program exposes the bug. It should just show a moving line on a yellow background until the video window is closed. Both for constructing the buffer and pushing it to the appsrc there are two ways of doing it. The one that uses the preferred api does not work, see comments in the file.
commit b3b018c916fbdc5009839ba1c04f85b55ab62973 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Fri Jun 12 15:21:28 2009 +0200 AppSrc.PushBuffer() takes ownership of the buffer Part of bug #585551. Now it works for me (as attached), well, it doesn't crash anymore :) But it uses more and more memory... I'll investigate.
Created attachment 136438 [details] [review] Fix for generator Why not fix the generator instead?
(In reply to comment #3) > Created an attachment (id=136438) [edit] > Fix for generator > > Why not fix the generator instead? To have a fast solution available to continue debugging :) Thanks for that patch, I've adapted it to also have the same for GObject. Now some custom code can go away
Created attachment 136489 [details] [review] Now some custom code can go away...
Actually some more custom code can go away: Gst.Pad.Push(), Gst.Bus.Post(), Gst.Bin.Add(), ... These are not using .OwnedHandle but instead call gst_mini_object_ref() on the Handle by hand. I'll commit your patch tomorrow and any other patch you've attached until then to remove the other custom code, otherwise I'll remove the custom code then ;)
Created attachment 136492 [details] [review] Now even more custom code can go away... I only touched custom code which used gst_mini_object_ref. The other places like gst_mini_object_copy and Bin.Add are left as an excercise for the committer ;-)
Thanks, it's all committed now :)
The buffer creation works too now: commit 1c1fe228b4d9d9b669bed91526e25f5b80bc6347 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Wed Jun 17 13:36:53 2009 +0200 Don't forget to set the freefunc of buffers Partially fixes bug #585551. Is there anything left for this bug except the increasing memory usage?
Yes! with this patch all the issues are fixed. Also the failure of PadTest.TestPushUnlinked I reported in #585541 is gone now, so we have a regression test in place and don't need the attached testcase anymore.
Great :) But does your attached test case use more and more memory over time for you too? I have no idea what exactly is leaking here, the native buffers (and their data) are all freed. Also, if you want to prepare a patch we can add that test case as an example application :)