GNOME Bugzilla – Bug 516649
[v4l2src] tries to VIDIOC_S_PARM without checking capabilities
Last modified: 2008-03-25 12:33:20 UTC
In v4l2src_calls.c:gst_v4l2src_set_capture VIDIOC_S_PARM always get used with a update timeperframe field. That is as long as the G_PARM ioctl succeeds. Now according to the v4l2spec this field is only supported when the driver has V4L2_CAP_TIMEPERFRAME in its capabilities field. Both my bttv card and my pwc webcam don't have this so they always give an (unneeded error) I'll attach a patch to check for this capability before doing {G,S}_PARM
Created attachment 105313 [details] [review] proposed patch
I'm not a v4l2 expert, but this looks pretty much in line with what the documentation says. However, it appears that at least with my pwc cam (Macbook Pro rev. 3 built-in iSight, ubuntu gutsy) the CAP_TIMEPERFRAME flag is not set, but the ioctls still work just fine, which would be a driver bug I suppose, but it still disables functionality that's currently there, so I'm not sure what the best thing to do is here.
I believe this patch is not correct. The statement if (v4l2src->v4l2object->vcap.capabilities & V4L2_CAP_TIMEPERFRAME) is in error, because vcap is a v4l2_captureparm structure, whereas the flag V4L2_CAP_TIMEPERFRAME is only present in a v4l2_capability structure. You may wish to look at the corresponding part of the proposed patch in bug #520092 to see how (I believe) this may be done correctly.
> I believe this patch is not correct. The statement > if (v4l2src->v4l2object->vcap.capabilities & V4L2_CAP_TIMEPERFRAME) > is in error, because vcap is a v4l2_captureparm structure, whereas the flag > V4L2_CAP_TIMEPERFRAME is only present in a v4l2_capability structure. I think you're right. This is is the list of flags used in the capabilities field of a v4l2_capability structure according to the docs: http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec/r13105.htm#DEVICE-CAPABILITIES > You may wish to look at the corresponding part of the proposed patch in bug > #520092 to see how (I believe) this may be done correctly. I've committed something based on your patch now: 2008-03-25 Tim-Philipp Müller <tim at centricular dot net> Based on patch by: William M. Brack <wbrack at mmm com hk> * sys/v4l2/v4l2src_calls.c: (fractions_are_equal), (gst_v4l2src_set_capture): Check whether the device supports setting the framerate before trying to set it and then posting a warning or error if it doesn't work (#516649, #520092). Also compare fractions more correctly. I guess it's better to output data with a non-wanted framerate than to error out, at least while we require a framerate to be specified in the caps. Some refactoring might be in order here one day.