[PATCH 0/3] HDR10 static metadata

[PATCH 0/3] HDR10 static metadata

[Stanimir Varbanov]

Hello,

This patchset adds two HDR10 HEVC v4l2 controls for Content Light Level
and Mastering display colour volume plus implenmentation in Venus encoder
driver.

Comments are welcome!

regards,
Stan

Stanimir Varbanov (3):
  v4l: Add HDR10 HEVC static metadata controls
  docs: media: Document CLL and Mastering display
  venus: venc: Add support for CLL and Mastering display controls

 .../media/v4l/ext-ctrls-codec.rst             | 61 +++++++++++++++++++
 drivers/media/platform/qcom/venus/core.h      |  3 +
 drivers/media/platform/qcom/venus/hfi_cmds.c  |  8 +++
 .../media/platform/qcom/venus/hfi_helper.h    | 20 ++++++
 drivers/media/platform/qcom/venus/venc.c      | 29 +++++++++
 .../media/platform/qcom/venus/venc_ctrls.c    | 16 ++++-
 drivers/media/v4l2-core/v4l2-ctrls.c          | 61 +++++++++++++++++++
 include/media/hevc-ctrls.h                    | 41 +++++++++++++
 include/media/v4l2-ctrls.h                    |  2 +
 9 files changed, 240 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH 1/3] v4l: Add HDR10 HEVC static metadata controls

[Stanimir Varbanov]

Add Content light level and Mastering display colour volume v4l2
compounf controls, relevant payload structures and validation.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 61 ++++++++++++++++++++++++++++
 include/media/hevc-ctrls.h           | 41 +++++++++++++++++++
 include/media/v4l2-ctrls.h           |  2 +
 3 files changed, 104 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index bd7f330c941c..f70eaa6a46df 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1023,6 +1023,8 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:		return "HEVC Slice Parameters";
 	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:		return "HEVC Decode Mode";
 	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:		return "HEVC Start Code";
+	case V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO:			return "HEVC Content Light Info";
+	case V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY:	return "HEVC Mastering Display";
 
 	/* CAMERA controls */
 	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
@@ -1461,6 +1463,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:
 		*type = V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS;
 		break;
+	case V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO:
+		*type = V4L2_CTRL_TYPE_HEVC_CLL_INFO;
+		break;
+	case V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY:
+		*type = V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY;
+		break;
 	case V4L2_CID_UNIT_CELL_SIZE:
 		*type = V4L2_CTRL_TYPE_AREA;
 		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
@@ -1775,6 +1783,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 	struct v4l2_ctrl_hevc_sps *p_hevc_sps;
 	struct v4l2_ctrl_hevc_pps *p_hevc_pps;
 	struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
+	struct v4l2_ctrl_hevc_mastering_display *p_hevc_mastering;
 	struct v4l2_area *area;
 	void *p = ptr.p + idx * ctrl->elem_size;
 	unsigned int i;
@@ -1934,6 +1943,52 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 		zero_padding(*p_hevc_slice_params);
 		break;
 
+	case V4L2_CTRL_TYPE_HEVC_CLL_INFO:
+		break;
+
+	case V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY:
+		p_hevc_mastering = p;
+
+		for (i = 0; i < 3; ++i) {
+			if (p_hevc_mastering->display_primaries_x[i] <
+				V4L2_HEVC_MASTERING_PRIMARIES_X_LOW ||
+			    p_hevc_mastering->display_primaries_x[i] >
+				V4L2_HEVC_MASTERING_PRIMARIES_X_HIGH ||
+			    p_hevc_mastering->display_primaries_y[i] <
+				V4L2_HEVC_MASTERING_PRIMARIES_Y_LOW ||
+			    p_hevc_mastering->display_primaries_y[i] >
+				V4L2_HEVC_MASTERING_PRIMARIES_Y_HIGH)
+				return -EINVAL;
+		}
+
+		if (p_hevc_mastering->white_point_x <
+			V4L2_HEVC_MASTERING_WHITE_POINT_X_LOW ||
+		    p_hevc_mastering->white_point_x >
+			V4L2_HEVC_MASTERING_WHITE_POINT_X_HIGH ||
+		    p_hevc_mastering->white_point_y <
+			V4L2_HEVC_MASTERING_WHITE_POINT_Y_LOW ||
+		    p_hevc_mastering->white_point_y >
+			V4L2_HEVC_MASTERING_WHITE_POINT_Y_HIGH)
+			return -EINVAL;
+
+		if (p_hevc_mastering->max_luminance <
+			V4L2_HEVC_MASTERING_MAX_LUMA_LOW ||
+		    p_hevc_mastering->max_luminance >
+			V4L2_HEVC_MASTERING_MAX_LUMA_HIGH ||
+		    p_hevc_mastering->min_luminance <
+			V4L2_HEVC_MASTERING_MIN_LUMA_LOW ||
+		    p_hevc_mastering->min_luminance >
+			V4L2_HEVC_MASTERING_MIN_LUMA_HIGH)
+			return -EINVAL;
+
+		if (p_hevc_mastering->max_luminance ==
+			V4L2_HEVC_MASTERING_MAX_LUMA_LOW &&
+		    p_hevc_mastering->min_luminance ==
+			V4L2_HEVC_MASTERING_MIN_LUMA_HIGH)
+			return -EINVAL;
+
+		break;
+
 	case V4L2_CTRL_TYPE_AREA:
 		area = p;
 		if (!area->width || !area->height)
@@ -2626,6 +2681,12 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS:
 		elem_size = sizeof(struct v4l2_ctrl_hevc_slice_params);
 		break;
+	case V4L2_CTRL_TYPE_HEVC_CLL_INFO:
+		elem_size = sizeof(struct v4l2_ctrl_hevc_cll_info);
+		break;
+	case V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY:
+		elem_size = sizeof(struct v4l2_ctrl_hevc_mastering_display);
+		break;
 	case V4L2_CTRL_TYPE_AREA:
 		elem_size = sizeof(struct v4l2_area);
 		break;
diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
index 1009cf0891cc..d254457d2846 100644
--- a/include/media/hevc-ctrls.h
+++ b/include/media/hevc-ctrls.h
@@ -209,4 +209,45 @@ struct v4l2_ctrl_hevc_slice_params {
 	__u64	flags;
 };
 
+/*
+ * Content light level information.
+ * Source Rec. ITU-T H.265 v7 (11/2019) HEVC; D.2.35
+ */
+#define V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO	(V4L2_CID_MPEG_BASE + 1017)
+#define V4L2_CTRL_TYPE_HEVC_CLL_INFO		0x0123
+
+struct v4l2_ctrl_hevc_cll_info {
+	__u16 max_content_light_level;
+	__u16 max_pic_average_light_level;
+};
+
+/*
+ * Mastering display colour volume.
+ * Source Rec. ITU-T H.265 v7 (11/2019) HEVC; D.2.28
+ */
+#define V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY (V4L2_CID_MPEG_BASE + 1018)
+#define V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY	0x0124
+
+#define V4L2_HEVC_MASTERING_PRIMARIES_X_LOW	5
+#define V4L2_HEVC_MASTERING_PRIMARIES_X_HIGH	37000
+#define V4L2_HEVC_MASTERING_PRIMARIES_Y_LOW	5
+#define V4L2_HEVC_MASTERING_PRIMARIES_Y_HIGH	42000
+#define V4L2_HEVC_MASTERING_WHITE_POINT_X_LOW	5
+#define V4L2_HEVC_MASTERING_WHITE_POINT_X_HIGH	37000
+#define V4L2_HEVC_MASTERING_WHITE_POINT_Y_LOW	5
+#define V4L2_HEVC_MASTERING_WHITE_POINT_Y_HIGH	42000
+#define V4L2_HEVC_MASTERING_MAX_LUMA_LOW	50000
+#define V4L2_HEVC_MASTERING_MAX_LUMA_HIGH	100000000
+#define V4L2_HEVC_MASTERING_MIN_LUMA_LOW	1
+#define V4L2_HEVC_MASTERING_MIN_LUMA_HIGH	50000
+
+struct v4l2_ctrl_hevc_mastering_display {
+	__u16 display_primaries_x[3];
+	__u16 display_primaries_y[3];
+	__u16 white_point_x;
+	__u16 white_point_y;
+	__u32 max_luminance;
+	__u32 min_luminance;
+};
+
 #endif
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index cb25f345e9ad..6120e29945e1 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -80,6 +80,8 @@ union v4l2_ctrl_ptr {
 	struct v4l2_ctrl_hevc_sps *p_hevc_sps;
 	struct v4l2_ctrl_hevc_pps *p_hevc_pps;
 	struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
+	struct v4l2_ctrl_hevc_cll_info *p_hevc_cll;
+	struct v4l2_ctrl_hevc_mastering_display *p_hevc_mastering;
 	struct v4l2_area *p_area;
 	void *p;
 	const void *p_const;
-- 
2.17.1

[PATCH 2/3] docs: media: Document CLL and Mastering display

[Stanimir Varbanov]

Document Content light level and Mastering display colour volume.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 .../media/v4l/ext-ctrls-codec.rst             | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index ce728c757eaf..39d0aab5ca3d 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -4382,3 +4382,64 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
       - Selecting this value specifies that HEVC slices are expected
         to be prefixed by Annex B start codes. According to :ref:`hevc`
         valid start codes can be 3-bytes 0x000001 or 4-bytes 0x00000001.
+
+``V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO (struct)``
+    The Content Light Level defines upper bounds for the nominal target
+    brightness light level of the pictures.
+
+.. c:type:: v4l2_ctrl_hevc_cll_info
+
+.. cssclass:: longtable
+
+.. flat-table:: struct v4l2_ctrl_hevc_cll_info
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u16
+      - ``max_content_light_level``
+      - An upper bound on the maximum light level among all individual
+        samples for the pictures of coded video sequence, cd/m2.
+    * - __u16
+      - ``max_pic_average_light_level``
+      - An upper bound on the maximum average light level among the
+        samples for any idividual picture of coded video sequence, cd/m2.
+
+``V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY (struct)``
+    The mastering display defines the colour volume (the colour primaries,
+    white point and luminance range) of a display considered to be the
+    mastering display for current video content.
+
+.. c:type:: v4l2_ctrl_hevc_mastering_display
+
+.. cssclass:: longtable
+
+.. flat-table:: struct v4l2_ctrl_hevc_mastering_display
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 1 2
+
+    * - __u16
+      - ``display_primaries_x[3]``
+      - Specifies the normalized x chromaticity coordinate of the colour
+        primary component of the mastering display.
+    * - __u16
+      - ``display_primaries_y[3]``
+      - Specifies the normalized y chromaticity coordinate of the colour
+        primary component of the mastering display.
+    * - __u16
+      - ``white_point_x``
+      - Specifies the normalized x chromaticity coordinate of the white
+        point of the mastering display.
+    * - __u16
+      - ``white_point_y``
+      - Specifies the normalized y chromaticity coordinate of the white
+        point of the mastering display.
+    * - __u32
+      - ``max_luminance``
+      - Specifies the nominal maximum display luminance of the mastering
+        display.
+    * - __u32
+      - ``min_luminance``
+      - specifies the nominal minimum display luminance of the mastering
+        display.
-- 
2.17.1

[PATCH 3/3] venus: venc: Add support for CLL and Mastering display controls

[Stanimir Varbanov]

Create CLL and Mastering display colour volume v4l2 controls for
encoder, add handling of HDR10 PQ SEI packet payloads for v4.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/core.h      |  3 ++
 drivers/media/platform/qcom/venus/hfi_cmds.c  |  8 +++++
 .../media/platform/qcom/venus/hfi_helper.h    | 20 +++++++++++++
 drivers/media/platform/qcom/venus/venc.c      | 29 +++++++++++++++++++
 .../media/platform/qcom/venus/venc_ctrls.c    | 16 +++++++++-
 5 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 7b79a33dc9d6..9d2637983927 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -245,6 +245,9 @@ struct venc_controls {
 
 	u32 profile;
 	u32 level;
+
+	struct v4l2_ctrl_hevc_cll_info cll;
+	struct v4l2_ctrl_hevc_mastering_display mastering;
 };
 
 struct venus_buffer {
diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
index 7022368c1e63..081e5a816bca 100644
--- a/drivers/media/platform/qcom/venus/hfi_cmds.c
+++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
@@ -1205,6 +1205,14 @@ pkt_session_set_property_4xx(struct hfi_session_set_property_pkt *pkt,
 		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*cu);
 		break;
 	}
+	case HFI_PROPERTY_PARAM_VENC_HDR10_PQ_SEI: {
+		struct hfi_hdr10_pq_sei *in = pdata, *hdr10 = prop_data;
+
+		memcpy(hdr10, in, sizeof(*hdr10));
+		pkt->shdr.hdr.size += sizeof(u32) + sizeof(*hdr10);
+		break;
+	}
+
 	case HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE:
 	case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER:
 	case HFI_PROPERTY_PARAM_BUFFER_ALLOC_MODE:
diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h
index 60ee2479f7a6..8e8dc6b5c855 100644
--- a/drivers/media/platform/qcom/venus/hfi_helper.h
+++ b/drivers/media/platform/qcom/venus/hfi_helper.h
@@ -506,6 +506,7 @@
 #define HFI_PROPERTY_PARAM_VENC_VPX_ERROR_RESILIENCE_MODE	0x2005029
 #define HFI_PROPERTY_PARAM_VENC_HIER_B_MAX_NUM_ENH_LAYER	0x200502c
 #define HFI_PROPERTY_PARAM_VENC_HIER_P_HYBRID_MODE		0x200502f
+#define HFI_PROPERTY_PARAM_VENC_HDR10_PQ_SEI			0x2005036
 
 /*
  * HFI_PROPERTY_CONFIG_VENC_COMMON_START
@@ -791,6 +792,25 @@ struct hfi_ltr_mark {
 	u32 mark_frame;
 };
 
+struct hfi_mastering_display_colour_sei_payload {
+	u32 display_primaries_x[3];
+	u32 display_primaries_y[3];
+	u32 white_point_x;
+	u32 white_point_y;
+	u32 max_display_mastering_luminance;
+	u32 min_display_mastering_luminance;
+};
+
+struct hfi_content_light_level_sei_payload {
+	u32 max_content_light;
+	u32 max_pic_average_light;
+};
+
+struct hfi_hdr10_pq_sei {
+	struct hfi_mastering_display_colour_sei_payload mastering;
+	struct hfi_content_light_level_sei_payload cll;
+};
+
 struct hfi_framesize {
 	u32 buffer_type;
 	u32 width;
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index f8b1484e7dcd..ae593a6a5a22 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -587,6 +587,35 @@ static int venc_set_properties(struct venus_inst *inst)
 			return ret;
 	}
 
+	if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_HEVC) {
+		struct hfi_hdr10_pq_sei hdr10;
+		unsigned int c;
+
+		ptype = HFI_PROPERTY_PARAM_VENC_HDR10_PQ_SEI;
+
+		for (c = 0; c < 3; c++) {
+			hdr10.mastering.display_primaries_x[c] =
+				ctr->mastering.display_primaries_x[c];
+			hdr10.mastering.display_primaries_y[c] =
+				ctr->mastering.display_primaries_y[c];
+		}
+
+		hdr10.mastering.white_point_x = ctr->mastering.white_point_x;
+		hdr10.mastering.white_point_y = ctr->mastering.white_point_y;
+		hdr10.mastering.max_display_mastering_luminance =
+			ctr->mastering.max_luminance;
+		hdr10.mastering.min_display_mastering_luminance =
+			ctr->mastering.min_luminance;
+
+		hdr10.cll.max_content_light = ctr->cll.max_content_light_level;
+		hdr10.cll.max_pic_average_light =
+			ctr->cll.max_pic_average_light_level;
+
+		ret = hfi_session_set_property(inst, ptype, &hdr10);
+		if (ret)
+			return ret;
+	}
+
 	/* IDR periodicity, n:
 	 * n = 0 - only the first I-frame is IDR frame
 	 * n = 1 - all I-frames will be IDR frames
diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
index 0708b3b89d0c..dab1286cbb1a 100644
--- a/drivers/media/platform/qcom/venus/venc_ctrls.c
+++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
@@ -198,6 +198,12 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_MPEG_VIDEO_FRAME_SKIP_MODE:
 		ctr->frame_skip_mode = ctrl->val;
 		break;
+	case V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO:
+		ctr->cll = *ctrl->p_new.p_hevc_cll;
+		break;
+	case V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY:
+		ctr->mastering = *ctrl->p_new.p_hevc_mastering;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -213,7 +219,7 @@ int venc_ctrl_init(struct venus_inst *inst)
 {
 	int ret;
 
-	ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 33);
+	ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 35);
 	if (ret)
 		return ret;
 
@@ -364,6 +370,14 @@ int venc_ctrl_init(struct venus_inst *inst)
 			       (1 << V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT)),
 			       V4L2_MPEG_VIDEO_FRAME_SKIP_MODE_DISABLED);
 
+	v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops,
+				   V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO,
+				   v4l2_ctrl_ptr_create(NULL));
+
+	v4l2_ctrl_new_std_compound(&inst->ctrl_handler, &venc_ctrl_ops,
+				   V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY,
+				   v4l2_ctrl_ptr_create(NULL));
+
 	ret = inst->ctrl_handler.error;
 	if (ret)
 		goto err;
-- 
2.17.1

Re: [PATCH 0/3] HDR10 static metadata

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]

Le lundi 09 novembre 2020 à 19:31 +0200, Stanimir Varbanov a écrit :
> Hello,
> 
> This patchset adds two HDR10 HEVC v4l2 controls for Content Light Level
> and Mastering display colour volume plus implenmentation in Venus encoder
> driver.
> 
> Comments are welcome!

It is not a formal review, but I did walked through the new API and
everything looks fine to me. One question though, are you aware that
the H.264/AVC equivalent is identical ? What is you plan for that ?

> 
> regards,
> Stan
> 
> Stanimir Varbanov (3):
>   v4l: Add HDR10 HEVC static metadata controls
>   docs: media: Document CLL and Mastering display
>   venus: venc: Add support for CLL and Mastering display controls
> 
>  .../media/v4l/ext-ctrls-codec.rst             | 61 +++++++++++++++++++
>  drivers/media/platform/qcom/venus/core.h      |  3 +
>  drivers/media/platform/qcom/venus/hfi_cmds.c  |  8 +++
>  .../media/platform/qcom/venus/hfi_helper.h    | 20 ++++++
>  drivers/media/platform/qcom/venus/venc.c      | 29 +++++++++
>  .../media/platform/qcom/venus/venc_ctrls.c    | 16 ++++-
>  drivers/media/v4l2-core/v4l2-ctrls.c          | 61 +++++++++++++++++++
>  include/media/hevc-ctrls.h                    | 41 +++++++++++++
>  include/media/v4l2-ctrls.h                    |  2 +
>  9 files changed, 240 insertions(+), 1 deletion(-)
> 


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 0/3] HDR10 static metadata

[Stanimir Varbanov]

On 11/9/20 9:53 PM, Nicolas Dufresne wrote:
> Le lundi 09 novembre 2020 à 19:31 +0200, Stanimir Varbanov a écrit :
>> Hello,
>>
>> This patchset adds two HDR10 HEVC v4l2 controls for Content Light Level
>> and Mastering display colour volume plus implenmentation in Venus encoder
>> driver.
>>
>> Comments are welcome!
> 
> It is not a formal review, but I did walked through the new API and
> everything looks fine to me. One question though, are you aware that
> the H.264/AVC equivalent is identical ? What is you plan for that ?

Thanks for the question, I haven't thought for avc, yet.

I guess we have few options:

1. introduce hdr10-ctrls.h, common control IDs
2. duplicate structures and control IDs in hevc/h264-ctrls.h
3. common structures and separate control IDs for avc and hevc
4. another option?

I'd prefer option 1. We could extend later option 1 with hdr10+.

> 
>>
>> regards,
>> Stan
>>
>> Stanimir Varbanov (3):
>>   v4l: Add HDR10 HEVC static metadata controls
>>   docs: media: Document CLL and Mastering display
>>   venus: venc: Add support for CLL and Mastering display controls
>>
>>  .../media/v4l/ext-ctrls-codec.rst             | 61 +++++++++++++++++++
>>  drivers/media/platform/qcom/venus/core.h      |  3 +
>>  drivers/media/platform/qcom/venus/hfi_cmds.c  |  8 +++
>>  .../media/platform/qcom/venus/hfi_helper.h    | 20 ++++++
>>  drivers/media/platform/qcom/venus/venc.c      | 29 +++++++++
>>  .../media/platform/qcom/venus/venc_ctrls.c    | 16 ++++-
>>  drivers/media/v4l2-core/v4l2-ctrls.c          | 61 +++++++++++++++++++
>>  include/media/hevc-ctrls.h                    | 41 +++++++++++++
>>  include/media/v4l2-ctrls.h                    |  2 +
>>  9 files changed, 240 insertions(+), 1 deletion(-)
>>
> 

-- 
regards,
Stan

Re: [PATCH 0/3] HDR10 static metadata

[Hans Verkuil]

On 09/11/2020 20:53, Nicolas Dufresne wrote:
> Le lundi 09 novembre 2020 à 19:31 +0200, Stanimir Varbanov a écrit :
>> Hello,
>>
>> This patchset adds two HDR10 HEVC v4l2 controls for Content Light Level
>> and Mastering display colour volume plus implenmentation in Venus encoder
>> driver.
>>
>> Comments are welcome!
> 
> It is not a formal review, but I did walked through the new API and
> everything looks fine to me. One question though, are you aware that
> the H.264/AVC equivalent is identical ? What is you plan for that ?

Not only that, but these structures are lifted straight from the
CTA-861-G standard: see "6.9 Dynamic Range and Mastering InfoFrame"
and "6.9.1 Static Metadata Type 1".

So this is equally useful for HDMI receivers and transmitters.

Actually, include/linux/hdmi.h contains a struct for that, but it seems
to be missing a lot of fields. But we need a v4l2 control anyway and hdmi.h
isn't a good fit for that.

Regards,

	Hans

> 
>>
>> regards,
>> Stan
>>
>> Stanimir Varbanov (3):
>>   v4l: Add HDR10 HEVC static metadata controls
>>   docs: media: Document CLL and Mastering display
>>   venus: venc: Add support for CLL and Mastering display controls
>>
>>  .../media/v4l/ext-ctrls-codec.rst             | 61 +++++++++++++++++++
>>  drivers/media/platform/qcom/venus/core.h      |  3 +
>>  drivers/media/platform/qcom/venus/hfi_cmds.c  |  8 +++
>>  .../media/platform/qcom/venus/hfi_helper.h    | 20 ++++++
>>  drivers/media/platform/qcom/venus/venc.c      | 29 +++++++++
>>  .../media/platform/qcom/venus/venc_ctrls.c    | 16 ++++-
>>  drivers/media/v4l2-core/v4l2-ctrls.c          | 61 +++++++++++++++++++
>>  include/media/hevc-ctrls.h                    | 41 +++++++++++++
>>  include/media/v4l2-ctrls.h                    |  2 +
>>  9 files changed, 240 insertions(+), 1 deletion(-)
>>
> 

Re: [PATCH 0/3] HDR10 static metadata

[Hans Verkuil]

On 10/11/2020 10:38, Hans Verkuil wrote:
> On 09/11/2020 20:53, Nicolas Dufresne wrote:
>> Le lundi 09 novembre 2020 à 19:31 +0200, Stanimir Varbanov a écrit :
>>> Hello,
>>>
>>> This patchset adds two HDR10 HEVC v4l2 controls for Content Light Level
>>> and Mastering display colour volume plus implenmentation in Venus encoder
>>> driver.
>>>
>>> Comments are welcome!
>>
>> It is not a formal review, but I did walked through the new API and
>> everything looks fine to me. One question though, are you aware that
>> the H.264/AVC equivalent is identical ? What is you plan for that ?
> 
> Not only that, but these structures are lifted straight from the
> CTA-861-G standard: see "6.9 Dynamic Range and Mastering InfoFrame"
> and "6.9.1 Static Metadata Type 1".

Correction: the CTA-861-G lifted it from SMPTE ST-2086 in turn.

Regards,

	Hans

> 
> So this is equally useful for HDMI receivers and transmitters.
> 
> Actually, include/linux/hdmi.h contains a struct for that, but it seems
> to be missing a lot of fields. But we need a v4l2 control anyway and hdmi.h
> isn't a good fit for that.
> 
> Regards,
> 
> 	Hans
> 
>>
>>>
>>> regards,
>>> Stan
>>>
>>> Stanimir Varbanov (3):
>>>   v4l: Add HDR10 HEVC static metadata controls
>>>   docs: media: Document CLL and Mastering display
>>>   venus: venc: Add support for CLL and Mastering display controls
>>>
>>>  .../media/v4l/ext-ctrls-codec.rst             | 61 +++++++++++++++++++
>>>  drivers/media/platform/qcom/venus/core.h      |  3 +
>>>  drivers/media/platform/qcom/venus/hfi_cmds.c  |  8 +++
>>>  .../media/platform/qcom/venus/hfi_helper.h    | 20 ++++++
>>>  drivers/media/platform/qcom/venus/venc.c      | 29 +++++++++
>>>  .../media/platform/qcom/venus/venc_ctrls.c    | 16 ++++-
>>>  drivers/media/v4l2-core/v4l2-ctrls.c          | 61 +++++++++++++++++++
>>>  include/media/hevc-ctrls.h                    | 41 +++++++++++++
>>>  include/media/v4l2-ctrls.h                    |  2 +
>>>  9 files changed, 240 insertions(+), 1 deletion(-)
>>>
>>
> 

Re: [PATCH 1/3] v4l: Add HDR10 HEVC static metadata controls

[Hans Verkuil]

On 09/11/2020 18:31, Stanimir Varbanov wrote:
> Add Content light level and Mastering display colour volume v4l2
> compounf controls, relevant payload structures and validation.

Typo: compounf -> compound

> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 61 ++++++++++++++++++++++++++++
>  include/media/hevc-ctrls.h           | 41 +++++++++++++++++++
>  include/media/v4l2-ctrls.h           |  2 +
>  3 files changed, 104 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index bd7f330c941c..f70eaa6a46df 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1023,6 +1023,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:		return "HEVC Slice Parameters";
>  	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:		return "HEVC Decode Mode";
>  	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:		return "HEVC Start Code";
> +	case V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO:			return "HEVC Content Light Info";
> +	case V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY:	return "HEVC Mastering Display";

Why is this split up in two controls? Can you have one, but not the other?

From what I can tell they are always combined (see CTA-861-G, SMPTE 2086).

Regards,

	Hans

>  
>  	/* CAMERA controls */
>  	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1461,6 +1463,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:
>  		*type = V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS;
>  		break;
> +	case V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO:
> +		*type = V4L2_CTRL_TYPE_HEVC_CLL_INFO;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY:
> +		*type = V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY;
> +		break;
>  	case V4L2_CID_UNIT_CELL_SIZE:
>  		*type = V4L2_CTRL_TYPE_AREA;
>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
> @@ -1775,6 +1783,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>  	struct v4l2_ctrl_hevc_sps *p_hevc_sps;
>  	struct v4l2_ctrl_hevc_pps *p_hevc_pps;
>  	struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
> +	struct v4l2_ctrl_hevc_mastering_display *p_hevc_mastering;
>  	struct v4l2_area *area;
>  	void *p = ptr.p + idx * ctrl->elem_size;
>  	unsigned int i;
> @@ -1934,6 +1943,52 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>  		zero_padding(*p_hevc_slice_params);
>  		break;
>  
> +	case V4L2_CTRL_TYPE_HEVC_CLL_INFO:
> +		break;
> +
> +	case V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY:
> +		p_hevc_mastering = p;
> +
> +		for (i = 0; i < 3; ++i) {
> +			if (p_hevc_mastering->display_primaries_x[i] <
> +				V4L2_HEVC_MASTERING_PRIMARIES_X_LOW ||
> +			    p_hevc_mastering->display_primaries_x[i] >
> +				V4L2_HEVC_MASTERING_PRIMARIES_X_HIGH ||
> +			    p_hevc_mastering->display_primaries_y[i] <
> +				V4L2_HEVC_MASTERING_PRIMARIES_Y_LOW ||
> +			    p_hevc_mastering->display_primaries_y[i] >
> +				V4L2_HEVC_MASTERING_PRIMARIES_Y_HIGH)
> +				return -EINVAL;
> +		}
> +
> +		if (p_hevc_mastering->white_point_x <
> +			V4L2_HEVC_MASTERING_WHITE_POINT_X_LOW ||
> +		    p_hevc_mastering->white_point_x >
> +			V4L2_HEVC_MASTERING_WHITE_POINT_X_HIGH ||
> +		    p_hevc_mastering->white_point_y <
> +			V4L2_HEVC_MASTERING_WHITE_POINT_Y_LOW ||
> +		    p_hevc_mastering->white_point_y >
> +			V4L2_HEVC_MASTERING_WHITE_POINT_Y_HIGH)
> +			return -EINVAL;
> +
> +		if (p_hevc_mastering->max_luminance <
> +			V4L2_HEVC_MASTERING_MAX_LUMA_LOW ||
> +		    p_hevc_mastering->max_luminance >
> +			V4L2_HEVC_MASTERING_MAX_LUMA_HIGH ||
> +		    p_hevc_mastering->min_luminance <
> +			V4L2_HEVC_MASTERING_MIN_LUMA_LOW ||
> +		    p_hevc_mastering->min_luminance >
> +			V4L2_HEVC_MASTERING_MIN_LUMA_HIGH)
> +			return -EINVAL;
> +
> +		if (p_hevc_mastering->max_luminance ==
> +			V4L2_HEVC_MASTERING_MAX_LUMA_LOW &&
> +		    p_hevc_mastering->min_luminance ==
> +			V4L2_HEVC_MASTERING_MIN_LUMA_HIGH)
> +			return -EINVAL;
> +
> +		break;
> +
>  	case V4L2_CTRL_TYPE_AREA:
>  		area = p;
>  		if (!area->width || !area->height)
> @@ -2626,6 +2681,12 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
>  	case V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS:
>  		elem_size = sizeof(struct v4l2_ctrl_hevc_slice_params);
>  		break;
> +	case V4L2_CTRL_TYPE_HEVC_CLL_INFO:
> +		elem_size = sizeof(struct v4l2_ctrl_hevc_cll_info);
> +		break;
> +	case V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY:
> +		elem_size = sizeof(struct v4l2_ctrl_hevc_mastering_display);
> +		break;
>  	case V4L2_CTRL_TYPE_AREA:
>  		elem_size = sizeof(struct v4l2_area);
>  		break;
> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
> index 1009cf0891cc..d254457d2846 100644
> --- a/include/media/hevc-ctrls.h
> +++ b/include/media/hevc-ctrls.h
> @@ -209,4 +209,45 @@ struct v4l2_ctrl_hevc_slice_params {
>  	__u64	flags;
>  };
>  
> +/*
> + * Content light level information.
> + * Source Rec. ITU-T H.265 v7 (11/2019) HEVC; D.2.35
> + */
> +#define V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO	(V4L2_CID_MPEG_BASE + 1017)
> +#define V4L2_CTRL_TYPE_HEVC_CLL_INFO		0x0123
> +
> +struct v4l2_ctrl_hevc_cll_info {
> +	__u16 max_content_light_level;
> +	__u16 max_pic_average_light_level;
> +};
> +
> +/*
> + * Mastering display colour volume.
> + * Source Rec. ITU-T H.265 v7 (11/2019) HEVC; D.2.28
> + */
> +#define V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY (V4L2_CID_MPEG_BASE + 1018)
> +#define V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY	0x0124
> +
> +#define V4L2_HEVC_MASTERING_PRIMARIES_X_LOW	5
> +#define V4L2_HEVC_MASTERING_PRIMARIES_X_HIGH	37000
> +#define V4L2_HEVC_MASTERING_PRIMARIES_Y_LOW	5
> +#define V4L2_HEVC_MASTERING_PRIMARIES_Y_HIGH	42000
> +#define V4L2_HEVC_MASTERING_WHITE_POINT_X_LOW	5
> +#define V4L2_HEVC_MASTERING_WHITE_POINT_X_HIGH	37000
> +#define V4L2_HEVC_MASTERING_WHITE_POINT_Y_LOW	5
> +#define V4L2_HEVC_MASTERING_WHITE_POINT_Y_HIGH	42000
> +#define V4L2_HEVC_MASTERING_MAX_LUMA_LOW	50000
> +#define V4L2_HEVC_MASTERING_MAX_LUMA_HIGH	100000000
> +#define V4L2_HEVC_MASTERING_MIN_LUMA_LOW	1
> +#define V4L2_HEVC_MASTERING_MIN_LUMA_HIGH	50000
> +
> +struct v4l2_ctrl_hevc_mastering_display {
> +	__u16 display_primaries_x[3];
> +	__u16 display_primaries_y[3];
> +	__u16 white_point_x;
> +	__u16 white_point_y;
> +	__u32 max_luminance;
> +	__u32 min_luminance;
> +};
> +
>  #endif
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index cb25f345e9ad..6120e29945e1 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -80,6 +80,8 @@ union v4l2_ctrl_ptr {
>  	struct v4l2_ctrl_hevc_sps *p_hevc_sps;
>  	struct v4l2_ctrl_hevc_pps *p_hevc_pps;
>  	struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
> +	struct v4l2_ctrl_hevc_cll_info *p_hevc_cll;
> +	struct v4l2_ctrl_hevc_mastering_display *p_hevc_mastering;
>  	struct v4l2_area *p_area;
>  	void *p;
>  	const void *p_const;
> 

Re: [PATCH 2/3] docs: media: Document CLL and Mastering display

[Hans Verkuil]

On 09/11/2020 18:31, Stanimir Varbanov wrote:
> Document Content light level and Mastering display colour volume.
> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  .../media/v4l/ext-ctrls-codec.rst             | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index ce728c757eaf..39d0aab5ca3d 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -4382,3 +4382,64 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>        - Selecting this value specifies that HEVC slices are expected
>          to be prefixed by Annex B start codes. According to :ref:`hevc`
>          valid start codes can be 3-bytes 0x000001 or 4-bytes 0x00000001.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO (struct)``
> +    The Content Light Level defines upper bounds for the nominal target
> +    brightness light level of the pictures.
> +
> +.. c:type:: v4l2_ctrl_hevc_cll_info
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_hevc_cll_info
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 1 2
> +
> +    * - __u16
> +      - ``max_content_light_level``
> +      - An upper bound on the maximum light level among all individual
> +        samples for the pictures of coded video sequence, cd/m2.
> +    * - __u16
> +      - ``max_pic_average_light_level``
> +      - An upper bound on the maximum average light level among the
> +        samples for any idividual picture of coded video sequence, cd/m2.

idividual -> individual

In the CTA-861-G spec value 0 is used to indicate that this information is
not present. How is that handled here? Can it be 0 as well in an HEVC stream?

Same for the next control.

> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY (struct)``
> +    The mastering display defines the colour volume (the colour primaries,
> +    white point and luminance range) of a display considered to be the
> +    mastering display for current video content.
> +
> +.. c:type:: v4l2_ctrl_hevc_mastering_display
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_hevc_mastering_display
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 1 2
> +
> +    * - __u16
> +      - ``display_primaries_x[3]``
> +      - Specifies the normalized x chromaticity coordinate of the colour
> +        primary component of the mastering display.

CTA-861-G defines this as: "coded as unsigned 16-bit values in units
of 0.00002, where 0x0000 represents zero and 0xC350 represents 1.0000."

Is that true here as well? If so, then this should be documented because
"normalized x chromaticity coordinate" doesn't say anything meaningful.

> +    * - __u16
> +      - ``display_primaries_y[3]``
> +      - Specifies the normalized y chromaticity coordinate of the colour
> +        primary component of the mastering display.
> +    * - __u16
> +      - ``white_point_x``
> +      - Specifies the normalized x chromaticity coordinate of the white
> +        point of the mastering display.
> +    * - __u16
> +      - ``white_point_y``
> +      - Specifies the normalized y chromaticity coordinate of the white
> +        point of the mastering display.
> +    * - __u32
> +      - ``max_luminance``
> +      - Specifies the nominal maximum display luminance of the mastering
> +        display.

In CTA-861-G this is in 1 cd/m^2 units.

> +    * - __u32
> +      - ``min_luminance``
> +      - specifies the nominal minimum display luminance of the mastering
> +        display.

And this in units of 0.0001 cd/m^2.

Regards,

	Hans

Re: [PATCH 1/3] v4l: Add HDR10 HEVC static metadata controls

[Stanimir Varbanov]

On 11/10/20 11:43 AM, Hans Verkuil wrote:
> On 09/11/2020 18:31, Stanimir Varbanov wrote:
>> Add Content light level and Mastering display colour volume v4l2
>> compounf controls, relevant payload structures and validation.
> 
> Typo: compounf -> compound
> 
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>>  drivers/media/v4l2-core/v4l2-ctrls.c | 61 ++++++++++++++++++++++++++++
>>  include/media/hevc-ctrls.h           | 41 +++++++++++++++++++
>>  include/media/v4l2-ctrls.h           |  2 +
>>  3 files changed, 104 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index bd7f330c941c..f70eaa6a46df 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -1023,6 +1023,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:		return "HEVC Slice Parameters";
>>  	case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:		return "HEVC Decode Mode";
>>  	case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:		return "HEVC Start Code";
>> +	case V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO:			return "HEVC Content Light Info";
>> +	case V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY:	return "HEVC Mastering Display";
> 
> Why is this split up in two controls? Can you have one, but not the other?
> 
> From what I can tell they are always combined (see CTA-861-G, SMPTE 2086).

I split to two control IDs because in ITU-T Rec. H265 CLL and Mastering
Display colour volume are different SEI messages. I guessed that they
could exist in the bitstream independently, though I'm not sure about that.

I think, if we decide that hdr10-ctrls.h will be better place for these
controls we can combine CLL and Mastering display in one control -
V4L2_CID_MPEG_HDR10_STATIC_METADATA.
And later we could introduce V4L2_CID_MPEG_HDR10_DYNAMIC_METADATA for
hdr10+ (2094-40).

> 
> Regards,
> 
> 	Hans
> 
>>  
>>  	/* CAMERA controls */
>>  	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>> @@ -1461,6 +1463,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>  	case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:
>>  		*type = V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS;
>>  		break;
>> +	case V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO:
>> +		*type = V4L2_CTRL_TYPE_HEVC_CLL_INFO;
>> +		break;
>> +	case V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY:
>> +		*type = V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY;
>> +		break;
>>  	case V4L2_CID_UNIT_CELL_SIZE:
>>  		*type = V4L2_CTRL_TYPE_AREA;
>>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
>> @@ -1775,6 +1783,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>>  	struct v4l2_ctrl_hevc_sps *p_hevc_sps;
>>  	struct v4l2_ctrl_hevc_pps *p_hevc_pps;
>>  	struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
>> +	struct v4l2_ctrl_hevc_mastering_display *p_hevc_mastering;
>>  	struct v4l2_area *area;
>>  	void *p = ptr.p + idx * ctrl->elem_size;
>>  	unsigned int i;
>> @@ -1934,6 +1943,52 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>>  		zero_padding(*p_hevc_slice_params);
>>  		break;
>>  
>> +	case V4L2_CTRL_TYPE_HEVC_CLL_INFO:
>> +		break;
>> +
>> +	case V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY:
>> +		p_hevc_mastering = p;
>> +
>> +		for (i = 0; i < 3; ++i) {
>> +			if (p_hevc_mastering->display_primaries_x[i] <
>> +				V4L2_HEVC_MASTERING_PRIMARIES_X_LOW ||
>> +			    p_hevc_mastering->display_primaries_x[i] >
>> +				V4L2_HEVC_MASTERING_PRIMARIES_X_HIGH ||
>> +			    p_hevc_mastering->display_primaries_y[i] <
>> +				V4L2_HEVC_MASTERING_PRIMARIES_Y_LOW ||
>> +			    p_hevc_mastering->display_primaries_y[i] >
>> +				V4L2_HEVC_MASTERING_PRIMARIES_Y_HIGH)
>> +				return -EINVAL;
>> +		}
>> +
>> +		if (p_hevc_mastering->white_point_x <
>> +			V4L2_HEVC_MASTERING_WHITE_POINT_X_LOW ||
>> +		    p_hevc_mastering->white_point_x >
>> +			V4L2_HEVC_MASTERING_WHITE_POINT_X_HIGH ||
>> +		    p_hevc_mastering->white_point_y <
>> +			V4L2_HEVC_MASTERING_WHITE_POINT_Y_LOW ||
>> +		    p_hevc_mastering->white_point_y >
>> +			V4L2_HEVC_MASTERING_WHITE_POINT_Y_HIGH)
>> +			return -EINVAL;
>> +
>> +		if (p_hevc_mastering->max_luminance <
>> +			V4L2_HEVC_MASTERING_MAX_LUMA_LOW ||
>> +		    p_hevc_mastering->max_luminance >
>> +			V4L2_HEVC_MASTERING_MAX_LUMA_HIGH ||
>> +		    p_hevc_mastering->min_luminance <
>> +			V4L2_HEVC_MASTERING_MIN_LUMA_LOW ||
>> +		    p_hevc_mastering->min_luminance >
>> +			V4L2_HEVC_MASTERING_MIN_LUMA_HIGH)
>> +			return -EINVAL;
>> +
>> +		if (p_hevc_mastering->max_luminance ==
>> +			V4L2_HEVC_MASTERING_MAX_LUMA_LOW &&
>> +		    p_hevc_mastering->min_luminance ==
>> +			V4L2_HEVC_MASTERING_MIN_LUMA_HIGH)
>> +			return -EINVAL;
>> +
>> +		break;
>> +
>>  	case V4L2_CTRL_TYPE_AREA:
>>  		area = p;
>>  		if (!area->width || !area->height)
>> @@ -2626,6 +2681,12 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
>>  	case V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS:
>>  		elem_size = sizeof(struct v4l2_ctrl_hevc_slice_params);
>>  		break;
>> +	case V4L2_CTRL_TYPE_HEVC_CLL_INFO:
>> +		elem_size = sizeof(struct v4l2_ctrl_hevc_cll_info);
>> +		break;
>> +	case V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY:
>> +		elem_size = sizeof(struct v4l2_ctrl_hevc_mastering_display);
>> +		break;
>>  	case V4L2_CTRL_TYPE_AREA:
>>  		elem_size = sizeof(struct v4l2_area);
>>  		break;
>> diff --git a/include/media/hevc-ctrls.h b/include/media/hevc-ctrls.h
>> index 1009cf0891cc..d254457d2846 100644
>> --- a/include/media/hevc-ctrls.h
>> +++ b/include/media/hevc-ctrls.h
>> @@ -209,4 +209,45 @@ struct v4l2_ctrl_hevc_slice_params {
>>  	__u64	flags;
>>  };
>>  
>> +/*
>> + * Content light level information.
>> + * Source Rec. ITU-T H.265 v7 (11/2019) HEVC; D.2.35
>> + */
>> +#define V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO	(V4L2_CID_MPEG_BASE + 1017)
>> +#define V4L2_CTRL_TYPE_HEVC_CLL_INFO		0x0123
>> +
>> +struct v4l2_ctrl_hevc_cll_info {
>> +	__u16 max_content_light_level;
>> +	__u16 max_pic_average_light_level;
>> +};
>> +
>> +/*
>> + * Mastering display colour volume.
>> + * Source Rec. ITU-T H.265 v7 (11/2019) HEVC; D.2.28
>> + */
>> +#define V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY (V4L2_CID_MPEG_BASE + 1018)
>> +#define V4L2_CTRL_TYPE_HEVC_MASTERING_DISPLAY	0x0124
>> +
>> +#define V4L2_HEVC_MASTERING_PRIMARIES_X_LOW	5
>> +#define V4L2_HEVC_MASTERING_PRIMARIES_X_HIGH	37000
>> +#define V4L2_HEVC_MASTERING_PRIMARIES_Y_LOW	5
>> +#define V4L2_HEVC_MASTERING_PRIMARIES_Y_HIGH	42000
>> +#define V4L2_HEVC_MASTERING_WHITE_POINT_X_LOW	5
>> +#define V4L2_HEVC_MASTERING_WHITE_POINT_X_HIGH	37000
>> +#define V4L2_HEVC_MASTERING_WHITE_POINT_Y_LOW	5
>> +#define V4L2_HEVC_MASTERING_WHITE_POINT_Y_HIGH	42000
>> +#define V4L2_HEVC_MASTERING_MAX_LUMA_LOW	50000
>> +#define V4L2_HEVC_MASTERING_MAX_LUMA_HIGH	100000000
>> +#define V4L2_HEVC_MASTERING_MIN_LUMA_LOW	1
>> +#define V4L2_HEVC_MASTERING_MIN_LUMA_HIGH	50000
>> +
>> +struct v4l2_ctrl_hevc_mastering_display {
>> +	__u16 display_primaries_x[3];
>> +	__u16 display_primaries_y[3];
>> +	__u16 white_point_x;
>> +	__u16 white_point_y;
>> +	__u32 max_luminance;
>> +	__u32 min_luminance;
>> +};
>> +
>>  #endif
>> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
>> index cb25f345e9ad..6120e29945e1 100644
>> --- a/include/media/v4l2-ctrls.h
>> +++ b/include/media/v4l2-ctrls.h
>> @@ -80,6 +80,8 @@ union v4l2_ctrl_ptr {
>>  	struct v4l2_ctrl_hevc_sps *p_hevc_sps;
>>  	struct v4l2_ctrl_hevc_pps *p_hevc_pps;
>>  	struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
>> +	struct v4l2_ctrl_hevc_cll_info *p_hevc_cll;
>> +	struct v4l2_ctrl_hevc_mastering_display *p_hevc_mastering;
>>  	struct v4l2_area *p_area;
>>  	void *p;
>>  	const void *p_const;
>>
> 

-- 
regards,
Stan

Re: [PATCH 2/3] docs: media: Document CLL and Mastering display

[Stanimir Varbanov]

On 11/10/20 11:50 AM, Hans Verkuil wrote:
> On 09/11/2020 18:31, Stanimir Varbanov wrote:
>> Document Content light level and Mastering display colour volume.
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>>  .../media/v4l/ext-ctrls-codec.rst             | 61 +++++++++++++++++++
>>  1 file changed, 61 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index ce728c757eaf..39d0aab5ca3d 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -4382,3 +4382,64 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
>>        - Selecting this value specifies that HEVC slices are expected
>>          to be prefixed by Annex B start codes. According to :ref:`hevc`
>>          valid start codes can be 3-bytes 0x000001 or 4-bytes 0x00000001.
>> +
>> +``V4L2_CID_MPEG_VIDEO_HEVC_CLL_INFO (struct)``
>> +    The Content Light Level defines upper bounds for the nominal target
>> +    brightness light level of the pictures.
>> +
>> +.. c:type:: v4l2_ctrl_hevc_cll_info
>> +
>> +.. cssclass:: longtable
>> +
>> +.. flat-table:: struct v4l2_ctrl_hevc_cll_info
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       1 1 2
>> +
>> +    * - __u16
>> +      - ``max_content_light_level``
>> +      - An upper bound on the maximum light level among all individual
>> +        samples for the pictures of coded video sequence, cd/m2.
>> +    * - __u16
>> +      - ``max_pic_average_light_level``
>> +      - An upper bound on the maximum average light level among the
>> +        samples for any idividual picture of coded video sequence, cd/m2.
> 
> idividual -> individual
> 
> In the CTA-861-G spec value 0 is used to indicate that this information is
> not present. How is that handled here? Can it be 0 as well in an HEVC stream?

ITU-T Rec. H265 says: When equal to 0, no such upper bound is indicated
by max_content_light_level.

So, the meaning is the same as in CTA-861-G.

> 
> Same for the next control.
> 
>> +
>> +``V4L2_CID_MPEG_VIDEO_HEVC_MASTERING_DISPLAY (struct)``
>> +    The mastering display defines the colour volume (the colour primaries,
>> +    white point and luminance range) of a display considered to be the
>> +    mastering display for current video content.
>> +
>> +.. c:type:: v4l2_ctrl_hevc_mastering_display
>> +
>> +.. cssclass:: longtable
>> +
>> +.. flat-table:: struct v4l2_ctrl_hevc_mastering_display
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       1 1 2
>> +
>> +    * - __u16
>> +      - ``display_primaries_x[3]``
>> +      - Specifies the normalized x chromaticity coordinate of the colour
>> +        primary component of the mastering display.
> 
> CTA-861-G defines this as: "coded as unsigned 16-bit values in units
> of 0.00002, where 0x0000 represents zero and 0xC350 represents 1.0000."
> 
> Is that true here as well? If so, then this should be documented because
> "normalized x chromaticity coordinate" doesn't say anything meaningful.

Yes, it is the same. Will document that in next version.

> 
>> +    * - __u16
>> +      - ``display_primaries_y[3]``
>> +      - Specifies the normalized y chromaticity coordinate of the colour
>> +        primary component of the mastering display.
>> +    * - __u16
>> +      - ``white_point_x``
>> +      - Specifies the normalized x chromaticity coordinate of the white
>> +        point of the mastering display.
>> +    * - __u16
>> +      - ``white_point_y``
>> +      - Specifies the normalized y chromaticity coordinate of the white
>> +        point of the mastering display.
>> +    * - __u32
>> +      - ``max_luminance``
>> +      - Specifies the nominal maximum display luminance of the mastering
>> +        display.
> 
> In CTA-861-G this is in 1 cd/m^2 units.

In Rec. H265 max_luminance is in the range of 50 000 to 100 000 000 and
units of 0.0001 cd/m2.

> 
>> +    * - __u32
>> +      - ``min_luminance``
>> +      - specifies the nominal minimum display luminance of the mastering
>> +        display.
> 
> And this in units of 0.0001 cd/m^2.

min_luminance - range of 1 to 50 000 and units of 0.0001 cd/m2.

I will update all these in next patchset version.

> 
> Regards,
> 
> 	Hans
> 

-- 
regards,
Stan