linux-kernel - Re: [RFC PATCH 1/3] media: uapi: Introduce new control for video encoder ROI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <763b6cdd-514c-4aff-b7db-8c0a17fdb645@oss.qualcomm.com>
Date: Wed, 14 Jan 2026 16:20:52 -0800
From: Deepa Guthyappa Madivalara <deepa.madivalara@....qualcomm.com>
To: Nicolas Dufresne <nicolas@...fresne.ca>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Vikash Garodia <vikash.garodia@....qualcomm.com>,
        Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>,
        Abhinav Kumar <abhinav.kumar@...ux.dev>,
        Bryan O'Donoghue <bod@...nel.org>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [RFC PATCH 1/3] media: uapi: Introduce new control for video
 encoder ROI


On 1/14/2026 8:14 AM, Nicolas Dufresne wrote:
> Hi,
>
> Le mardi 13 janvier 2026 à 12:33 -0800, Deepa Guthyappa Madivalara a écrit :
>> Add compound control, V4L2_CID_MPEG_VIDEO_ENC_ROI, for
>> video encoder Region of Interest to allow applications to specify
>> different quality levels for specific regions in video frames. Define
>> struct v4l2_ctrl_enc_roi_params to hold up to 10 rectangular ROI,
>> regions and their corresponding delta_qp value (v4l2_roi_param)
>> that adjust quantization relative to the frame's base value.
>>
>> This enables use cases like prioritizing quality for faces in video
>> conferencing or important objects in surveillance footage while reducing
>> bitrate for less critical areas.
>>
>> Signed-off-by: Deepa Guthyappa Madivalara <deepa.madivalara@....qualcomm.com>
>> ---
>>   .../userspace-api/media/v4l/ext-ctrls-codec.rst         |  7 +++++++
>>   include/media/v4l2-ctrls.h                              |  1 +
>>   include/uapi/linux/v4l2-controls.h                      |  1 +
>>   include/uapi/linux/videodev2.h                          | 17 +++++++++++++++++
>>   4 files changed, 26 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index c8890cb5e00ac05649e6c344c2a6b938b2ec1b24..0eecb46bb356c01411dfc313b92376593bcd86f6 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -1668,6 +1668,13 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>>       Codecs need to always use the specified range, rather then a HW custom range.
>>       Applicable to encoders
>>   
>> +``V4L2_CID_MPEG_VIDEO_ENC_ROI (struct)``
>> +    Defines the control id to configure specific delta QP for one or more
>> +    rectangular regions of interest. The struct v4l2_ctrl_enc_roi_params
>> +    is defined to hold up to 10 v4l2_rect regions and their corresponding
>> +    delta_qp with a range of -31 to 30.
>> +    Applicable to encoders
> Any justification for this range ? Also, I believe I've seen hardware support
> both delta and absolute values. Since it meant to be generic, some research is
> needed. If we delibaritly ignore absolute, perhaps the CID should be named
> accordingly ? Something like V4L2_CID_MPEG_VIDEO_ENC__DELTAQP_ROI ?

As per Android ROI API - MediaCodec API QP from the app is an offset QP,
meaning userspace will received offset Qp and it converts it to deltaQp
before passing onto the driver in Android HAL. I have used the same idea.
Delta MbQP = frame QP + Offset Qp. This is clamped to -31 to 30 currently
and set to driver as delta QP, hence I have it as -31 to 30.

Absolute values are mostly for frame QP, I would say. All the 
information out there for ROI
kind of implies to deltaQP, but we could be more precise as well.
Let me know if it is a must to change to CID.

>> +
>>   .. raw:: latex
>>   
>>       \normalsize
>> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
>> index 31fc1bee3797bfe532931889188c8f7a9dedad39..c44fad7f51db45a437dd3287aa16830585ac42f3 100644
>> --- a/include/media/v4l2-ctrls.h
>> +++ b/include/media/v4l2-ctrls.h
>> @@ -91,6 +91,7 @@ union v4l2_ctrl_ptr {
>>   	struct v4l2_ctrl_av1_frame *p_av1_frame;
>>   	struct v4l2_ctrl_av1_film_grain *p_av1_film_grain;
>>   	struct v4l2_rect *p_rect;
>> +	struct v4l2_ctrl_enc_roi_params *p_enc_roi_params;
>>   	void *p;
>>   	const void *p_const;
>>   };
>> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>> index f84ed133a6c9b2ddc1aedbd582ddf78cb71f34e5..5f2621365593ee19a7792fb25ea29acf6a7860f1 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -918,6 +918,7 @@ enum v4l2_mpeg_video_av1_level {
>>   };
>>   
>>   #define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
>> +#define V4L2_CID_MPEG_VIDEO_ENC_ROI  (V4L2_CID_CODEC_BASE + 658)
>>   
>>   /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
>>   #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index add08188f06890182a5c399a223c1ab0a546cae1..18a5ae34842721c2647a7a76365e4d299d2b8a44 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -1909,6 +1909,7 @@ struct v4l2_ext_control {
>>   		struct v4l2_ctrl_av1_film_grain __user *p_av1_film_grain;
>>   		struct v4l2_ctrl_hdr10_cll_info __user *p_hdr10_cll_info;
>>   		struct v4l2_ctrl_hdr10_mastering_display __user *p_hdr10_mastering_display;
>> +		struct v4l2_ctrl_enc_roi_params __user *p_enc_roi_params;
>>   		void __user *ptr;
>>   	} __attribute__ ((packed));
>>   } __attribute__ ((packed));
>> @@ -1990,6 +1991,8 @@ enum v4l2_ctrl_type {
>>   	V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY = 0x281,
>>   	V4L2_CTRL_TYPE_AV1_FRAME	    = 0x282,
>>   	V4L2_CTRL_TYPE_AV1_FILM_GRAIN	    = 0x283,
>> +
>> +	V4L2_CTRL_TYPE_ENC_ROI_PARAMS	    = 0x284,
>>   };
>>   
>>   /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
>> @@ -2540,6 +2543,20 @@ struct v4l2_streamparm {
>>   	} parm;
>>   };
>>   
>> +/* Roi format
>> + */
>> +#define VIDEO_MAX_ROI_REGIONS         10
> Let's not hardcode hardware spececific constraints in the API. We have dynamic
> arrays now in compount controls, and you can define the maximum dimension and
> all.
>
>
>> +
>> +struct v4l2_roi_param {
>> +	struct v4l2_rect roi_rect;
>> +	__s32 delta_qp;
>> +};
>> +
>> +struct v4l2_ctrl_enc_roi_params {
>> +	__u32 num_roi_regions;
> With the dynamic arrays, this will not be needed.
>
> cheers,
> Nicolas
>
>> +	struct v4l2_roi_param roi_params[VIDEO_MAX_ROI_REGIONS];
>> +};
>> +
>>   /*
>>    *	E V E N T S
>>    */