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@xxxxxxxxxx>
>>>> ---
>>>> .../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
