Re: [PATCH v2 1/4] media: v4l: dev-decoder: Add source change V4L2_EVENT_SRC_CH_COLORSPACE

["Ming Qian(OSS)" <ming.qian@oss.nxp.com>]

Hi Hans,

On 2025/4/8 16:30, Hans Verkuil wrote:
> On 08/04/2025 08:34, Ming Qian(OSS) wrote:
>> Hi Hans,
>>
>> On 2025/4/7 17:54, Hans Verkuil wrote:
>>> On 17/01/2025 07:19, Ming Qian wrote:
>>>> Add a new source change V4L2_EVENT_SRC_CH_COLORSPACE that
>>>> indicates colorspace change in the stream.
>>>> The change V4L2_EVENT_SRC_CH_RESOLUTION will always affect
>>>> the allocation, but V4L2_EVENT_SRC_CH_COLORSPACE won't.
>>>>
>>>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>>>> ---
>>>>    Documentation/userspace-api/media/v4l/vidioc-dqevent.rst | 9 +++++++++
>>>>    .../userspace-api/media/videodev2.h.rst.exceptions       | 1 +
>>>>    include/uapi/linux/videodev2.h                           | 1 +
>>>>    3 files changed, 11 insertions(+)
>>>>
>>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>> index 8db103760930..91e6b86c976d 100644
>>>> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
>>>> @@ -369,6 +369,15 @@ call.
>>>>    	loss of signal and so restarting streaming I/O is required in order for
>>>>    	the hardware to synchronize to the video signal.
>>>>    
>>>> +    * - ``V4L2_EVENT_SRC_CH_COLORSPACE``
>>>> +      - 0x0002
>>>> +      - This event gets triggered when a colorsapce change is detected at
>>>
>>> colorsapce -> colorspace
>>>
>>
>> Will fix in v3
>>
>>>> +	an input. This can come from a video decoder. Applications will query
>>>
>>> It can also come from a video receiver. E.g. an HDMI source changes colorspace
>>> signaling, but not the resolution.
>>>
>>>> +	the new colorspace information (if any, the signal may also have been
>>>> +	lost)
>>>
>>> Missing . at the end. Also, if the signal is lost, then that is a CH_RESOLUTION
>>> change, not CH_COLORSPACE.
>>>
>> OK, will fix in v3
>>>> +
>>>> +	For stateful decoders follow the guidelines in :ref:`decoder`.
>>>
>>> I think this should emphasize that if CH_COLORSPACE is set, but not CH_RESOLUTION,
>>> then only the colorspace changed and there is no need to reallocate buffers.
>>>
>>
>> OK, will add in v3
>>
>>> I also wonder if the description of CH_RESOLUTION should be enhanced to explain
>>> that this might also imply a colorspace change. I'm not sure what existing codec
>>> drivers do if there is a colorspace change but no resolution change.
>>
>> I think there is no uniform behavior at the moment, it depends on the
>> behavior of the decoder. Maybe most decoders ignore this.
> 
> Can you try to do a quick analysis of this? Don't spend too much time on this,
> but it is helpful to have an idea of how existing codecs handle this.
> 
> Regards,
> 
> 	Hans
> 

I checked the vpu used in our platforms,
1. amphion vpu, it will ignore the colorspace change.
2. hantro g1/g2 decoder, it also ignore the colorspace change.
3. chipsnmedia wave6 decoder, the firmware detect the colorspace change
for HEVC format, but ignore for AVC format. But its driver just ignore
it.
4. chipsnmedia wave511 decoder, same as wave6.

Regards,
Ming

>>
>>>
>>> I'm a bit concerned about backwards compatibility issues: if a userspace application
>>> doesn't understand this new flag and just honors CH_RESOLUTION, then it would
>>> never react to just a colorspace change.
>>>
>>> Nicolas, does gstreamer look at these flags?
>>
>> I checked the gstreamer code, it does check this flag:
>>
>> if (event.type == V4L2_EVENT_SOURCE_CHANGE &&
>>       (event.u.src_change.changes & V4L2_EVENT_SRC_CH_RESOLUTION)) {
>>     GST_DEBUG_OBJECT (v4l2object->dbg_obj,
>>         "Can't streamon capture as the resolution have changed.");
>>     ret = GST_V4L2_FLOW_RESOLUTION_CHANGE;
>> }
>>
>> Currently the gstreamer can't handle the CH_COLORSPACE flag.
>>
>> Thanks,
>> Ming
>>
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>>>> +
>>>>    Return Value
>>>>    ============
>>>>    
>>>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>> index 35d3456cc812..ac47c6d9448b 100644
>>>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>>>> @@ -526,6 +526,7 @@ replace define V4L2_EVENT_CTRL_CH_RANGE ctrl-changes-flags
>>>>    replace define V4L2_EVENT_CTRL_CH_DIMENSIONS ctrl-changes-flags
>>>>    
>>>>    replace define V4L2_EVENT_SRC_CH_RESOLUTION src-changes-flags
>>>> +replace define V4L2_EVENT_SRC_CH_COLORSPACE src-changes-flags
>>>>    
>>>>    replace define V4L2_EVENT_MD_FL_HAVE_FRAME_SEQ :c:type:`v4l2_event_motion_det`
>>>>    
>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>> index c8cb2796130f..242242c8e57b 100644
>>>> --- a/include/uapi/linux/videodev2.h
>>>> +++ b/include/uapi/linux/videodev2.h
>>>> @@ -2559,6 +2559,7 @@ struct v4l2_event_frame_sync {
>>>>    };
>>>>    
>>>>    #define V4L2_EVENT_SRC_CH_RESOLUTION		(1 << 0)
>>>> +#define V4L2_EVENT_SRC_CH_COLORSPACE		(1 << 1)
>>>>    
>>>>    struct v4l2_event_src_change {
>>>>    	__u32 changes;
>>>
>>
>