[Bug 735660] v4l2: fix new v4l2 code not working with certain devices (regression)

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=735660
  GStreamer | gst-plugins-good | 1.4.1

--- Comment #9 from Hans de Goede <jwrdegoede at fedoraproject.org> 2014-08-29 19:24:01 UTC ---
(In reply to comment #6)
> Review of attachment 284800 [details]:
> 
> ::: sys/v4l2/gstv4l2allocator.c
> @@ +489,3 @@
> +    breq.count = 0;
> +    breq.memory = memory;
> +    v4l2_ioctl (allocator->video_fd, VIDIOC_REQBUFS, &breq);
> 
> Just spoke with Hans. 18 drivers are affected, 16 in 3.17. Though he said:à
> 
> <hverkuil> Realistically, if reqbufs(0) is not supported by the driver, then
> the only safe alternative is to close and reopen the device node.
> 
> So this patch isn't correct.

(In reply to comment #8)
> Unless I'm mistaken, this method would only work if you made sure
> S_FMT is not called afterward. I don't want this approach since it will cause
> further issue for memory type like DMABUF and USERPTR, as it will prevent any
> attempt to adapt memory alignment.

<sigh>, strictly speaking Hans V. is correct, the only 101% safe way is freeing
the buffers, but in practice most drivers which are affected to allow changing
the amount of buffers, and doing a set_fmt after buffers have been requested,
as long as no streaming is being done, as is clearly mentioned in the commit
message of the patch.

Without this patch gstreamer apps do not work with my bttv cards, where as with
1.0 (don't know if I ever got around to testing this with 1.2) they did work.
This clearly is a regression, and this clearly needs to be fixed.

As I've already said in comment #3 before you even started registering your
objections, it may be better to only try with 1 buffer if the request with 0
buffers fails. And yes that code path should have a comment explaining this is
a workaround for certain devices.

Or even better why do the probe if the device supports normal mmap buffers at
all? AFAIK if the device claims V4L2_CAP_STREAMING it must at least support the
standard mmap type buffers, so we can just assume that. We should probably
double check this with Hans V.

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