On Thu, Mar 28, 2019 at 10:59 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>
> On 3/28/19 8:20 AM, Tomasz Figa wrote:
>
> <snip>
>
> >> +static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
> >> +{
> >> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> >> +
> >> + vbuf->field = V4L2_FIELD_NONE;
> >
> > Hmm, "validate" in the name of this callback would suggest that we
> > should just check the contents, not change them. Hans, what was the
> > intention when adding this callback? Are we missing the const
> > specifier in the argument?
>
> See the original commit log:
>
> https://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg143163.html
>
> It is allowed to either just validate and return an error if wrong,
> or change it to something known to be valid. In particular, userspace
> can set this to FIELD_ANY, and in that case the driver must replace
> it with something valid.
>
> Most drivers just support FIELD_NONE, and just set it.
>
> That said, I think we should tighten the spec for this as this is
> not well documented.
>
> I propose that if vbuf->field == FIELD_ANY, then replace it with something
> sane. Otherwise validate it and return an error if the field value
> is not supported.
>
> And FIELD_ALTERNATE is never allowed here, that's always wrong.
Thanks for clarifying. Makes sense.
Best regards,
Tomasz
