2013-07-16 23:07:01

by Alban Browaeys

Subject: [PATCH 4/4] [media] em28xx: Fix vidioc fmt vid cap v4l2 compliance

Set fmt.pix.priv to zero in vidioc_g_fmt_vid_cap
and vidioc_try_fmt_vid_cap.

Catched by v4l2-compliance.

Signed-off-by: Alban Browaeys <[email protected]>
---
drivers/media/usb/em28xx/em28xx-video.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 1a577ed..42930a4 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -943,6 +943,8 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
else
f->fmt.pix.field = dev->interlaced ?
V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP;
+ f->fmt.pix.priv = 0;
+
return 0;
}

@@ -1008,6 +1010,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
else
f->fmt.pix.field = dev->interlaced ?
V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP;
+ f->fmt.pix.priv = 0;

return 0;
}
--
1.8.3.2

2013-07-18 02:07:56

by Devin Heitmueller

Subject: Re: [PATCH 4/4] [media] em28xx: Fix vidioc fmt vid cap v4l2 compliance

On Tue, Jul 16, 2013 at 7:06 PM, Alban Browaeys
<[email protected]> wrote:
> Set fmt.pix.priv to zero in vidioc_g_fmt_vid_cap
> and vidioc_try_fmt_vid_cap.

Any reason not to have the v4l2 core do this before dispatching to the
driver? Set it to zero before the core calls g_fmt. This avoids all
the drivers (most of which don't use the field) from having to set the
value themselves.

Devin

--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

2013-07-18 08:05:48

by Hans Verkuil

Subject: Re: [PATCH 4/4] [media] em28xx: Fix vidioc fmt vid cap v4l2 compliance

On Thu 18 July 2013 04:07:51 Devin Heitmueller wrote:
> On Tue, Jul 16, 2013 at 7:06 PM, Alban Browaeys
> <[email protected]> wrote:
> > Set fmt.pix.priv to zero in vidioc_g_fmt_vid_cap
> > and vidioc_try_fmt_vid_cap.
>
> Any reason not to have the v4l2 core do this before dispatching to the
> driver? Set it to zero before the core calls g_fmt. This avoids all
> the drivers (most of which don't use the field) from having to set the
> value themselves.

There is still one driver (sn9c102) that's (ab)using it. Although perhaps I
should take a look at it and fix it.

Note that priv only needs to be cleared for try/s_fmt. g_fmt does clear it
already in the core before handing it over to the driver.

That said, I am undecided whether to put this in the core. We might actually
start to use this field for something useful in the future. By having drivers
clear it explicitly it will be easier to do that.

Regards,

Hans

2013-07-26 13:25:24

by Hans Verkuil

Subject: Re: [PATCH 4/4] [media] em28xx: Fix vidioc fmt vid cap v4l2 compliance

The change to g_fmt_vid_cap isn't necessary as that's automatically cleared.
Only the s_fmt_vid_cap change is needed. I'll drop the first chunk and accept the
second.

Thanks,

Hans

On 07/17/2013 01:06 AM, Alban Browaeys wrote:
> Set fmt.pix.priv to zero in vidioc_g_fmt_vid_cap
> and vidioc_try_fmt_vid_cap.
>
> Catched by v4l2-compliance.
>
> Signed-off-by: Alban Browaeys <[email protected]>
> ---
> drivers/media/usb/em28xx/em28xx-video.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index 1a577ed..42930a4 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -943,6 +943,8 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
> else
> f->fmt.pix.field = dev->interlaced ?
> V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP;
> + f->fmt.pix.priv = 0;
> +
> return 0;
> }
>
> @@ -1008,6 +1010,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
> else
> f->fmt.pix.field = dev->interlaced ?
> V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP;
> + f->fmt.pix.priv = 0;
>
> return 0;
> }
>