On Tuesday, February 22, 2011 03:52:11 Mauro Carvalho Chehab wrote: > Em 21-02-2011 23:17, Mauro Carvalho Chehab escreveu: > > This series contain a minor cleanup for tuner and tvp5150, and two fixes > > for em28xx controls. Before the em28xx patches, s_ctrl were failing on > > qv4l2, because it were returning a positive value of 1 for some calls. > > > > It also contains a fix for control get/set, as it will now check if the > > control exists before actually calling subdev for get/set. > > > > Mauro Carvalho Chehab (4): > ... > > [media] em28xx: Fix return value for s_ctrl > > [media] em28xx: properly handle subdev controls > > Hans, > > I discovered the issue with em28xx that I commented you on IRC. > > There were, in fact, 3 issues. > > One is clearly a driver problem, corrected by "em28xx: properly > handle subdev controls". > > The second one being partially qv4l2 and partially driver issue, > fixed by "em28xx: Fix return value for s_ctrl". Basically, V4L2 > API and ioctl man page says that an error is indicated by -1 value, > being 0 or positive value a non-error. Well, qv4l2 understands a > positive value as -EBUSY. The driver were returning a non-standard > value of 1 for s_ctrl. I fixed the driver part. > > The last issue is with v4l2-ctl and qv4l2. Also, the latest version > of xawtv had the same issue, probably due to some changes I did at > console/v4l-info.c. > > What happens is that em28xx doesn't implement the control BASE+4, > due to one simple reason: it is not currently defined. The ctrl > loop were understanding the -EINVAL return of BASE+4 as the end of > the user controls. So, on xawtv, only the 3 image controls were > returned. I didn't dig into v4l2-ctl, but there, it doesn't show > the first 3 controls. It shows only the audio controls: > > volume (int) : min=0 max=65535 step=655 default=58880 value=58880 flags=slider > balance (int) : min=0 max=65535 step=655 default=32768 value=32768 flags=slider > bass (int) : min=0 max=65535 step=655 default=32768 value=32768 flags=slider > treble (int) : min=0 max=65535 step=655 default=32768 value=32768 flags=slider > mute (bool) : default=0 value=0 > loudness (bool) : default=0 value=0 > > The xawtv fix is at: > http://git.linuxtv.org/xawtv3.git?a=commitdiff;h=fda070af9cfd75b360db1339bde3c6d3c64ed627 > > A similar fix is needed for v4l2-ctl and qv4l2. Actually, v4l2-ctrl and qv4l2 handle 'holes' correctly. I think this is a different bug relating to the handling of V4L2_CTRL_FLAG_NEXT_CTRL. Can you try this patch: diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c index ef66d2a..15eda86 100644 --- a/drivers/media/video/v4l2-ctrls.c +++ b/drivers/media/video/v4l2-ctrls.c @@ -1364,6 +1364,8 @@ EXPORT_SYMBOL(v4l2_queryctrl); int v4l2_subdev_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl *qc) { + if (qc->id & V4L2_CTRL_FLAG_NEXT_CTRL) + return -EINVAL; return v4l2_queryctrl(sd->ctrl_handler, qc); } EXPORT_SYMBOL(v4l2_subdev_queryctrl); v4l2-ctl and qv4l2 enumerate the controls using this flag, falling back to the old method if the flag isn't supported. The v4l2_subdev_queryctrl function will currently handle that flag, but for the controls of the subdev only. This isn't right, it should refuse this flag. Without this fix v4l2-ctl will only see the controls of the first subdev, which is exactly what you got. I never saw this bug because the HVR900 has just a single subdev. I also suspect that s_ctrl is wrong: can you test setting a video control? I think that v4l2_device_call_until_err will always return an error. I'm not sure if there is an easy fix for this other than converting em28xx to the control framework. I need to think about this. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by Cisco -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
