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

[majja@codeaurora.org]

Hi Hans,

Thank you for the suggestion. I will update the patch accordingly.

Regards,
     Maheshwar.


On 2020-05-18 23:45, Hans Verkuil wrote:
> Hi Maheshwar,
> 
> On 18/05/2020 23:09, majja@codeaurora.org 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@codeaurora.org>
>>> ---
>>>  .../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)