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

[Nicolas Dufresne <nicolas@ndufresne.ca>]

[-- Attachment #1: Type: text/plain, Size: 2294 bytes --]

Hi,

Le mercredi 14 janvier 2026 à 16:20 -0800, Deepa Guthyappa Madivalara a écrit :
> > > +``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.

That's exactly what I want to avoid, hardcoding Android HAL into V4L2 without
having our own rational and documentation. Also, Android HAL is a much older API
then D3D and Vulkan, and its not as well defined.The second is hardcoding range
for one specific implementation. Since this is codec agnostic, and hardware
agnostic control, I would prefer is defined in a way that it requires no scaling
by the driver. IIRC, some codec have QP values from 0 to 63, so why don't we
allo from -63 to 63 ? The alternative is to let the driver expose its range, but
its a little tricky, you will have to specify when this information is available
in the Stateful Video Encoder spec.

As for the rest, you haven't considered extensibility in your proposal, what if
a non Qualcomm hardware do have features like aboslute QP ? (Hantro/VSI does
btw). How do we add that in a clean way ?

Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]