[PATCH 0/2] Add decoder conceal color ctrl

[PATCH 0/2] Add decoder conceal color ctrl

[Stanimir Varbanov]

Hello,

The first patch is adding a new control for conceal error color and the
second adds support for it in the Venus decoder driver.

Comments are welcome!

regards,
Stan

Stanimir Varbanov (2):
  v4l2-ctrl: Add decoder conceal color control
  venus: vdec: Add support for conceal control

 .../media/v4l/ext-ctrls-codec.rst             | 20 +++++++++++++++++++
 drivers/media/platform/qcom/venus/core.h      |  1 +
 drivers/media/platform/qcom/venus/hfi_cmds.c  | 18 +++++++++++++++--
 .../media/platform/qcom/venus/hfi_helper.h    | 10 ++++++++++
 drivers/media/platform/qcom/venus/vdec.c      | 11 +++++++++-
 .../media/platform/qcom/venus/vdec_ctrls.c    |  7 +++++++
 drivers/media/v4l2-core/v4l2-ctrls.c          |  9 +++++++++
 include/uapi/linux/v4l2-controls.h            |  1 +
 8 files changed, 74 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 1/2] v4l2-ctrl: Add decoder conceal color control

[Stanimir Varbanov]

Add decoder v4l2 control to set conceal color.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 .../media/v4l/ext-ctrls-codec.rst             | 20 +++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ctrls.c          |  9 +++++++++
 include/uapi/linux/v4l2-controls.h            |  1 +
 3 files changed, 30 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index 00944e97d638..994650052333 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode -
     is currently displayed (decoded). This value is reset to 0 whenever
     the decoder is started.
 
+``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)``
+    This control sets conceal color in YUV color space. It describes the
+    client preference of error conceal color in case of error where
+    reference frame is missing. The decoder would paint the reference
+    buffer with preferred color and use it for future decoding.
+    Applicable to decoders.
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+
+    * - Bit 0:15
+      - Y luminance
+    * - Bit 16:31
+      - Cb chrominance
+    * - Bit 32:47
+      - Cr chrominance
+    * - Bit 48:63
+      - Must be zero
+
 ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)``
     If enabled the decoder expects to receive a single slice per buffer,
     otherwise the decoder expects a single frame in per buffer.
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 016cf6204cbb..a3b9d28a00b7 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:			return "VBV Buffer Size";
 	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
 	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
+	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:		return "Video Decoder Conceal Color";
 	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
 	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
 	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
@@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		*max = 0x7fffffffffffffffLL;
 		*step = 1;
 		break;
+	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
+		*type = V4L2_CTRL_TYPE_INTEGER64;
+		*min = 0;
+		/* default for 8bit black, luma is 16, chroma is 128 */
+		*def = 0x8000800010LL;
+		*max = 0xffffffffffffLL;
+		*step = 1;
+		break;
 	case V4L2_CID_PIXEL_RATE:
 		*type = V4L2_CTRL_TYPE_INTEGER64;
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 039c0d7add1b..5e5a3068be2d 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -428,6 +428,7 @@ enum v4l2_mpeg_video_multi_slice_mode {
 #define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_CODEC_BASE+228)
 #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME		(V4L2_CID_CODEC_BASE+229)
 #define V4L2_CID_MPEG_VIDEO_BASELAYER_PRIORITY_ID	(V4L2_CID_CODEC_BASE+230)
+#define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR		(V4L2_CID_CODEC_BASE+231)
 
 /* CIDs for the MPEG-2 Part 2 (H.262) codec */
 #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL			(V4L2_CID_CODEC_BASE+270)
-- 
2.25.1

[PATCH 2/2] venus: vdec: Add support for conceal control

[Stanimir Varbanov]

Adds support for decoder conceal color control.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/core.h       |  1 +
 drivers/media/platform/qcom/venus/hfi_cmds.c   | 18 ++++++++++++++++--
 drivers/media/platform/qcom/venus/hfi_helper.h | 10 ++++++++++
 drivers/media/platform/qcom/venus/vdec.c       | 11 ++++++++++-
 drivers/media/platform/qcom/venus/vdec_ctrls.c |  7 +++++++
 5 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index a252ed32cc14..b70ff14e5a5b 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -172,6 +172,7 @@ struct vdec_controls {
 	u32 post_loop_deb_mode;
 	u32 profile;
 	u32 level;
+	u64 conceal_color;
 };
 
 struct venc_controls {
diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
index 4f7565834469..884339aae6a7 100644
--- a/drivers/media/platform/qcom/venus/hfi_cmds.c
+++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
@@ -760,7 +760,9 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt,
 		struct hfi_conceal_color *color = prop_data;
 		u32 *in = pdata;
 
-		color->conceal_color = *in;
+		color->conceal_color = *in & 0xff;
+		color->conceal_color |= ((*in >> 10) & 0xff) << 8;
+		color->conceal_color |= ((*in >> 20) & 0xff) << 16;
 		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*color);
 		break;
 	}
@@ -1255,7 +1257,19 @@ pkt_session_set_property_6xx(struct hfi_session_set_property_pkt *pkt,
 		cq->frame_quality = in->frame_quality;
 		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*cq);
 		break;
-	} default:
+	}
+	case HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR: {
+		struct hfi_conceal_color_v4 *color = prop_data;
+		u32 *in = pdata;
+
+		color->conceal_color_8bit = *in & 0xff;
+		color->conceal_color_8bit |= ((*in >> 10) & 0xff) << 8;
+		color->conceal_color_8bit |= ((*in >> 20) & 0xff) << 16;
+		color->conceal_color_10bit = *in;
+		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*color);
+		break;
+	}
+	default:
 		return pkt_session_set_property_4xx(pkt, cookie, ptype, pdata);
 	}
 
diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
index 6b524c7cde5f..fa49b49170b7 100644
--- a/drivers/media/platform/qcom/venus/hfi_helper.h
+++ b/drivers/media/platform/qcom/venus/hfi_helper.h
@@ -685,10 +685,20 @@ struct hfi_vc1e_perf_cfg_type {
 	u32 search_range_y_subsampled[3];
 };
 
+/*
+ * 0 - 7bit -> Luma (def: 16)
+ * 8 - 15bit -> Chroma (def: 128)
+ * format is valid up to v4
+ */
 struct hfi_conceal_color {
 	u32 conceal_color;
 };
 
+struct hfi_conceal_color_v4 {
+	u32 conceal_color_8bit;
+	u32 conceal_color_10bit;
+};
+
 struct hfi_intra_period {
 	u32 pframes;
 	u32 bframes;
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index e4dc97f00fc3..8d98fca55db1 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -620,7 +620,7 @@ static int vdec_set_properties(struct venus_inst *inst)
 {
 	struct vdec_controls *ctr = &inst->controls.dec;
 	struct hfi_enable en = { .enable = 1 };
-	u32 ptype;
+	u32 ptype, conceal;
 	int ret;
 
 	if (ctr->post_loop_deb_mode) {
@@ -630,6 +630,15 @@ static int vdec_set_properties(struct venus_inst *inst)
 			return ret;
 	}
 
+	ptype = HFI_PROPERTY_PARAM_VDEC_CONCEAL_COLOR;
+	conceal = ctr->conceal_color & 0xffff;
+	conceal |= ((ctr->conceal_color >> 16) & 0xffff) << 10;
+	conceal |= ((ctr->conceal_color >> 32) & 0xffff) << 20;
+
+	ret = hfi_session_set_property(inst, ptype, &conceal);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
diff --git a/drivers/media/platform/qcom/venus/vdec_ctrls.c b/drivers/media/platform/qcom/venus/vdec_ctrls.c
index 974110b75b93..9ff3db1c4a0f 100644
--- a/drivers/media/platform/qcom/venus/vdec_ctrls.c
+++ b/drivers/media/platform/qcom/venus/vdec_ctrls.c
@@ -30,6 +30,9 @@ static int vdec_op_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_MPEG_VIDEO_VP9_LEVEL:
 		ctr->level = ctrl->val;
 		break;
+	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
+		ctr->conceal_color = *ctrl->p_new.p_s64;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -158,6 +161,10 @@ int vdec_ctrl_init(struct venus_inst *inst)
 	if (ctrl)
 		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
 
+	v4l2_ctrl_new_std(&inst->ctrl_handler, &vdec_ctrl_ops,
+			  V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR, 0,
+			  0xffffffffffffLL, 1, 0x8000800010LL);
+
 	ret = inst->ctrl_handler.error;
 	if (ret) {
 		v4l2_ctrl_handler_free(&inst->ctrl_handler);
-- 
2.25.1

Re: [PATCH 1/2] v4l2-ctrl: Add decoder conceal color control

[Hans Verkuil]

On 09/02/2021 10:45, Stanimir Varbanov wrote:
> Add decoder v4l2 control to set conceal color.
> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  .../media/v4l/ext-ctrls-codec.rst             | 20 +++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-ctrls.c          |  9 +++++++++
>  include/uapi/linux/v4l2-controls.h            |  1 +
>  3 files changed, 30 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 00944e97d638..994650052333 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode -
>      is currently displayed (decoded). This value is reset to 0 whenever
>      the decoder is started.
>  
> +``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)``
> +    This control sets conceal color in YUV color space. It describes the
> +    client preference of error conceal color in case of error where
> +    reference frame is missing. The decoder would paint the reference
> +    buffer with preferred color and use it for future decoding.
> +    Applicable to decoders.

You should mention explicitly that this is using 16-bit color components
and expects Limited Range.

> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +
> +    * - Bit 0:15
> +      - Y luminance
> +    * - Bit 16:31
> +      - Cb chrominance
> +    * - Bit 32:47
> +      - Cr chrominance
> +    * - Bit 48:63
> +      - Must be zero
> +
>  ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)``
>      If enabled the decoder expects to receive a single slice per buffer,
>      otherwise the decoder expects a single frame in per buffer.
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 016cf6204cbb..a3b9d28a00b7 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:			return "VBV Buffer Size";
>  	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
>  	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:		return "Video Decoder Conceal Color";
>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
>  	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
>  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
> @@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  		*max = 0x7fffffffffffffffLL;
>  		*step = 1;
>  		break;
> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
> +		*type = V4L2_CTRL_TYPE_INTEGER64;
> +		*min = 0;
> +		/* default for 8bit black, luma is 16, chroma is 128 */

Since this is 16 bit the actual default luma value for black is 4096 and for chroma use
32768 (i.e. both values are times 256).

> +		*def = 0x8000800010LL;
> +		*max = 0xffffffffffffLL;
> +		*step = 1;
> +		break;
>  	case V4L2_CID_PIXEL_RATE:
>  		*type = V4L2_CTRL_TYPE_INTEGER64;
>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 039c0d7add1b..5e5a3068be2d 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -428,6 +428,7 @@ enum v4l2_mpeg_video_multi_slice_mode {
>  #define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_CODEC_BASE+228)
>  #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME		(V4L2_CID_CODEC_BASE+229)
>  #define V4L2_CID_MPEG_VIDEO_BASELAYER_PRIORITY_ID	(V4L2_CID_CODEC_BASE+230)
> +#define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR		(V4L2_CID_CODEC_BASE+231)
>  
>  /* CIDs for the MPEG-2 Part 2 (H.262) codec */
>  #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL			(V4L2_CID_CODEC_BASE+270)
> 

Regards,

	Hans

Re: [PATCH 1/2] v4l2-ctrl: Add decoder conceal color control

[Stanimir Varbanov]

On 2/9/21 1:05 PM, Hans Verkuil wrote:
> On 09/02/2021 10:45, Stanimir Varbanov wrote:
>> Add decoder v4l2 control to set conceal color.
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>>  .../media/v4l/ext-ctrls-codec.rst             | 20 +++++++++++++++++++
>>  drivers/media/v4l2-core/v4l2-ctrls.c          |  9 +++++++++
>>  include/uapi/linux/v4l2-controls.h            |  1 +
>>  3 files changed, 30 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index 00944e97d638..994650052333 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode -
>>      is currently displayed (decoded). This value is reset to 0 whenever
>>      the decoder is started.
>>  
>> +``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)``
>> +    This control sets conceal color in YUV color space. It describes the
>> +    client preference of error conceal color in case of error where
>> +    reference frame is missing. The decoder would paint the reference
>> +    buffer with preferred color and use it for future decoding.
>> +    Applicable to decoders.
> 
> You should mention explicitly that this is using 16-bit color components
> and expects Limited Range.

I don't want to limit the client to Limited range only. I'll mention in
the description that both ranges are valid.

> 
>> +
>> +.. flat-table::
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +
>> +    * - Bit 0:15
>> +      - Y luminance
>> +    * - Bit 16:31
>> +      - Cb chrominance
>> +    * - Bit 32:47
>> +      - Cr chrominance
>> +    * - Bit 48:63
>> +      - Must be zero
>> +
>>  ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)``
>>      If enabled the decoder expects to receive a single slice per buffer,
>>      otherwise the decoder expects a single frame in per buffer.
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 016cf6204cbb..a3b9d28a00b7 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:			return "VBV Buffer Size";
>>  	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
>>  	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:		return "Video Decoder Conceal Color";
>>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
>>  	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
>>  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
>> @@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>  		*max = 0x7fffffffffffffffLL;
>>  		*step = 1;
>>  		break;
>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
>> +		*type = V4L2_CTRL_TYPE_INTEGER64;
>> +		*min = 0;
>> +		/* default for 8bit black, luma is 16, chroma is 128 */
> 
> Since this is 16 bit the actual default luma value for black is 4096 and for chroma use
> 32768 (i.e. both values are times 256).

If we follow this for pixel format with 10bit per channel we have to
multiply by 64?

> 
>> +		*def = 0x8000800010LL;
>> +		*max = 0xffffffffffffLL;
>> +		*step = 1;
>> +		break;
>>  	case V4L2_CID_PIXEL_RATE:
>>  		*type = V4L2_CTRL_TYPE_INTEGER64;
>>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>> index 039c0d7add1b..5e5a3068be2d 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -428,6 +428,7 @@ enum v4l2_mpeg_video_multi_slice_mode {
>>  #define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_CODEC_BASE+228)
>>  #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME		(V4L2_CID_CODEC_BASE+229)
>>  #define V4L2_CID_MPEG_VIDEO_BASELAYER_PRIORITY_ID	(V4L2_CID_CODEC_BASE+230)
>> +#define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR		(V4L2_CID_CODEC_BASE+231)
>>  
>>  /* CIDs for the MPEG-2 Part 2 (H.262) codec */
>>  #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL			(V4L2_CID_CODEC_BASE+270)
>>
> 
> Regards,
> 
> 	Hans
> 

-- 
regards,
Stan

Re: [PATCH 1/2] v4l2-ctrl: Add decoder conceal color control

[Hans Verkuil]

On 15/02/2021 12:32, Stanimir Varbanov wrote:
> 
> 
> On 2/9/21 1:05 PM, Hans Verkuil wrote:
>> On 09/02/2021 10:45, Stanimir Varbanov wrote:
>>> Add decoder v4l2 control to set conceal color.
>>>
>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>>> ---
>>>  .../media/v4l/ext-ctrls-codec.rst             | 20 +++++++++++++++++++
>>>  drivers/media/v4l2-core/v4l2-ctrls.c          |  9 +++++++++
>>>  include/uapi/linux/v4l2-controls.h            |  1 +
>>>  3 files changed, 30 insertions(+)
>>>
>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> index 00944e97d638..994650052333 100644
>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> @@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode -
>>>      is currently displayed (decoded). This value is reset to 0 whenever
>>>      the decoder is started.
>>>  
>>> +``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)``
>>> +    This control sets conceal color in YUV color space. It describes the
>>> +    client preference of error conceal color in case of error where
>>> +    reference frame is missing. The decoder would paint the reference
>>> +    buffer with preferred color and use it for future decoding.
>>> +    Applicable to decoders.
>>
>> You should mention explicitly that this is using 16-bit color components
>> and expects Limited Range.
> 
> I don't want to limit the client to Limited range only. I'll mention in
> the description that both ranges are valid.

OK, but then you need to describe what the color format depends on. See more
below.

> 
>>
>>> +
>>> +.. flat-table::
>>> +    :header-rows:  0
>>> +    :stub-columns: 0
>>> +
>>> +    * - Bit 0:15
>>> +      - Y luminance
>>> +    * - Bit 16:31
>>> +      - Cb chrominance
>>> +    * - Bit 32:47
>>> +      - Cr chrominance
>>> +    * - Bit 48:63
>>> +      - Must be zero
>>> +
>>>  ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)``
>>>      If enabled the decoder expects to receive a single slice per buffer,
>>>      otherwise the decoder expects a single frame in per buffer.
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index 016cf6204cbb..a3b9d28a00b7 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>>  	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:			return "VBV Buffer Size";
>>>  	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
>>>  	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
>>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:		return "Video Decoder Conceal Color";
>>>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
>>>  	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
>>>  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
>>> @@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>>  		*max = 0x7fffffffffffffffLL;
>>>  		*step = 1;
>>>  		break;
>>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
>>> +		*type = V4L2_CTRL_TYPE_INTEGER64;
>>> +		*min = 0;
>>> +		/* default for 8bit black, luma is 16, chroma is 128 */
>>
>> Since this is 16 bit the actual default luma value for black is 4096 and for chroma use
>> 32768 (i.e. both values are times 256).
> 
> If we follow this for pixel format with 10bit per channel we have to
> multiply by 64?

No, you multiply by 4. 12 bit depth will multiple by 16, and 16 bit depth by 256.

But how do you format this? Using bits 29-0? Or use 9-0 for one color component,
25-16 for another and 41-32 for the last component?

Also missing is an explanation of which bits are for Y', which for Cb and which for Cr.

It is surprisingly hard to provide an unambiguous description of this :-)

Regards,

	Hans

> 
>>
>>> +		*def = 0x8000800010LL;
>>> +		*max = 0xffffffffffffLL;
>>> +		*step = 1;
>>> +		break;
>>>  	case V4L2_CID_PIXEL_RATE:
>>>  		*type = V4L2_CTRL_TYPE_INTEGER64;
>>>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>>> index 039c0d7add1b..5e5a3068be2d 100644
>>> --- a/include/uapi/linux/v4l2-controls.h
>>> +++ b/include/uapi/linux/v4l2-controls.h
>>> @@ -428,6 +428,7 @@ enum v4l2_mpeg_video_multi_slice_mode {
>>>  #define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_CODEC_BASE+228)
>>>  #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME		(V4L2_CID_CODEC_BASE+229)
>>>  #define V4L2_CID_MPEG_VIDEO_BASELAYER_PRIORITY_ID	(V4L2_CID_CODEC_BASE+230)
>>> +#define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR		(V4L2_CID_CODEC_BASE+231)
>>>  
>>>  /* CIDs for the MPEG-2 Part 2 (H.262) codec */
>>>  #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL			(V4L2_CID_CODEC_BASE+270)
>>>
>>
>> Regards,
>>
>> 	Hans
>>
> 

Re: [PATCH 1/2] v4l2-ctrl: Add decoder conceal color control

[Stanimir Varbanov]

On 2/15/21 1:57 PM, Hans Verkuil wrote:
> On 15/02/2021 12:32, Stanimir Varbanov wrote:
>>
>>
>> On 2/9/21 1:05 PM, Hans Verkuil wrote:
>>> On 09/02/2021 10:45, Stanimir Varbanov wrote:
>>>> Add decoder v4l2 control to set conceal color.
>>>>
>>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>>>> ---
>>>>  .../media/v4l/ext-ctrls-codec.rst             | 20 +++++++++++++++++++
>>>>  drivers/media/v4l2-core/v4l2-ctrls.c          |  9 +++++++++
>>>>  include/uapi/linux/v4l2-controls.h            |  1 +
>>>>  3 files changed, 30 insertions(+)
>>>>
>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>> index 00944e97d638..994650052333 100644
>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>> @@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode -
>>>>      is currently displayed (decoded). This value is reset to 0 whenever
>>>>      the decoder is started.
>>>>  
>>>> +``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)``
>>>> +    This control sets conceal color in YUV color space. It describes the
>>>> +    client preference of error conceal color in case of error where
>>>> +    reference frame is missing. The decoder would paint the reference
>>>> +    buffer with preferred color and use it for future decoding.
>>>> +    Applicable to decoders.
>>>
>>> You should mention explicitly that this is using 16-bit color components
>>> and expects Limited Range.
>>
>> I don't want to limit the client to Limited range only. I'll mention in
>> the description that both ranges are valid.
> 
> OK, but then you need to describe what the color format depends on. See more
> below.
> 
>>
>>>
>>>> +
>>>> +.. flat-table::
>>>> +    :header-rows:  0
>>>> +    :stub-columns: 0
>>>> +
>>>> +    * - Bit 0:15
>>>> +      - Y luminance
>>>> +    * - Bit 16:31
>>>> +      - Cb chrominance
>>>> +    * - Bit 32:47
>>>> +      - Cr chrominance
>>>> +    * - Bit 48:63
>>>> +      - Must be zero
>>>> +

The table how the bits are spread into int64.

>>>>  ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)``
>>>>      If enabled the decoder expects to receive a single slice per buffer,
>>>>      otherwise the decoder expects a single frame in per buffer.
>>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>>> index 016cf6204cbb..a3b9d28a00b7 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>>> @@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>>>  	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:			return "VBV Buffer Size";
>>>>  	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
>>>>  	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
>>>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:		return "Video Decoder Conceal Color";
>>>>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
>>>>  	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
>>>>  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
>>>> @@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>>>  		*max = 0x7fffffffffffffffLL;
>>>>  		*step = 1;
>>>>  		break;
>>>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
>>>> +		*type = V4L2_CTRL_TYPE_INTEGER64;
>>>> +		*min = 0;
>>>> +		/* default for 8bit black, luma is 16, chroma is 128 */
>>>
>>> Since this is 16 bit the actual default luma value for black is 4096 and for chroma use
>>> 32768 (i.e. both values are times 256).
>>
>> If we follow this for pixel format with 10bit per channel we have to
>> multiply by 64?
> 
> No, you multiply by 4. 12 bit depth will multiple by 16, and 16 bit depth by 256.
> 
> But how do you format this? Using bits 29-0? Or use 9-0 for one color component,
> 25-16 for another and 41-32 for the last component?

I described this in the table above:

Bit  0:15 - Y luminance
Bit 16:31 - Cb chrominance
Bit 32:47 - Cr chrominance
Bit 48:63 - Must be zero

So depending on the bit depth of the current pixel format:

 8bit - 0:7  Y', 16:23 Cb, 32:39 Cr
10bit - 0:9  Y', 16:25 Cb, 32:41 Cr
12bit - 0:11 Y', 16:27 Cb, 32:43 Cr

> 
> Also missing is an explanation of which bits are for Y', which for Cb and which for Cr.
> 
> It is surprisingly hard to provide an unambiguous description of this :-)
> 
> Regards,
> 
> 	Hans
> 
>>
>>>
>>>> +		*def = 0x8000800010LL;
>>>> +		*max = 0xffffffffffffLL;
>>>> +		*step = 1;
>>>> +		break;
>>>>  	case V4L2_CID_PIXEL_RATE:
>>>>  		*type = V4L2_CTRL_TYPE_INTEGER64;
>>>>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>>>> index 039c0d7add1b..5e5a3068be2d 100644
>>>> --- a/include/uapi/linux/v4l2-controls.h
>>>> +++ b/include/uapi/linux/v4l2-controls.h
>>>> @@ -428,6 +428,7 @@ enum v4l2_mpeg_video_multi_slice_mode {
>>>>  #define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_CODEC_BASE+228)
>>>>  #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME		(V4L2_CID_CODEC_BASE+229)
>>>>  #define V4L2_CID_MPEG_VIDEO_BASELAYER_PRIORITY_ID	(V4L2_CID_CODEC_BASE+230)
>>>> +#define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR		(V4L2_CID_CODEC_BASE+231)
>>>>  
>>>>  /* CIDs for the MPEG-2 Part 2 (H.262) codec */
>>>>  #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL			(V4L2_CID_CODEC_BASE+270)
>>>>
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>>
> 

-- 
regards,
Stan

Re: [PATCH 1/2] v4l2-ctrl: Add decoder conceal color control

[Hans Verkuil]

On 16/02/2021 09:56, Stanimir Varbanov wrote:
> 
> 
> On 2/15/21 1:57 PM, Hans Verkuil wrote:
>> On 15/02/2021 12:32, Stanimir Varbanov wrote:
>>>
>>>
>>> On 2/9/21 1:05 PM, Hans Verkuil wrote:
>>>> On 09/02/2021 10:45, Stanimir Varbanov wrote:
>>>>> Add decoder v4l2 control to set conceal color.
>>>>>
>>>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>>>>> ---
>>>>>  .../media/v4l/ext-ctrls-codec.rst             | 20 +++++++++++++++++++
>>>>>  drivers/media/v4l2-core/v4l2-ctrls.c          |  9 +++++++++
>>>>>  include/uapi/linux/v4l2-controls.h            |  1 +
>>>>>  3 files changed, 30 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>>> index 00944e97d638..994650052333 100644
>>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>>> @@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode -
>>>>>      is currently displayed (decoded). This value is reset to 0 whenever
>>>>>      the decoder is started.
>>>>>  
>>>>> +``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)``
>>>>> +    This control sets conceal color in YUV color space. It describes the
>>>>> +    client preference of error conceal color in case of error where
>>>>> +    reference frame is missing. The decoder would paint the reference
>>>>> +    buffer with preferred color and use it for future decoding.
>>>>> +    Applicable to decoders.
>>>>
>>>> You should mention explicitly that this is using 16-bit color components
>>>> and expects Limited Range.
>>>
>>> I don't want to limit the client to Limited range only. I'll mention in
>>> the description that both ranges are valid.
>>
>> OK, but then you need to describe what the color format depends on. See more
>> below.
>>
>>>
>>>>
>>>>> +
>>>>> +.. flat-table::
>>>>> +    :header-rows:  0
>>>>> +    :stub-columns: 0
>>>>> +
>>>>> +    * - Bit 0:15
>>>>> +      - Y luminance
>>>>> +    * - Bit 16:31
>>>>> +      - Cb chrominance
>>>>> +    * - Bit 32:47
>>>>> +      - Cr chrominance
>>>>> +    * - Bit 48:63
>>>>> +      - Must be zero
>>>>> +
> 
> The table how the bits are spread into int64.
> 
>>>>>  ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)``
>>>>>      If enabled the decoder expects to receive a single slice per buffer,
>>>>>      otherwise the decoder expects a single frame in per buffer.
>>>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>>>> index 016cf6204cbb..a3b9d28a00b7 100644
>>>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>>>> @@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>>>>  	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:			return "VBV Buffer Size";
>>>>>  	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
>>>>>  	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
>>>>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:		return "Video Decoder Conceal Color";
>>>>>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
>>>>>  	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
>>>>>  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
>>>>> @@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>>>>  		*max = 0x7fffffffffffffffLL;
>>>>>  		*step = 1;
>>>>>  		break;
>>>>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
>>>>> +		*type = V4L2_CTRL_TYPE_INTEGER64;
>>>>> +		*min = 0;
>>>>> +		/* default for 8bit black, luma is 16, chroma is 128 */
>>>>
>>>> Since this is 16 bit the actual default luma value for black is 4096 and for chroma use
>>>> 32768 (i.e. both values are times 256).
>>>
>>> If we follow this for pixel format with 10bit per channel we have to
>>> multiply by 64?
>>
>> No, you multiply by 4. 12 bit depth will multiple by 16, and 16 bit depth by 256.
>>
>> But how do you format this? Using bits 29-0? Or use 9-0 for one color component,
>> 25-16 for another and 41-32 for the last component?
> 
> I described this in the table above:
> 
> Bit  0:15 - Y luminance
> Bit 16:31 - Cb chrominance
> Bit 32:47 - Cr chrominance
> Bit 48:63 - Must be zero
> 
> So depending on the bit depth of the current pixel format:
> 
>  8bit - 0:7  Y', 16:23 Cb, 32:39 Cr
> 10bit - 0:9  Y', 16:25 Cb, 32:41 Cr
> 12bit - 0:11 Y', 16:27 Cb, 32:43 Cr

Apologies, I missed that table!

Regards,

	Hans

> 
>>
>> Also missing is an explanation of which bits are for Y', which for Cb and which for Cr.
>>
>> It is surprisingly hard to provide an unambiguous description of this :-)
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>>>
>>>>> +		*def = 0x8000800010LL;
>>>>> +		*max = 0xffffffffffffLL;
>>>>> +		*step = 1;
>>>>> +		break;
>>>>>  	case V4L2_CID_PIXEL_RATE:
>>>>>  		*type = V4L2_CTRL_TYPE_INTEGER64;
>>>>>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>>>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>>>>> index 039c0d7add1b..5e5a3068be2d 100644
>>>>> --- a/include/uapi/linux/v4l2-controls.h
>>>>> +++ b/include/uapi/linux/v4l2-controls.h
>>>>> @@ -428,6 +428,7 @@ enum v4l2_mpeg_video_multi_slice_mode {
>>>>>  #define V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE		(V4L2_CID_CODEC_BASE+228)
>>>>>  #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME		(V4L2_CID_CODEC_BASE+229)
>>>>>  #define V4L2_CID_MPEG_VIDEO_BASELAYER_PRIORITY_ID	(V4L2_CID_CODEC_BASE+230)
>>>>> +#define V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR		(V4L2_CID_CODEC_BASE+231)
>>>>>  
>>>>>  /* CIDs for the MPEG-2 Part 2 (H.262) codec */
>>>>>  #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL			(V4L2_CID_CODEC_BASE+270)
>>>>>
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>>
>>
> 

Re: [PATCH 1/2] v4l2-ctrl: Add decoder conceal color control

[Stanimir Varbanov]

On 2/16/21 10:58 AM, Hans Verkuil wrote:
> On 16/02/2021 09:56, Stanimir Varbanov wrote:
>>
>>
>> On 2/15/21 1:57 PM, Hans Verkuil wrote:
>>> On 15/02/2021 12:32, Stanimir Varbanov wrote:
>>>>
>>>>
>>>> On 2/9/21 1:05 PM, Hans Verkuil wrote:
>>>>> On 09/02/2021 10:45, Stanimir Varbanov wrote:
>>>>>> Add decoder v4l2 control to set conceal color.
>>>>>>
>>>>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>>>>>> ---
>>>>>>  .../media/v4l/ext-ctrls-codec.rst             | 20 +++++++++++++++++++
>>>>>>  drivers/media/v4l2-core/v4l2-ctrls.c          |  9 +++++++++
>>>>>>  include/uapi/linux/v4l2-controls.h            |  1 +
>>>>>>  3 files changed, 30 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>>>> index 00944e97d638..994650052333 100644
>>>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>>>>> @@ -674,6 +674,26 @@ enum v4l2_mpeg_video_frame_skip_mode -
>>>>>>      is currently displayed (decoded). This value is reset to 0 whenever
>>>>>>      the decoder is started.
>>>>>>  
>>>>>> +``V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR (integer64)``
>>>>>> +    This control sets conceal color in YUV color space. It describes the
>>>>>> +    client preference of error conceal color in case of error where
>>>>>> +    reference frame is missing. The decoder would paint the reference
>>>>>> +    buffer with preferred color and use it for future decoding.
>>>>>> +    Applicable to decoders.
>>>>>
>>>>> You should mention explicitly that this is using 16-bit color components
>>>>> and expects Limited Range.
>>>>
>>>> I don't want to limit the client to Limited range only. I'll mention in
>>>> the description that both ranges are valid.
>>>
>>> OK, but then you need to describe what the color format depends on. See more
>>> below.
>>>
>>>>
>>>>>
>>>>>> +
>>>>>> +.. flat-table::
>>>>>> +    :header-rows:  0
>>>>>> +    :stub-columns: 0
>>>>>> +
>>>>>> +    * - Bit 0:15
>>>>>> +      - Y luminance
>>>>>> +    * - Bit 16:31
>>>>>> +      - Cb chrominance
>>>>>> +    * - Bit 32:47
>>>>>> +      - Cr chrominance
>>>>>> +    * - Bit 48:63
>>>>>> +      - Must be zero
>>>>>> +
>>
>> The table how the bits are spread into int64.
>>
>>>>>>  ``V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE (boolean)``
>>>>>>      If enabled the decoder expects to receive a single slice per buffer,
>>>>>>      otherwise the decoder expects a single frame in per buffer.
>>>>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>>>>>> index 016cf6204cbb..a3b9d28a00b7 100644
>>>>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>>>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>>>>> @@ -945,6 +945,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>>>>>  	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:			return "VBV Buffer Size";
>>>>>>  	case V4L2_CID_MPEG_VIDEO_DEC_PTS:			return "Video Decoder PTS";
>>>>>>  	case V4L2_CID_MPEG_VIDEO_DEC_FRAME:			return "Video Decoder Frame Count";
>>>>>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:		return "Video Decoder Conceal Color";
>>>>>>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
>>>>>>  	case V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE:		return "Horizontal MV Search Range";
>>>>>>  	case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:		return "Vertical MV Search Range";
>>>>>> @@ -1430,6 +1431,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>>>>>  		*max = 0x7fffffffffffffffLL;
>>>>>>  		*step = 1;
>>>>>>  		break;
>>>>>> +	case V4L2_CID_MPEG_VIDEO_DEC_CONCEAL_COLOR:
>>>>>> +		*type = V4L2_CTRL_TYPE_INTEGER64;
>>>>>> +		*min = 0;
>>>>>> +		/* default for 8bit black, luma is 16, chroma is 128 */
>>>>>
>>>>> Since this is 16 bit the actual default luma value for black is 4096 and for chroma use
>>>>> 32768 (i.e. both values are times 256).
>>>>
>>>> If we follow this for pixel format with 10bit per channel we have to
>>>> multiply by 64?
>>>
>>> No, you multiply by 4. 12 bit depth will multiple by 16, and 16 bit depth by 256.
>>>
>>> But how do you format this? Using bits 29-0? Or use 9-0 for one color component,
>>> 25-16 for another and 41-32 for the last component?
>>
>> I described this in the table above:
>>
>> Bit  0:15 - Y luminance
>> Bit 16:31 - Cb chrominance
>> Bit 32:47 - Cr chrominance
>> Bit 48:63 - Must be zero
>>
>> So depending on the bit depth of the current pixel format:
>>
>>  8bit - 0:7  Y', 16:23 Cb, 32:39 Cr
>> 10bit - 0:9  Y', 16:25 Cb, 32:41 Cr
>> 12bit - 0:11 Y', 16:27 Cb, 32:43 Cr

^^^^

> 
> Apologies, I missed that table!

Hans, do you want me to update with the above ^^^^ components bits or
something else?

-- 
regards,
Stan