linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Tomasz Figa <tfiga@chromium.org>,
	Ezequiel Garcia <ezequiel@collabora.com>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	kernel@collabora.com,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	Heiko Stuebner <heiko@sntech.de>, Jonas Karlman <jonas@kwiboo.se>
Subject: Re: [PATCH v2 08/11] rockchip/vpu: Support the Request API
Date: Thu, 28 Mar 2019 14:59:00 +0100	[thread overview]
Message-ID: <0618322f-ac2b-fbcd-47f2-68173318f8d7@xs4all.nl> (raw)
In-Reply-To: <CAAFQd5C5zo=03-mLh4OztKS+dCjVMUe1Za25ff+dWjxQDLFLng@mail.gmail.com>

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@vger.kernel.org/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

  reply	other threads:[~2019-03-28 13:59 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-04 19:25 [PATCH v2 00/11] Add MPEG-2 decoding to Rockchip VPU Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 01/11] rockchip/vpu: Rename pixel format helpers Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 02/11] media: Introduce helpers to fill pixel format structs Ezequiel Garcia
2019-03-12  8:29   ` Hans Verkuil
2019-03-22 17:29     ` Ezequiel Garcia
2019-03-25 14:32   ` Emil Velikov
2019-03-04 19:25 ` [PATCH v2 03/11] rockchip/vpu: Use pixel format helpers Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 04/11] rockchip/vpu: Use v4l2_m2m_buf_copy_metadata Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 05/11] rockchip/vpu: Cleanup macroblock alignment Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 06/11] rockchip/vpu: Cleanup JPEG bounce buffer management Ezequiel Garcia
2019-03-28  6:15   ` Tomasz Figa
2019-03-28 18:30     ` Ezequiel Garcia
2019-03-29  3:21       ` Tomasz Figa
2019-03-04 19:25 ` [PATCH v2 07/11] rockchip/vpu: Open-code media controller register Ezequiel Garcia
2019-03-28  7:11   ` Tomasz Figa
2019-03-28 20:05     ` Ezequiel Garcia
2019-03-29  7:43       ` Tomasz Figa
2019-03-04 19:25 ` [PATCH v2 08/11] rockchip/vpu: Support the Request API Ezequiel Garcia
2019-03-28  7:20   ` Tomasz Figa
2019-03-28 13:59     ` Hans Verkuil [this message]
2019-03-29  3:23       ` Tomasz Figa
2019-03-28 19:07     ` Ezequiel Garcia
2019-03-04 19:25 ` [PATCH v2 09/11] rockchip/vpu: Add decoder boilerplate Ezequiel Garcia
2019-03-28  9:57   ` Tomasz Figa
2019-03-28 19:23     ` Ezequiel Garcia
2019-03-29  7:40       ` Tomasz Figa
2019-03-04 19:25 ` [PATCH v2 10/11] rockchip/vpu: Add support for non-standard controls Ezequiel Garcia
2019-04-01  3:14   ` Tomasz Figa
2019-04-12 19:25     ` Ezequiel Garcia
2019-04-15  4:07       ` Tomasz Figa
2019-03-04 19:25 ` [PATCH v2 11/11] rockchip/vpu: Add support for MPEG-2 decoding Ezequiel Garcia
2019-04-01  3:52   ` Tomasz Figa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0618322f-ac2b-fbcd-47f2-68173318f8d7@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=ezequiel@collabora.com \
    --cc=hans.verkuil@cisco.com \
    --cc=heiko@sntech.de \
    --cc=jonas@kwiboo.se \
    --cc=kernel@collabora.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=nicolas.dufresne@collabora.com \
    --cc=tfiga@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).