Re: [PATCH] media: v4l2-ctrls: Add encoded frame quality controls

[Hans Verkuil]

Hi Maheshwar,

On 18/05/2020 23:09, majja@xxxxxxxxxxxxxx wrote:
> Hi,
> 
> Regarding below patch -
> 
> HEIF/HEIC image encoding uses HEVC/AVC encoders and client can set image 
> quality level using
> V4L2_CID_MPEG_VIDEO_FRAME_QUALITY control.
> 
> Reference BITRATE_MODE_CQ at
> https://developer.android.com/reference/android/media/MediaCodecInfo.EncoderCapabilities#BITRATE_MODE_CQ

So what you are really introducing here is a new enum v4l2_mpeg_video_bitrate_mode
mode: V4L2_MPEG_VIDEO_BITRATE_MODE_CQ.

Why not just add that new mode, then add V4L2_CID_MPEG_VIDEO_CONSTANT_QUALITY
which is only used if the bitrate mode is MODE_CQ.

That builds nicely on top of the already existing V4L2_CID_MPEG_VIDEO_BITRATE_MODE
control.

Regards,

	Hans

> 
> Regards,
>      Maheshwar.
> 
> 
> On 2020-05-18 14:07, Maheshwar Ajja wrote:
>> When frame quality control is enabled encoder will choose
>> the appropriate quantization parameter and bitrate to
>> produce the client requested frame quality level.
>> When frame quality control is disabled then frame quality
>> is decided based on appropriate controls (i.e.
>> V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE and/or
>> V4L2_CID_MPEG_VIDEO_BITRATE_MODE)
>>
>> Signed-off-by: Maheshwar Ajja <majja@xxxxxxxxxxxxxx>
>> ---
>>  .../userspace-api/media/v4l/ext-ctrls-codec.rst          | 16 
>> ++++++++++++++++
>>  drivers/media/v4l2-core/v4l2-ctrls.c                     |  3 +++
>>  include/uapi/linux/v4l2-controls.h                       |  2 ++
>>  3 files changed, 21 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index d0d506a..495b39b 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -1081,6 +1081,22 @@ enum v4l2_mpeg_video_h264_entropy_mode -
>>      Macroblock level rate control enable. Applicable to the MPEG4 and
>>      H264 encoders.
>>
>> +``V4L2_CID_MPEG_VIDEO_FRAME_QUALITY_ENABLE (boolean)``
>> +    Encoded frame quality control enable. If this control is enabled 
>> then
>> +    the quality level of the encoded frame is set with control
>> +    ``V4L2_CID_MPEG_VIDEO_CONSTANT_QUALITY``. If this control is 
>> disabled
>> +    then the quality level of encoded frame is adjusted with 
>> appropriate
>> +    controls (e.g. ``V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE`` or
>> +    ``V4L2_CID_MPEG_VIDEO_BITRATE_MODE``). Applicable to encoders.
>> +
>> +``V4L2_CID_MPEG_VIDEO_FRAME_QUALITY (integer)``
>> +    Encoded frame quality control. If the control
>> +    ``V4L2_CID_MPEG_VIDEO_FRAME_QUALITY_ENABLE`` is enabled then the
>> +    quality of encoded frame is set with this control. Valid range is 
>> 1 to
>> +    100 where 1 indicates lowest quality and 100 indicates highest 
>> quality.
>> +    Encoder will decide the appropriate quantization parameter and 
>> bitrate
>> +    to produce requested frame quality. Applicable to encoders.
>> +
>>  ``V4L2_CID_MPEG_VIDEO_MPEG4_QPEL (boolean)``
>>      Quarter pixel motion estimation for MPEG4. Applicable to the MPEG4
>>      encoder.
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 1c617b4..1477198 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -982,6 +982,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_FRAME_QUALITY_ENABLE:		return "Frame Quality 
>> Enable";
>> +	case V4L2_CID_MPEG_VIDEO_FRAME_QUALITY:			return "Frame Quality";
>>
>>  	/* CAMERA controls */
>>  	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
>> @@ -1178,6 +1180,7 @@ void v4l2_ctrl_fill(u32 id, const char **name,
>> enum v4l2_ctrl_type *type,
>>  	case V4L2_CID_FLASH_READY:
>>  	case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER:
>>  	case V4L2_CID_MPEG_VIDEO_DECODER_SLICE_INTERFACE:
>> +	case V4L2_CID_MPEG_VIDEO_FRAME_QUALITY_ENABLE:
>>  	case V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE:
>>  	case V4L2_CID_MPEG_VIDEO_MB_RC_ENABLE:
>>  	case V4L2_CID_MPEG_VIDEO_H264_8X8_TRANSFORM:
>> diff --git a/include/uapi/linux/v4l2-controls.h
>> b/include/uapi/linux/v4l2-controls.h
>> index 0ba1005..d97a934 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -742,6 +742,8 @@ enum v4l2_cid_mpeg_video_hevc_size_of_length_field 
>> {
>>  #define V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_BR	(V4L2_CID_MPEG_BASE 
>> + 642)
>>  #define V4L2_CID_MPEG_VIDEO_REF_NUMBER_FOR_PFRAMES	(V4L2_CID_MPEG_BASE 
>> + 643)
>>  #define V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR	(V4L2_CID_MPEG_BASE 
>> + 644)
>> +#define V4L2_CID_MPEG_VIDEO_FRAME_QUALITY_ENABLE	(V4L2_CID_MPEG_BASE + 
>> 645)
>> +#define V4L2_CID_MPEG_VIDEO_FRAME_QUALITY		(V4L2_CID_MPEG_BASE + 646)
>>
>>  /*  MPEG-class control IDs specific to the CX2341x driver as defined 
>> by V4L2 */
>>  #define V4L2_CID_MPEG_CX2341X_BASE				(V4L2_CTRL_CLASS_MPEG | 0x1000)