Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
From: "Ming Qian(OSS)" <ming.qian@oss.nxp.com>
To: Nicolas Dufresne <nicolas@ndufresne.ca>,
	mchehab@kernel.org, hverkuil-cisco@xs4all.nl
Cc: shawnguo@kernel.org, robh+dt@kernel.org, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com, linux-imx@nxp.com,
	xiahong.bao@nxp.com, eagle.zhou@nxp.com, tao.jiang_2@nxp.com,
	imx@lists.linux.dev, linux-media@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] media: docs: dev-decoder: Trigger dynamic source change for colorspace
Date: Thu, 9 Jan 2025 10:25:04 +0800	[thread overview]
Message-ID: <1c1bb1b3-7e58-4a49-83bb-2c560cd66858@oss.nxp.com> (raw)
In-Reply-To: <dcae7fbb810ebfa6e539c3b45c20e1d659600d80.camel@ndufresne.ca>


Hi Nicolas,

On 2025/1/9 3:34, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 07 janvier 2025 à 14:36 +0900, Ming Qian a écrit :
>> If colorspace changes, the client needs to renegotiate the pipeline,
>> otherwise the decoded frame may not be displayed correctly.
>>
>> If it can trigger an source change event, then client can switch to the
>> correct stream setting. And each frame can be displayed properly.
>>
>> So add colorspace as a trigger parameter for dynamic resolution change.
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>>   Documentation/userspace-api/media/v4l/dev-decoder.rst | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/dev-decoder.rst b/Documentation/userspace-api/media/v4l/dev-decoder.rst
>> index ef8e8cf31f90..49566569ad26 100644
>> --- a/Documentation/userspace-api/media/v4l/dev-decoder.rst
>> +++ b/Documentation/userspace-api/media/v4l/dev-decoder.rst
>> @@ -932,7 +932,9 @@ reflected by corresponding queries):
>>   
>>   * the minimum number of buffers needed for decoding,
>>   
>> -* bit-depth of the bitstream has been changed.
>> +* bit-depth of the bitstream has been changed,
>> +
>> +* colorspace of the bitstream has been changed.
> 
> Did you really mean colorspace in the way this term is used in V4L2 ? What we
> want this event to be used for is when the capture storage size or amount
> changes, perhaps you mean when the capture pixelformat changes ? This will
> indeed happen if you change the bit-depth, subsampling (not mentioned here
> either) or change the way colors are repsented (RGB, YCbCr, etc.).
> 

I am referring to the following attributes in v4l2_pix_fmt:
	__u32		colorspace;	/* enum v4l2_colorspace */
	__u32		ycbcr_enc;	/* enum v4l2_ycbcr_encoding */
	__u32		quantization;	/* enum v4l2_quantization */
	__u32		xfer_func;	/* enum v4l2_xfer_func */

For decoder, they are parsed from the sequence header.
Our issue is that when only these properties change in the middle of
some bitstream, but not the resolution or dpb amount, the decoder needs
to nofity the user.  As these properties are in v4l2_pix fmt, user need
to get/set them via VIDIOC_G_FMT/VIDIOC_S_FMT.
So in my opinion, it's reasonable to nitify user a source change event,
then user can call v4l_g_fmt() and renegotiate the pipeline.

Apart from this, all I can think of is that user call v4l_g_fmt() before
dequeueing each frame. But I don't think this is a good idea.

As these properties are parts of the v4l2_format, I think it's
reasonable to handle their changes via the dynamic source change flow.

We're currently facing some real cases on android.

Or do you have any good suggestions? Then I can give a try.

Thanks,
Ming

>>   
>>   Whenever that happens, the decoder must proceed as follows:
>>   
> 

  reply	other threads:[~2025-01-09  2:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-07  5:36 [PATCH] media: docs: dev-decoder: Trigger dynamic source change for colorspace Ming Qian
2025-01-08 19:34 ` Nicolas Dufresne
2025-01-09  2:25   ` Ming Qian(OSS) [this message]
2025-01-09 16:03     ` Nicolas Dufresne
2025-01-10  2:25       ` Ming Qian(OSS)

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=1c1bb1b3-7e58-4a49-83bb-2c560cd66858@oss.nxp.com \
    --to=ming.qian@oss.nxp.com \
    --cc=eagle.zhou@nxp.com \
    --cc=festevam@gmail.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=nicolas@ndufresne.ca \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=tao.jiang_2@nxp.com \
    --cc=xiahong.bao@nxp.com \
    /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