GNOME Bugzilla – Bug 712137
v4l2sink: fixes for uploading compressed
Last modified: 2013-11-13 11:05:25 UTC
This report contains a number of smaller fixes for uploading compressed (H.264) over the V4L2 interface. Most of the work was already there, but I suspect it was not yet tested due to the limited amount of hardware that supports this. The fixes are: 0001-v4l2-init-v4l2_buffer-to-0x0-before-ioctl.patch: The v4l2_buffer is currently not set to 0x0 when passed to the kernel ioctl. According to the V4L2 spec, the driver does not need to set all the variables. This results in the GStreamer code using uninitialised variables as real data (boooo!). 0002-v4l2-set-bytesused-to-bytes-copied-not-bufsize.patch: when uploading data to the kernel, the amount of data passed (bytesused) is set to be the size of the kernel buffer, not of the data that is being passed upstream. In the case of uncompressed, this is the same as the data being passed from userspace; but in the case of compressed, this is not since the kernel buffer is larger than the (variable) amount of data that is being passed. 0003-v4l2-use-to-raw-copy-if-compressed.patch: In the case of compressed; the data must be passed in 'raw' form; not using the GstVideo interfaces...
Created attachment 259642 [details] [review] initialise to 0x0
Created attachment 259643 [details] [review] set bytesused correctly
Created attachment 259644 [details] [review] use raw copy for compressed
Review of attachment 259644 [details] [review]: Already fixed by fd0123800c8c1cf1468c0fa5d592ad0d0d8b4140
Review of attachment 259643 [details] [review]: Is this really necessary? gst_v4l2_object_copy() copies the buffer size along with the buffer content, it does gst_buffer_resize (dest, 0, gst_buffer_get_size (src));. Then in gst_v4l2_buffer_pool_dqbuf(), the bytesused is set from the buffer size?
Review of attachment 259642 [details] [review]: Pushed as Author: Marc Leeman <marc.leeman@gmail.com> Date: Fri Nov 8 11:09:21 2013 +0000 v4l2: init v4l2_buffer to 0x0 before ioctl https://bugzilla.gnome.org/show_bug.cgi?id=712137
Review of attachment 259643 [details] [review]: This is already fixed in 1.2+, 59d7d5c6bbb9dcd5a78213456241b31e9ec2d027
Thanks! Marking this bug as fixed.
Shall we backport to 1.0 ?
Cherry-picked the "initialise to 0x0" to the 1.2 branch, it will be in 1.2.2. It probably makes sense to cherry-pick all 3 to 1.0 ?
(In reply to comment #5) > Review of attachment 259643 [details] [review]: > > Is this really necessary? gst_v4l2_object_copy() copies the buffer size along > with the buffer content, it does gst_buffer_resize (dest, 0, > gst_buffer_get_size (src));. Then in gst_v4l2_buffer_pool_dqbuf(), the > bytesused is set from the buffer size? Well, for the 1.0.7 it was needed, before the fix, my colleague next to my was complaining loudly that I was sending crappy data over the interface, after the fix, he was silent. We need to bump to 1.2, but it's a bit busy atm :-/
'crappy data' -> 'crappy data sizes'