| Message ID | 20250107053622.1287461-1-ming.qian@oss.nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: docs: dev-decoder: Trigger dynamic source change for colorspace | expand |
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.). > > Whenever that happens, the decoder must proceed as follows: >
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: >> >
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@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. 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: > > > > >
Hi Nicolas, On 2025/1/10 0:03, Nicolas Dufresne wrote: > 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@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. > > 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 > I agree with you. Defining a new colorspace changes does make things more clearer. And it can avoid buffer reallocations. Then it will not affect the existing flow. Maybe I can prepare a v2 patch after doing some basic verification. Thanks, Ming >> >> Thanks, >> Ming >> >>>> >>>> Whenever that happens, the decoder must proceed as follows: >>>> >>> >
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. Whenever that happens, the decoder must proceed as follows:
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(-)