Re: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM
On Sun, 29 Mar 2009, Hans Verkuil wrote: > On Sunday 29 March 2009 13:03:13 Trent Piepho wrote: > > How does overlay depend on video capture in any way? It's perfectly > > reasonable for a driver to support _only_ overlay and not video capture. > > The zr36067 chip is only designed to support uncompressed data for video > > overlay for example. Allowing uncompressed video capture is a hack that > > the driver didn't have at one point. > > Ah. Live and learn. In that case the spec is out of date and needs to be > updated. Do you mean there is something in the spec that makes overlay depend on video capture? Or that s/g_parm don't mention buffer types other than video capture and video output? > > > No, I agree with the spec in that I see no use case for it. Should > > > there be one, then I'd like to see that in an actual driver > > > implementation and in that case the spec should be adjusted as well. > > > > How about getting the frame rate of video overlay? Works with bttv. > > Hmm, I grepped only on s_parm, not on g_parm. It would be nice to use s_parm with drivers like bttv to reduce the frame rate. With multi-chip capture cards running out of bus bandwidth is a big problem. Reducing the frame rate is one way to deal with it. The bt8x8 and cx2388x chips do have a temporal decimation feature and I've tried to add support for it but I never got it to work correctly. > > > In addition, g/s_parm is only used in combination with webcams/sensors > > > for which overlays and vbi are irrelevant. > > > > There are several drivers for non-webcams, like bttv, saa7134, and > > saa7146, that support g_parm. Why is returning the frame rate for video > > capture not valid? Why does the number of buffers used for read() mode > > only make sense for webcams? > > OK, I'd forgot to check for the usage of g_parm. My bad. There is also a default g_parm handler in video_ioctl2 that might be used if the driver doesn't provide one. I don't know what drivers actually use it. > > > > Thinking about it now, I think what makes the most sense is to use > > > > "capture" for VIDEO_OVERLAY, VBI_CAPTURE, and SLICED_VBI_CAPTURE. > > > > And use "output" for VBI_OUTPUT, SLICED_VBI_OUTPUT and > > > > VIDEO_OUTPUT_OVERLAY. > > You're absolutely correct. I was too hasty. > > Can you update the spec to reflect this? I'll try, but I hear the doc build system is quite a challenge. > > > > > I also wonder whether check_fmt shouldn't check for the presence of > > > > > the s_fmt callbacks instead of try_fmt since try_fmt is an optional > > > > > ioctl. > > > > > > > > I noticed that too. saa7146 doesn't have a try_fmt call for > > > > vbi_capture but is apparently supposed to support it. I sent a > > > > message about that earlier. > > > > > > I saw that. So why not check for s_fmt instead of try_fmt? That would > > > solve this potential problem. > > > > Because that's clearly a change that belongs in another patch. > > OK, great. My patch just called check_fmt() for s/g_parm. I haven't touched check_fmt(). But you're right that try_fmt is optional so it makes a bad test. Maybe it should use g_fmt? saa7146 doesn't provide a s_fmt call either for vbi capture, only g_fmt. Though maybe this is a bug in saa7146? It seems like any driver that provides g_fmt can always just use that same method for s_fmt as well and be correct. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM
On Sunday 29 March 2009 13:03:13 Trent Piepho wrote: > On Sun, 29 Mar 2009, Hans Verkuil wrote: > > On Sunday 29 March 2009 12:21:58 Trent Piepho wrote: > > > On Sun, 29 Mar 2009, Hans Verkuil wrote: > > > > On Sunday 29 March 2009 10:50:02 Patch from Trent Piepho wrote: > > > > > From: Trent Piepho > > > > > v4l2-ioctl: Check format for S_PARM and G_PARM > > > > > > > > > > > > > > > Return EINVAL if VIDIOC_S/G_PARM is called for a buffer type that > > > > > the driver doesn't define a ->vidioc_try_fmt_XXX() method for. > > > > > Several other ioctls, like QUERYBUF, QBUF, and DQBUF, etc. do > > > > > this too. It saves each driver from having to check if the > > > > > buffer type is one that it supports. > > > > > > > > Hi Trent, > > > > > > > > I wonder whether this change is correct. Looking at the spec I see > > > > that g/s_parm only supports VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE > > > > or up. > > > > > > > > So what should happen if the type is VIDEO_OVERLAY? I think the > > > > g/s_parm implementation in v4l2-ioctl.c should first exclude the > > > > unsupported types before calling check_fmt. > > > > > > This change doesn't actually enable g/s_parm for VIDEO_OVERLAY (or > > > VBI_CAPTURE). It's the later bttv and saa7146 changes that do that. > > > I considered this when I made those changes, as mentioned in those > > > patch descriptions, but decided it was better to allow it. > > > > > > In those drivers g_parm only returns the frame rate, which seems just > > > as valid for VIDEO_OVERLAY as it does for VIDEO_CAPTURE. Why should > > > the driver not be allowed to return the frame rate for video overlay? > > > Why should setting the overlay frame rate not be allowed? Those > > > seems like perfectly reasonable operations to me. > > > > Not to me. VIDEO_OVERLAY just defines where the overlay is. But the > > actual framerate is entirely dependent on the VIDEO_CAPTURE framerate. > > Just keep to the spec for now. If a new driver appears that needs it > > then we can always change it. > > How does overlay depend on video capture in any way? It's perfectly > reasonable for a driver to support _only_ overlay and not video capture. > The zr36067 chip is only designed to support uncompressed data for video > overlay for example. Allowing uncompressed video capture is a hack that > the driver didn't have at one point. Ah. Live and learn. In that case the spec is out of date and needs to be updated. > > > The spec doesn't explicitly say that only VIDEO_CAPTURE, VIDEO_OUTPUT > > > and PRIVATE are supported. It says the "capture" field of the parm > > > union is used when type is VIDEO_CAPTURE, the "output" field is used > > > for VIDEO_OUTPUT, and "raw_data" is used for PRIVATE or higher. > > > You're right in that it doesn't say what you're supposed to use for > > > VIDEO_OVERLAY, VBI_CAPTURE or any other buffer types. But it doesn't > > > say they're not allowed either. IMHO, it's likely the spec authors' > > > intent wasn't to not allow g_parm with VIDEO_OVERLAY, but rather that > > > they just didn't think of that case. > > > > No, I agree with the spec in that I see no use case for it. Should > > there be one, then I'd like to see that in an actual driver > > implementation and in that case the spec should be adjusted as well. > > How about getting the frame rate of video overlay? Works with bttv. Hmm, I grepped only on s_parm, not on g_parm. > > In addition, g/s_parm is only used in combination with webcams/sensors > > for which overlays and vbi are irrelevant. > > There are several drivers for non-webcams, like bttv, saa7134, and > saa7146, that support g_parm. Why is returning the frame rate for video > capture not valid? Why does the number of buffers used for read() mode > only make sense for webcams? OK, I'd forgot to check for the usage of g_parm. My bad. > > > Thinking about it now, I think what makes the most sense is to use > > > "capture" for VIDEO_OVERLAY, VBI_CAPTURE, and SLICED_VBI_CAPTURE. > > > And use "output" for VBI_OUTPUT, SLICED_VBI_OUTPUT and > > > VIDEO_OUTPUT_OVERLAY. You're absolutely correct. I was too hasty. Can you update the spec to reflect this? > > > > I also wonder whether check_fmt shouldn't check for the presence of > > > > the s_fmt callbacks instead of try_fmt since try_fmt is an optional > > > > ioctl. > > > > > > I noticed that too. saa7146 doesn't have a try_fmt call for > > > vbi_capture but is apparently supposed to support it. I sent a > > > message about that earlier. > > > > I saw that. So why not check for s_fmt instead of try_fmt? That would > > solve this potential problem. > > Because that's clearly a change that belongs in another patch. OK, great. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-i
Re: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM
On Sun, 29 Mar 2009, Hans Verkuil wrote: > On Sunday 29 March 2009 12:21:58 Trent Piepho wrote: > > On Sun, 29 Mar 2009, Hans Verkuil wrote: > > > On Sunday 29 March 2009 10:50:02 Patch from Trent Piepho wrote: > > > > From: Trent Piepho > > > > v4l2-ioctl: Check format for S_PARM and G_PARM > > > > > > > > > > > > Return EINVAL if VIDIOC_S/G_PARM is called for a buffer type that the > > > > driver doesn't define a ->vidioc_try_fmt_XXX() method for. Several > > > > other ioctls, like QUERYBUF, QBUF, and DQBUF, etc. do this too. It > > > > saves each driver from having to check if the buffer type is one that > > > > it supports. > > > > > > Hi Trent, > > > > > > I wonder whether this change is correct. Looking at the spec I see that > > > g/s_parm only supports VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE or up. > > > > > > So what should happen if the type is VIDEO_OVERLAY? I think the > > > g/s_parm implementation in v4l2-ioctl.c should first exclude the > > > unsupported types before calling check_fmt. > > > > This change doesn't actually enable g/s_parm for VIDEO_OVERLAY (or > > VBI_CAPTURE). It's the later bttv and saa7146 changes that do that. I > > considered this when I made those changes, as mentioned in those patch > > descriptions, but decided it was better to allow it. > > > > In those drivers g_parm only returns the frame rate, which seems just as > > valid for VIDEO_OVERLAY as it does for VIDEO_CAPTURE. Why should the > > driver not be allowed to return the frame rate for video overlay? Why > > should setting the overlay frame rate not be allowed? Those seems like > > perfectly reasonable operations to me. > > Not to me. VIDEO_OVERLAY just defines where the overlay is. But the actual > framerate is entirely dependent on the VIDEO_CAPTURE framerate. Just keep > to the spec for now. If a new driver appears that needs it then we can > always change it. How does overlay depend on video capture in any way? It's perfectly reasonable for a driver to support _only_ overlay and not video capture. The zr36067 chip is only designed to support uncompressed data for video overlay for example. Allowing uncompressed video capture is a hack that the driver didn't have at one point. > > The spec doesn't explicitly say that only VIDEO_CAPTURE, VIDEO_OUTPUT and > > PRIVATE are supported. It says the "capture" field of the parm union is > > used when type is VIDEO_CAPTURE, the "output" field is used for > > VIDEO_OUTPUT, and "raw_data" is used for PRIVATE or higher. You're right > > in that it doesn't say what you're supposed to use for VIDEO_OVERLAY, > > VBI_CAPTURE or any other buffer types. But it doesn't say they're not > > allowed either. IMHO, it's likely the spec authors' intent wasn't to not > > allow g_parm with VIDEO_OVERLAY, but rather that they just didn't think > > of that case. > > No, I agree with the spec in that I see no use case for it. Should there be > one, then I'd like to see that in an actual driver implementation and in > that case the spec should be adjusted as well. How about getting the frame rate of video overlay? Works with bttv. > In addition, g/s_parm is only used in combination with webcams/sensors for > which overlays and vbi are irrelevant. There are several drivers for non-webcams, like bttv, saa7134, and saa7146, that support g_parm. Why is returning the frame rate for video capture not valid? Why does the number of buffers used for read() mode only make sense for webcams? > > Thinking about it now, I think what makes the most sense is to use > > "capture" for VIDEO_OVERLAY, VBI_CAPTURE, and SLICED_VBI_CAPTURE. And > > use "output" for VBI_OUTPUT, SLICED_VBI_OUTPUT and VIDEO_OUTPUT_OVERLAY. > > > > > I also wonder whether check_fmt shouldn't check for the presence of the > > > s_fmt callbacks instead of try_fmt since try_fmt is an optional ioctl. > > > > I noticed that too. saa7146 doesn't have a try_fmt call for vbi_capture > > but is apparently supposed to support it. I sent a message about that > > earlier. > > I saw that. So why not check for s_fmt instead of try_fmt? That would solve > this potential problem. Because that's clearly a change that belongs in another patch. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM
On Sunday 29 March 2009 12:21:58 Trent Piepho wrote: > On Sun, 29 Mar 2009, Hans Verkuil wrote: > > On Sunday 29 March 2009 10:50:02 Patch from Trent Piepho wrote: > > > From: Trent Piepho > > > v4l2-ioctl: Check format for S_PARM and G_PARM > > > > > > > > > Return EINVAL if VIDIOC_S/G_PARM is called for a buffer type that the > > > driver doesn't define a ->vidioc_try_fmt_XXX() method for. Several > > > other ioctls, like QUERYBUF, QBUF, and DQBUF, etc. do this too. It > > > saves each driver from having to check if the buffer type is one that > > > it supports. > > > > Hi Trent, > > > > I wonder whether this change is correct. Looking at the spec I see that > > g/s_parm only supports VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE or up. > > > > So what should happen if the type is VIDEO_OVERLAY? I think the > > g/s_parm implementation in v4l2-ioctl.c should first exclude the > > unsupported types before calling check_fmt. > > This change doesn't actually enable g/s_parm for VIDEO_OVERLAY (or > VBI_CAPTURE). It's the later bttv and saa7146 changes that do that. I > considered this when I made those changes, as mentioned in those patch > descriptions, but decided it was better to allow it. > > In those drivers g_parm only returns the frame rate, which seems just as > valid for VIDEO_OVERLAY as it does for VIDEO_CAPTURE. Why should the > driver not be allowed to return the frame rate for video overlay? Why > should setting the overlay frame rate not be allowed? Those seems like > perfectly reasonable operations to me. Not to me. VIDEO_OVERLAY just defines where the overlay is. But the actual framerate is entirely dependent on the VIDEO_CAPTURE framerate. Just keep to the spec for now. If a new driver appears that needs it then we can always change it. > > The spec doesn't explicitly say that only VIDEO_CAPTURE, VIDEO_OUTPUT and > PRIVATE are supported. It says the "capture" field of the parm union is > used when type is VIDEO_CAPTURE, the "output" field is used for > VIDEO_OUTPUT, and "raw_data" is used for PRIVATE or higher. You're right > in that it doesn't say what you're supposed to use for VIDEO_OVERLAY, > VBI_CAPTURE or any other buffer types. But it doesn't say they're not > allowed either. IMHO, it's likely the spec authors' intent wasn't to not > allow g_parm with VIDEO_OVERLAY, but rather that they just didn't think > of that case. No, I agree with the spec in that I see no use case for it. Should there be one, then I'd like to see that in an actual driver implementation and in that case the spec should be adjusted as well. In addition, g/s_parm is only used in combination with webcams/sensors for which overlays and vbi are irrelevant. > > Thinking about it now, I think what makes the most sense is to use > "capture" for VIDEO_OVERLAY, VBI_CAPTURE, and SLICED_VBI_CAPTURE. And > use "output" for VBI_OUTPUT, SLICED_VBI_OUTPUT and VIDEO_OUTPUT_OVERLAY. > > > I also wonder whether check_fmt shouldn't check for the presence of the > > s_fmt callbacks instead of try_fmt since try_fmt is an optional ioctl. > > I noticed that too. saa7146 doesn't have a try_fmt call for vbi_capture > but is apparently supposed to support it. I sent a message about that > earlier. I saw that. So why not check for s_fmt instead of try_fmt? That would solve this potential problem. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM
On Sun, 29 Mar 2009, Hans Verkuil wrote: > On Sunday 29 March 2009 10:50:02 Patch from Trent Piepho wrote: > > From: Trent Piepho > > v4l2-ioctl: Check format for S_PARM and G_PARM > > > > > > Return EINVAL if VIDIOC_S/G_PARM is called for a buffer type that the > > driver doesn't define a ->vidioc_try_fmt_XXX() method for. Several other > > ioctls, like QUERYBUF, QBUF, and DQBUF, etc. do this too. It saves each > > driver from having to check if the buffer type is one that it supports. > > Hi Trent, > > I wonder whether this change is correct. Looking at the spec I see that > g/s_parm only supports VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE or up. > > So what should happen if the type is VIDEO_OVERLAY? I think the g/s_parm > implementation in v4l2-ioctl.c should first exclude the unsupported types > before calling check_fmt. This change doesn't actually enable g/s_parm for VIDEO_OVERLAY (or VBI_CAPTURE). It's the later bttv and saa7146 changes that do that. I considered this when I made those changes, as mentioned in those patch descriptions, but decided it was better to allow it. In those drivers g_parm only returns the frame rate, which seems just as valid for VIDEO_OVERLAY as it does for VIDEO_CAPTURE. Why should the driver not be allowed to return the frame rate for video overlay? Why should setting the overlay frame rate not be allowed? Those seems like perfectly reasonable operations to me. The spec doesn't explicitly say that only VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE are supported. It says the "capture" field of the parm union is used when type is VIDEO_CAPTURE, the "output" field is used for VIDEO_OUTPUT, and "raw_data" is used for PRIVATE or higher. You're right in that it doesn't say what you're supposed to use for VIDEO_OVERLAY, VBI_CAPTURE or any other buffer types. But it doesn't say they're not allowed either. IMHO, it's likely the spec authors' intent wasn't to not allow g_parm with VIDEO_OVERLAY, but rather that they just didn't think of that case. Thinking about it now, I think what makes the most sense is to use "capture" for VIDEO_OVERLAY, VBI_CAPTURE, and SLICED_VBI_CAPTURE. And use "output" for VBI_OUTPUT, SLICED_VBI_OUTPUT and VIDEO_OUTPUT_OVERLAY. > I also wonder whether check_fmt shouldn't check for the presence of the > s_fmt callbacks instead of try_fmt since try_fmt is an optional ioctl. I noticed that too. saa7146 doesn't have a try_fmt call for vbi_capture but is apparently supposed to support it. I sent a message about that earlier. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM
On Sunday 29 March 2009 11:35:45 Mauro Carvalho Chehab wrote: > On Sun, 29 Mar 2009 11:06:19 +0200 > > Hans Verkuil wrote: > > On Sunday 29 March 2009 10:50:02 Patch from Trent Piepho wrote: > > > The patch number 11260 was added via Trent Piepho > > > to http://linuxtv.org/hg/v4l-dvb master > > > development tree. > > > > > > Kernel patches in this development tree may be modified to be > > > backward compatible with older kernels. Compatibility modifications > > > will be removed before inclusion into the mainstream Kernel > > > > > > If anyone has any objections, please let us know by sending a message > > > to: Linux Media Mailing List > > > > > > -- > > > > > > From: Trent Piepho > > > v4l2-ioctl: Check format for S_PARM and G_PARM > > > > > > > > > Return EINVAL if VIDIOC_S/G_PARM is called for a buffer type that the > > > driver doesn't define a ->vidioc_try_fmt_XXX() method for. Several > > > other ioctls, like QUERYBUF, QBUF, and DQBUF, etc. do this too. It > > > saves each driver from having to check if the buffer type is one that > > > it supports. > > > > Hi Trent, > > > > I wonder whether this change is correct. Looking at the spec I see that > > g/s_parm only supports VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE or up. > > > > So what should happen if the type is VIDEO_OVERLAY? I think the > > g/s_parm implementation in v4l2-ioctl.c should first exclude the > > unsupported types before calling check_fmt. > > Makes sense to me. > > > I also wonder whether check_fmt shouldn't check for the presence of the > > s_fmt callbacks instead of try_fmt since try_fmt is an optional ioctl. > > One developer suggested to merge try_fmt and s_fmt into one callback. > IMO, this makes sense, since I have the feeling that this will simplify > the code a little bit on the drivers. If we go this way, then we can > check for the new try_s_fmt callback. I agree with this. It's also easy to gradually migrate to such a new callback since it is probably quite difficult to do this in one big bang patch. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM
On Sun, 29 Mar 2009 11:06:19 +0200 Hans Verkuil wrote: > On Sunday 29 March 2009 10:50:02 Patch from Trent Piepho wrote: > > The patch number 11260 was added via Trent Piepho > > to http://linuxtv.org/hg/v4l-dvb master development tree. > > > > Kernel patches in this development tree may be modified to be backward > > compatible with older kernels. Compatibility modifications will be > > removed before inclusion into the mainstream Kernel > > > > If anyone has any objections, please let us know by sending a message to: > > Linux Media Mailing List > > > > -- > > > > From: Trent Piepho > > v4l2-ioctl: Check format for S_PARM and G_PARM > > > > > > Return EINVAL if VIDIOC_S/G_PARM is called for a buffer type that the > > driver doesn't define a ->vidioc_try_fmt_XXX() method for. Several other > > ioctls, like QUERYBUF, QBUF, and DQBUF, etc. do this too. It saves each > > driver from having to check if the buffer type is one that it supports. > > Hi Trent, > > I wonder whether this change is correct. Looking at the spec I see that > g/s_parm only supports VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE or up. > > So what should happen if the type is VIDEO_OVERLAY? I think the g/s_parm > implementation in v4l2-ioctl.c should first exclude the unsupported types > before calling check_fmt. Makes sense to me. > I also wonder whether check_fmt shouldn't check for the presence of the > s_fmt callbacks instead of try_fmt since try_fmt is an optional ioctl. One developer suggested to merge try_fmt and s_fmt into one callback. IMO, this makes sense, since I have the feeling that this will simplify the code a little bit on the drivers. If we go this way, then we can check for the new try_s_fmt callback. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linuxtv-commits] [hg:v4l-dvb] v4l2-ioctl: Check format for S_PARM and G_PARM
On Sunday 29 March 2009 10:50:02 Patch from Trent Piepho wrote:
> The patch number 11260 was added via Trent Piepho
> to http://linuxtv.org/hg/v4l-dvb master development tree.
>
> Kernel patches in this development tree may be modified to be backward
> compatible with older kernels. Compatibility modifications will be
> removed before inclusion into the mainstream Kernel
>
> If anyone has any objections, please let us know by sending a message to:
> Linux Media Mailing List
>
> --
>
> From: Trent Piepho
> v4l2-ioctl: Check format for S_PARM and G_PARM
>
>
> Return EINVAL if VIDIOC_S/G_PARM is called for a buffer type that the
> driver doesn't define a ->vidioc_try_fmt_XXX() method for. Several other
> ioctls, like QUERYBUF, QBUF, and DQBUF, etc. do this too. It saves each
> driver from having to check if the buffer type is one that it supports.
Hi Trent,
I wonder whether this change is correct. Looking at the spec I see that
g/s_parm only supports VIDEO_CAPTURE, VIDEO_OUTPUT and PRIVATE or up.
So what should happen if the type is VIDEO_OVERLAY? I think the g/s_parm
implementation in v4l2-ioctl.c should first exclude the unsupported types
before calling check_fmt.
I also wonder whether check_fmt shouldn't check for the presence of the
s_fmt callbacks instead of try_fmt since try_fmt is an optional ioctl.
Regards,
Hans
>
> Priority: normal
>
> Signed-off-by: Trent Piepho
>
>
> ---
>
> linux/drivers/media/video/v4l2-ioctl.c |7 +++
> 1 file changed, 7 insertions(+)
>
> diff -r 346bab8698ea -r 44632c5fe5b2
> linux/drivers/media/video/v4l2-ioctl.c ---
> a/linux/drivers/media/video/v4l2-ioctl.c Sat Mar 28 18:25:35 2009 -0700
> +++ b/linux/drivers/media/video/v4l2-ioctl.c Sat Mar 28 18:25:35 2009
> -0700 @@ -1552,6 +1552,9 @@ static long __video_do_ioctl(struct file
> struct v4l2_streamparm *p = arg;
>
> if (ops->vidioc_g_parm) {
> + ret = check_fmt(ops, p->type);
> + if (ret)
> + break;
> ret = ops->vidioc_g_parm(file, fh, p);
> } else {
> if (p->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> @@ -1571,6 +1574,10 @@ static long __video_do_ioctl(struct file
>
> if (!ops->vidioc_s_parm)
> break;
> + ret = check_fmt(ops, p->type);
> + if (ret)
> + break;
> +
> dbgarg(cmd, "type=%d\n", p->type);
> ret = ops->vidioc_s_parm(file, fh, p);
> break;
>
>
> ---
>
> Patch is available at:
> http://linuxtv.org/hg/v4l-dvb/rev/44632c5fe5b2cd973c75501a88d61b43a39f6c4
>3
>
> ___
> linuxtv-commits mailing list
> linuxtv-comm...@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/linuxtv-commits
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
