linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nicolas Dufresne <nicolas@ndufresne.ca>
To: Hans Verkuil <hverkuil@xs4all.nl>,
	Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
	linux-media@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org, devel@driverdev.osuosl.org,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Ezequiel Garcia <ezequiel@collabora.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	kernel@collabora.com,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Adrian Ratiu <adrian.ratiu@collabora.com>
Subject: Re: [RFC RESEND 2/3] media: uapi: Add VP9 stateless decoder controls
Date: Wed, 05 May 2021 13:36:06 -0400	[thread overview]
Message-ID: <7fc04abf3959720a9f344bc3a67b4792552811c1.camel@ndufresne.ca> (raw)
In-Reply-To: <12493c55-e6cf-0491-d310-7561c7f77258@xs4all.nl>

Hi Hans,

just a partial reply, I'll let Andrzej extend.

Le jeudi 29 avril 2021 à 12:20 +0200, Hans Verkuil a écrit :
> > +      - ``frame_width_minus_1``
> > +      - Add 1 to get the frame width expressed in pixels.
> > +    * - __u16
> > +      - ``frame_height_minus_1``
> > +      - Add 1 to get the frame height expressed in pixels.
> 
> These two fields are weird. Isn't this defined by setting the output format?
> And why the 'minus_1'?
> 
> > +    * - __u16
> > +      - ``render_width_minus_1``
> > +      - Add 1 to get the expected render width expressed in pixels. This is
> > +        not used during the decoding process but might be used by HW
> > scalers to
> > +        prepare a frame that's ready for scanout.
> > +    * - __u16
> > +      - render_height_minus_1
> > +      - Add 1 to get the expected render height expressed in pixels. This
> > is
> > +        not used during the decoding process but might be used by HW
> > scalers to
> > +        prepare a frame that's ready for scanout.
> 
> No idea what these fields are about. I suspect this can be defined by setting
> the capture format, but I'm not sure.

We have the same for other codecs. Each codec bitstream include the coded and
the display size in some form. For H264/H265 that was passed as as number of
macroblock and a crop rectangle. For VP9 value - 1 is as defined in the spec. As
0 is not valid, the boolean coder can save some bits when storing the value.
Though, for parameters, we usually start with the way it's encoded first, and
change it only if we think it make sense. We'll take note and consider this
whenever we have a second driver to look at.

Now, why do we include both here. Well in fact, the HW may have some extra
constraints. Which means there will be up to 3 frame sizes that matters. We
can't also determine if the display/render or coded size will be used for minim
CAPTURE format, as this will in fact depends if a post processor will be used or
not. 

So to recap, we limit the CAPTURE format base on this information, and not the
opposite. The width/height on OUTPUT FMT has been define as meaningless in the
spec (to align with stateful I suppose ?). This way, the driver got all the
information aligned with how it works inside the codec, without having to do a
translation dance, and then properly implement CAPTURE TRY_FMT base on that.

To make an analogy with stateful codec, this replaces the queuing of a frame
that contains codec headers. We skip the SRC_CH events, since this is no longer
asynchronous.

Nicolas


  parent reply	other threads:[~2021-05-05 18:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21 10:00 [RFC RESEND 0/3] vp9 v4l2 stateless uapi Andrzej Pietrasiewicz
2021-04-21 10:00 ` [RFC RESEND 1/3] media: rkvdec: Fix .buf_prepare Andrzej Pietrasiewicz
2021-04-21 10:00 ` [RFC RESEND 2/3] media: uapi: Add VP9 stateless decoder controls Andrzej Pietrasiewicz
2021-04-29 10:20   ` Hans Verkuil
2021-04-30 16:54     ` Andrzej Pietrasiewicz
2021-05-05 17:36     ` Nicolas Dufresne [this message]
2021-04-21 10:00 ` [RFC RESEND 3/3] media: rkvdec: Add the VP9 backend Andrzej Pietrasiewicz
2021-04-29 10:28   ` Hans Verkuil
2021-04-26  7:38 ` [RFC RESEND 0/3] vp9 v4l2 stateless uapi Hans Verkuil
2021-04-26 17:37   ` Nicolas Dufresne
2021-04-26 23:34     ` Ezequiel Garcia
2021-04-29  9:23       ` Hans Verkuil
2021-04-29 19:38         ` Nicolas Dufresne
2021-04-30  8:12           ` Hans Verkuil

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=7fc04abf3959720a9f344bc3a67b4792552811c1.camel@ndufresne.ca \
    --to=nicolas@ndufresne.ca \
    --cc=adrian.ratiu@collabora.com \
    --cc=andrzej.p@collabora.com \
    --cc=boris.brezillon@collabora.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=ezequiel@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hverkuil@xs4all.nl \
    --cc=kernel@collabora.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mchehab@kernel.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).