linux-kernel - Re: [PATCH] media: docs: dev-decoder: Trigger dynamic source change for colorspace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bed10b8bcd85b4c03737107ac0aea1375d18a50e.camel@ndufresne.ca>
Date: Thu, 09 Jan 2025 11:03:58 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: "Ming Qian(OSS)" <ming.qian@....nxp.com>, mchehab@...nel.org, 
	hverkuil-cisco@...all.nl
Cc: shawnguo@...nel.org, robh+dt@...nel.org, s.hauer@...gutronix.de, 
	kernel@...gutronix.de, festevam@...il.com, linux-imx@....com,
 xiahong.bao@....com, 	eagle.zhou@....com, tao.jiang_2@....com,
 imx@...ts.linux.dev, 	linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org, 	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] media: docs: dev-decoder: Trigger dynamic source change
 for colorspace

Le jeudi 09 janvier 2025 à 10:25 +0800, Ming Qian(OSS) a écrit :
> 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@....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.

I agree an event is better then this ...

> 
> 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.

But I think this is too much to put this under the changes
  
   #define V4L2_EVENT_SRC_CH_RESOLUTION            (1 << 0)

We include under "resolution" everything that affects the allocation. So perhaps
for this one we can introduce

   #define V4L2_EVENT_SRC_CH_COLORSPACE            (2 << 0)

Of course, userspace implementation will be needed. Anyone one else have an
opinion here ?

Nicolas

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