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.
Regards,
Hans
