GNOME Bugzilla – Bug 736118
videofilter: The buffer is not writable in transform_frame_ip
Last modified: 2014-09-13 13:50:09 UTC
Created attachment 285476 [details] Example test case. When creating a simple video filter the buffer is not writable in transform_frame_ip: static GstFlowReturn gst_test_transform_frame_ip (GstVideoFilter * filter, GstVideoFrame * frame) { GstTest *test = GST_TEST (filter); GST_DEBUG_OBJECT (test, "transform_frame_ip"); GST_INFO_OBJECT (test, gst_buffer_is_writable(frame->buffer) ? "writable" : "not writable"); return GST_FLOW_OK; }
Happens because the GstVideoFrame has yet another reference to the buffer.
We could probably unref the buffer once before calling transform/transform_ip
Created attachment 285480 [details] [review] videofilter: Unref buffers before calling the transform_frame functions GstVideoFrame has another reference, so the buffer looks unwriteable, meaning that we can't attach any metas or anything to it
Untested patch, ugly but maybe the best we can do here.
Works great for me! Thanks. Do you think this will turn up in 1.4.2 or 1.6.0?
Review of attachment 285480 [details] [review]: That is really ugly. Can't we find a way to transfer the buffer ownership to the VideoFrame instead, and keep that around ?
Yeah, maybe a custom map flag? GST_VIDEO_FRAME_MAP_NOREF?
(In reply to comment #7) > Yeah, maybe a custom map flag? GST_VIDEO_FRAME_MAP_NOREF? Hmm, seems tricky, mapping memory will ref it, though it's more at buffer level that it fails. It's not like if we where to create a Frame and then map/unmap it, that would have been simple to solve the issue ... I need to think a bit more about that.
Mapping buffers or memory does not ref it, only the GstVideoFrame API refs the buffer when mapping
Review of attachment 285480 [details] [review]: Ok, the context is a bit missing in the patch itself, but as we map the frame (hence ref the buffer), unref the buffer, transform frame, ref buffer and finally unmap the frame (hence unref the buffer), this patch clearly works. It a weird dance, but it clearly show that we know what we are doing. I like the flag in the end, but we need this method first in the likely case we want to backport to 1.4. Feel free to introduce the flag in 1.5 though, then it would be worth adding this change at the other places (e.g. in OpenCV elements base class, which isn't base on VideoFilter).
Let's do that then. I added a longer explanation in the code. commit f711288c7c86c33aed18f52ed6d7787d19b7741d Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Sep 12 14:27:44 2014 +0300 videofilter: Unref buffers before calling the transform_frame functions GstVideoFrame has another reference, so the buffer looks unwriteable, meaning that we can't attach any metas or anything to it https://bugzilla.gnome.org/show_bug.cgi?id=736118
I'll backport the first patch to 1.4 later, for 1.5 we have a better solution now: commit 3a7cdcdfc9c5b0d20394fe51b3b8cda23931ca6d Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Sep 12 14:41:01 2014 +0300 videofilter: Use new GST_VIDEO_FRAME_MAP_FLAG_NO_REF https://bugzilla.gnome.org/show_bug.cgi?id=736118 commit 40a293d44d1aeccf5eb8e86f23a0b13666111c5c Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Sep 12 14:39:16 2014 +0300 video-frame: Add GST_VIDEO_FRAME_MAP_FLAG_NO_REF This makes sure that the buffer is not reffed another time when storing it in the GstVideoFrame, keeping it writable if it was writable. https://bugzilla.gnome.org/show_bug.cgi?id=736118