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

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

[Ming Qian]

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
+	an input. This can come from a video decoder. Applications will query
+	the new colorspace information (if any, the signal may also have been
+	lost)
+
+	For stateful decoders follow the guidelines in :ref:`decoder`.
+
 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;
-- 
2.43.0-rc1

[PATCH v2 2/4] media: docs: dev-decoder: Trigger dynamic source change for colorspace

[Ming Qian]

If colorspace changes, the client needs to renegotiate the pipeline,
otherwise the decoded frame may not be displayed correctly.

When a colorspace change in the stream, the decoder sends a
V4L2_EVENT_SOURCE_CHANGE event with changes set to
V4L2_EVENT_SRC_CH_COLORSPACE. After client receive this 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>
---
v2
- Add V4L2_EVENT_SRC_CH_COLORSPACE for colorspace source change event

 .../userspace-api/media/v4l/dev-decoder.rst     | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/dev-decoder.rst b/Documentation/userspace-api/media/v4l/dev-decoder.rst
index ef8e8cf31f90..51d6da3eea4a 100644
--- a/Documentation/userspace-api/media/v4l/dev-decoder.rst
+++ b/Documentation/userspace-api/media/v4l/dev-decoder.rst
@@ -784,8 +784,8 @@ before the sequence started. Last of the buffers will have the
 must check if there is any pending event and:
 
 * if a ``V4L2_EVENT_SOURCE_CHANGE`` event with ``changes`` set to
-  ``V4L2_EVENT_SRC_CH_RESOLUTION`` is pending, the `Dynamic Resolution
-  Change` sequence needs to be followed,
+  ``V4L2_EVENT_SRC_CH_RESOLUTION`` or ``V4L2_EVENT_SRC_CH_COLORSPACE`` is pending,
+  the `Dynamic Resolution Change` sequence needs to be followed,
 
 * if a ``V4L2_EVENT_EOS`` event is pending, the `End of Stream` sequence needs
   to be followed.
@@ -932,13 +932,17 @@ 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:
 
 1.  After encountering a resolution change in the stream, the decoder sends a
     ``V4L2_EVENT_SOURCE_CHANGE`` event with ``changes`` set to
-    ``V4L2_EVENT_SRC_CH_RESOLUTION``.
+    ``V4L2_EVENT_SRC_CH_RESOLUTION``, or a colorspace change in the stream, the
+    decoder sends a ``V4L2_EVENT_SOURCE_CHANGE`` event with ``changes`` set to
+    ``V4L2_EVENT_SRC_CH_COLORSPACE``.
 
     .. important::
 
@@ -946,6 +950,11 @@ Whenever that happens, the decoder must proceed as follows:
        values applying to the stream after the resolution change, including
        queue formats, selection rectangles and controls.
 
+.. note::
+        A ``V4L2_EVENT_SOURCE_CHANGE`` event with ``changes`` set to
+        ``V4L2_EVENT_SRC_CH_RESOLUTION`` will affect the allocation, but
+        ``V4L2_EVENT_SRC_CH_COLORSPACE`` won't.
+
 2.  The decoder will then process and decode all remaining buffers from before
     the resolution change point.
 
-- 
2.43.0-rc1

[PATCH v2 3/4] media: amphion: Clear last_buffer_dequeued flag for DEC_CMD_START

[Ming Qian]

From: Ming Qian <ming.qian@nxp.com>

The V4L2_DEC_CMD_START command may be used to handle the dynamic source
change, which will triggers an implicit decoder drain.
The last_buffer_dequeued flag is set in the implicit decoder drain,
so driver need to clear it to continue the following decoding flow.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 drivers/media/platform/amphion/vdec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
index b752afc6276f..3b848ac2cd0c 100644
--- a/drivers/media/platform/amphion/vdec.c
+++ b/drivers/media/platform/amphion/vdec.c
@@ -728,6 +728,7 @@ static int vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd
 	switch (cmd->cmd) {
 	case V4L2_DEC_CMD_START:
 		vdec_cmd_start(inst);
+		vb2_clear_last_buffer_dequeued(v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx));
 		break;
 	case V4L2_DEC_CMD_STOP:
 		vdec_cmd_stop(inst);
-- 
2.43.0-rc1

[PATCH v2 4/4] media: amphion: Trigger source change if colorspace chagned

[Ming Qian]

From: Ming Qian <ming.qian@nxp.com>

After encountering a colorspace change in the stream, the decoder
sends a V4L2_EVENT_SOURCE_CHANGE event with changes set to
V4L2_EVENT_SRC_CH_COLORSPACE.

Then the client can handle the colorspace change without any buffer
reallocation

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 drivers/media/platform/amphion/vdec.c     | 89 +++++++++++++++--------
 drivers/media/platform/amphion/vpu.h      |  1 +
 drivers/media/platform/amphion/vpu_v4l2.c |  7 +-
 3 files changed, 63 insertions(+), 34 deletions(-)

diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
index 3b848ac2cd0c..868a96d29fac 100644
--- a/drivers/media/platform/amphion/vdec.c
+++ b/drivers/media/platform/amphion/vdec.c
@@ -369,12 +369,16 @@ static void vdec_handle_resolution_change(struct vpu_inst *inst)
 	if (!vdec->source_change)
 		return;
 
+	if (inst->changes) {
+		vpu_notify_source_change(inst);
+		inst->changes = 0;
+	}
+
 	q = v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx);
 	if (!list_empty(&q->done_list))
 		return;
 
 	vdec->source_change--;
-	vpu_notify_source_change(inst);
 	vpu_set_last_buffer_dequeued(inst, false);
 }
 
@@ -954,10 +958,11 @@ static void vdec_stop_done(struct vpu_inst *inst)
 	vpu_inst_unlock(inst);
 }
 
-static bool vdec_check_source_change(struct vpu_inst *inst)
+static bool vdec_check_source_change(struct vpu_inst *inst, struct vpu_dec_codec_info *hdr)
 {
 	struct vdec_t *vdec = inst->priv;
 	const struct vpu_format *sibling;
+	u32 changes = 0;
 
 	if (!inst->fh.m2m_ctx)
 		return false;
@@ -966,27 +971,41 @@ static bool vdec_check_source_change(struct vpu_inst *inst)
 		return false;
 
 	sibling = vpu_helper_find_sibling(inst, inst->cap_format.type, inst->cap_format.pixfmt);
-	if (sibling && vdec->codec_info.pixfmt == sibling->pixfmt)
-		vdec->codec_info.pixfmt = inst->cap_format.pixfmt;
+	if (sibling && hdr->pixfmt == sibling->pixfmt)
+		hdr->pixfmt = inst->cap_format.pixfmt;
 
 	if (!vb2_is_streaming(v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx)))
-		return true;
-	if (inst->cap_format.pixfmt != vdec->codec_info.pixfmt)
-		return true;
-	if (inst->cap_format.width != vdec->codec_info.decoded_width)
-		return true;
-	if (inst->cap_format.height != vdec->codec_info.decoded_height)
-		return true;
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+	if (inst->cap_format.pixfmt != hdr->pixfmt)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+	if (inst->cap_format.width != hdr->decoded_width)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+	if (inst->cap_format.height != hdr->decoded_height)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
 	if (vpu_get_num_buffers(inst, inst->cap_format.type) < inst->min_buffer_cap)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+	if (inst->crop.left != hdr->offset_x)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+	if (inst->crop.top != hdr->offset_y)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+	if (inst->crop.width != hdr->width)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+	if (inst->crop.height != hdr->height)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+	if (!hdr->progressive)
+		changes |= V4L2_EVENT_SRC_CH_RESOLUTION;
+
+	if (vdec->seq_hdr_found &&
+	    (hdr->color_primaries != vdec->codec_info.color_primaries ||
+	     hdr->transfer_chars != vdec->codec_info.transfer_chars ||
+	     hdr->matrix_coeffs != vdec->codec_info.matrix_coeffs ||
+	     hdr->full_range != vdec->codec_info.full_range))
+		changes |= V4L2_EVENT_SRC_CH_COLORSPACE;
+
+	if (changes) {
+		inst->changes |= changes;
 		return true;
-	if (inst->crop.left != vdec->codec_info.offset_x)
-		return true;
-	if (inst->crop.top != vdec->codec_info.offset_y)
-		return true;
-	if (inst->crop.width != vdec->codec_info.width)
-		return true;
-	if (inst->crop.height != vdec->codec_info.height)
-		return true;
+	}
 
 	return false;
 }
@@ -1337,20 +1356,25 @@ static void vdec_event_seq_hdr(struct vpu_inst *inst, struct vpu_dec_codec_info
 	struct vdec_t *vdec = inst->priv;
 
 	vpu_inst_lock(inst);
-	memcpy(&vdec->codec_info, hdr, sizeof(vdec->codec_info));
 
-	vpu_trace(inst->dev, "[%d] %d x %d, crop : (%d, %d) %d x %d, %d, %d\n",
+	vpu_trace(inst->dev,
+		  "[%d] %d x %d, crop : (%d, %d) %d x %d, %d, %d, colorspace: %d, %d, %d, %d\n",
 		  inst->id,
-		  vdec->codec_info.decoded_width,
-		  vdec->codec_info.decoded_height,
-		  vdec->codec_info.offset_x,
-		  vdec->codec_info.offset_y,
-		  vdec->codec_info.width,
-		  vdec->codec_info.height,
+		  hdr->decoded_width,
+		  hdr->decoded_height,
+		  hdr->offset_x,
+		  hdr->offset_y,
+		  hdr->width,
+		  hdr->height,
 		  hdr->num_ref_frms,
-		  hdr->num_dpb_frms);
+		  hdr->num_dpb_frms,
+		  hdr->color_primaries,
+		  hdr->transfer_chars,
+		  hdr->matrix_coeffs,
+		  hdr->full_range);
 	inst->min_buffer_cap = hdr->num_ref_frms + hdr->num_dpb_frms;
-	vdec->is_source_changed = vdec_check_source_change(inst);
+	vdec->is_source_changed = vdec_check_source_change(inst, hdr);
+	memcpy(&vdec->codec_info, hdr, sizeof(vdec->codec_info));
 	vdec_init_fmt(inst);
 	vdec_init_crop(inst);
 	vdec_init_mbi(inst);
@@ -1379,7 +1403,12 @@ static void vdec_event_resolution_change(struct vpu_inst *inst)
 {
 	struct vdec_t *vdec = inst->priv;
 
-	vpu_trace(inst->dev, "[%d]\n", inst->id);
+	vpu_trace(inst->dev, "[%d] input : %d, decoded : %d, display : %d, sequence : %d\n",
+		  inst->id,
+		  vdec->params.frame_count,
+		  vdec->decoded_frame_count,
+		  vdec->display_frame_count,
+		  vdec->sequence);
 	vpu_inst_lock(inst);
 	vdec->seq_tag = vdec->codec_info.tag;
 	vdec_clear_fs(&vdec->mbi);
diff --git a/drivers/media/platform/amphion/vpu.h b/drivers/media/platform/amphion/vpu.h
index 76bfd6b26170..d8100da160d1 100644
--- a/drivers/media/platform/amphion/vpu.h
+++ b/drivers/media/platform/amphion/vpu.h
@@ -272,6 +272,7 @@ struct vpu_inst {
 	u8 xfer_func;
 	u32 sequence;
 	u32 extra_size;
+	u32 changes;
 
 	u32 flows[16];
 	u32 flow_idx;
diff --git a/drivers/media/platform/amphion/vpu_v4l2.c b/drivers/media/platform/amphion/vpu_v4l2.c
index 74668fa362e2..37ef706c29dd 100644
--- a/drivers/media/platform/amphion/vpu_v4l2.c
+++ b/drivers/media/platform/amphion/vpu_v4l2.c
@@ -96,13 +96,12 @@ int vpu_notify_eos(struct vpu_inst *inst)
 
 int vpu_notify_source_change(struct vpu_inst *inst)
 {
-	static const struct v4l2_event ev = {
-		.id = 0,
+	const struct v4l2_event ev = {
 		.type = V4L2_EVENT_SOURCE_CHANGE,
-		.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION
+		.u.src_change.changes = inst->changes
 	};
 
-	vpu_trace(inst->dev, "[%d]\n", inst->id);
+	vpu_trace(inst->dev, "[%d] source change 0x%x\n", inst->id, inst->changes);
 	v4l2_event_queue_fh(&inst->fh, &ev);
 	return 0;
 }
-- 
2.43.0-rc1

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

[Hans Verkuil]

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

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

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

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'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?

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;

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

[Ming Qian(OSS)]

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.

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

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

[Hans Verkuil]

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'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;
>>
> 

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

[Ming Qian(OSS)]

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

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

[Hans Verkuil]

On 4/8/25 10:45, Ming Qian(OSS) wrote:
> 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.

I meant stateful codec drivers that are in mainline. Out-of-tree drivers do not
concern me.

Regards,

	Hans

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

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

[Ming Qian(OSS)]

Hi Hans,

On 2025/4/8 17:39, Hans Verkuil wrote:
> On 4/8/25 10:45, Ming Qian(OSS) wrote:
>> 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.
> 
> I meant stateful codec drivers that are in mainline. Out-of-tree drivers do not
> concern me.
> 
> Regards,
> 
> 	Hans
> 

Sorry, I misunderstand

1. Chips&Media wave5 driver, it enable colorspace change detection for 
AVC format,
but disable for HEVC format, but in firmware, it only detect colorspace
change for HEVC format, so the result is it ignore the colorspace
change.
2. Aspeed driver, it only check resolution change. ignore colorspace.
3. Amphion driver, it ignores the colorspace change.
4. Mediatek vcodec driver, it only check resolution change.
5. Qualcomm iris decoder driver, not sure, firmware report seq change
event.
6. Qualcomm Venus driver, not sure, firmwae report sequence change
event, but vdec_event_change() didn't handle colorspace.
7. Amlogic video decoder driver, it only check resolution change, no
colorspace change check.

Regards,
Ming

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

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

[Nicolas Dufresne]

Hi Hans,

Le lundi 07 avril 2025 à 11:54 +0200, Hans Verkuil a écrit :
> 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
> 
> > +	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.
> 
> > +
> > +	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.
> 
> 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'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?

So its used in two places:

At runtime (shared between HDMI receiver and decoders):

    if ((event.u.src_change.changes & V4L2_EVENT_SRC_CH_RESOLUTION) ==
0) {
      GST_DEBUG_OBJECT (v4l2object->dbg_obj,
          "Received non-resolution source-change, ignoring.");
      goto again;
    }


Meaning we currently completely ignore the event if it does not have
the flag. And the second usage is while waiting for the initial
resolution:

      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;
      }



I remember some doc explaining that once we receive that even we must
cycle through streamoff/on, but can't find it now, I'll check further
tomorrow. I wanted to check if that was specific to
V4L2_EVENT_SRC_CH_RESOLUTION or global to V4L2_EVENT_SOURCE_CHANGE.
Also, there is likely some difference between codec and HDMI.

I think backward compatibility will be difficult on that one. I'm happy
that you find the time to have a look, and looking for good ideas.

Nicolas


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