Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5 4/5] docs: Document CLL and Mastering display
 colorimetry controls
To:     Stanimir Varbanov <stanimir.varbanov@linaro.org>,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org
Cc:     Ezequiel Garcia <ezequiel@collabora.com>,
        Nicolas Dufresne <nicolas.dufresne@collabora.com>
References: <20210209162425.3970393-1-stanimir.varbanov@linaro.org>
 <20210209162425.3970393-5-stanimir.varbanov@linaro.org>
From:   Hans Verkuil <hverkuil-cisco@xs4all.nl>
Message-ID: <ebb4d08f-9af6-f483-f901-4846911cd71b@xs4all.nl>
Date:   Tue, 16 Mar 2021 13:22:55 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210209162425.3970393-5-stanimir.varbanov@linaro.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 09/02/2021 17:24, Stanimir Varbanov wrote:
> Document Content Light Level and Mastering Display v4l2 colorimetry
> controls.
> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  .../media/v4l/ext-ctrls-colorimetry.rst       | 71 +++++++++++++++++++
>  .../media/v4l/vidioc-g-ext-ctrls.rst          |  8 +++
>  .../media/videodev2.h.rst.exceptions          |  2 +
>  3 files changed, 81 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-colorimetry.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-colorimetry.rst
> index 862f78b41b32..a9f206a46175 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-colorimetry.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-colorimetry.rst
> @@ -20,3 +20,74 @@ Colorimetry Control IDs
>      The Colorimetry class descriptor. Calling
>      :ref:`VIDIOC_QUERYCTRL` for this control will
>      return a description of this control class.
> +
> +``V4L2_CID_COLORIMETRY_HDR10_CLL_INFO (struct)``
> +    The Content Light Level defines upper bounds for the nominal target
> +    brightness light level of the pictures.
> +
> +.. c:type:: v4l2_ctrl_hdr10_cll_info
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_hdr10_cll_info
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 1 2
> +
> +    * - __u16
> +      - ``max_content_light_level``
> +      - The upper bound for the maximum light level among all individual
> +        samples for the pictures of a coded video sequence, cd/m\ :sup:`2`.

I think 'coded' should be removed. This is equally valid when receiving
uncoded HDMI video, after all.

> +        When equal to 0 no such upper bound is present.
> +    * - __u16
> +      - ``max_pic_average_light_level``
> +      - The upper bound for the maximum average light level among the
> +        samples for any individual picture of a coded video sequence,

ditto.

> +        cd/m\ :sup:`2`. When equal to 0 no such upper bound is present.
> +
> +``V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY (struct)``
> +    The mastering display defines the color volume (the color primaries,
> +    white point and luminance range) of a display considered to be the
> +    mastering display for the current video content.
> +
> +.. c:type:: v4l2_ctrl_hdr10_mastering_display
> +
> +.. cssclass:: longtable
> +
> +.. flat-table:: struct v4l2_ctrl_hdr10_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 color
> +        primary component c of the mastering display in increments of 0.00002.
> +        For describing the mastering display that uses Red, Green and Blue
> +        color primaries, index value c equal to 0 corresponds to the Green
> +        primary, c equal to 1 corresponds to Blue primary and c equal to 2
> +        corresponds to the Red color primary.
> +    * - __u16
> +      - ``display_primaries_y[3]``
> +      - Specifies the normalized y chromaticity coordinate of the color
> +        primary component c of the mastering display in increments of 0.00002.
> +        For describing the mastering display that uses Red, Green and Blue
> +        color primaries, index value c equal to 0 corresponds to the Green
> +        primary, c equal to 1 corresponds to Blue primary and c equal to 2
> +        corresponds to Red color primary.
> +    * - __u16
> +      - ``white_point_x``
> +      - Specifies the normalized x chromaticity coordinate of the white
> +        point of the mastering display in increments of 0.00002.
> +    * - __u16
> +      - ``white_point_y``
> +      - Specifies the normalized y chromaticity coordinate of the white
> +        point of the mastering display in increments of 0.00002.
> +    * - __u32
> +      - ``max_luminance``
> +      - Specifies the nominal maximum display luminance of the mastering
> +        display in units of 0.0001 cd/m\ :sup:`2`.
> +    * - __u32
> +      - ``min_luminance``
> +      - specifies the nominal minimum display luminance of the mastering
> +        display in units of 0.0001 cd/m\ :sup:`2`.
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> index 8a95ebdd499a..2f6d0539fb93 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> @@ -212,6 +212,14 @@ still cause this situation.
>        - ``p_fwht_params``
>        - A pointer to a struct :c:type:`v4l2_ctrl_fwht_params`. Valid if this control is
>          of type ``V4L2_CTRL_TYPE_FWHT_PARAMS``.
> +    * - struct :c:type:`v4l2_ctrl_hdr10_cll_info` *
> +      - ``p_hdr10_cll``
> +      - A pointer to a struct :c:type:`v4l2_ctrl_hdr10_cll_info`. Valid if this control is
> +        of type ``V4L2_CTRL_TYPE_HDR10_CLL_INFO``.
> +    * - struct :c:type:`v4l2_ctrl_hdr10_mastering_display` *
> +      - ``p_hdr10_mastering``
> +      - A pointer to a struct :c:type:`v4l2_ctrl_hdr10_mastering_display`. Valid if this control is
> +        of type ``V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY``.
>      * - void *
>        - ``ptr``
>        - A pointer to a compound type which can be an N-dimensional array
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index 0ed170c6e720..38b31a9b9580 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -147,6 +147,8 @@ replace symbol V4L2_CTRL_TYPE_HEVC_PPS :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_AREA :c:type:`v4l2_ctrl_type`
>  replace symbol V4L2_CTRL_TYPE_FWHT_PARAMS :c:type:`v4l2_ctrl_type`
> +replace symbol V4L2_CTRL_TYPE_HDR10_CLL_INFO :c:type:`v4l2_ctrl_type`
> +replace symbol V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY :c:type:`v4l2_ctrl_type`
>  
>  # V4L2 capability defines
>  replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities
> 

After making the change suggested above, you can add my:

Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Regards,

	Hans