diff mbox series

[1/2] media: v4l2-ctrls: Add average qp control

Message ID 20240329092352.2648837-1-ming.qian@nxp.com (mailing list archive)
State New, archived
Headers show
Series [1/2] media: v4l2-ctrls: Add average qp control | expand

Commit Message

Ming Qian March 29, 2024, 9:23 a.m. UTC
Add a control V4L2_CID_MPEG_VIDEO_AVERAGE_QP to report the average qp
value of current encoded frame.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
 Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++++
 drivers/media/v4l2-core/v4l2-ctrls-defs.c                 | 5 +++++
 include/uapi/linux/v4l2-controls.h                        | 2 ++
 3 files changed, 11 insertions(+)

Comments

Nicolas Dufresne April 4, 2024, 6:14 p.m. UTC | #1
Hi,

Le vendredi 29 mars 2024 à 18:23 +0900, Ming Qian a écrit :
> Add a control V4L2_CID_MPEG_VIDEO_AVERAGE_QP to report the average qp
> value of current encoded frame.
> 
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
>  Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++++
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c                 | 5 +++++
>  include/uapi/linux/v4l2-controls.h                        | 2 ++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 2a165ae063fb..cef20b3f54ca 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -1653,6 +1653,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>      Quantization parameter for a P frame for FWHT. Valid range: from 1
>      to 31.
>  
> +``V4L2_CID_MPEG_VIDEO_AVERAGE_QP (integer)``
> +    This read-only control returns the average qp value of the currently
> +    encoded frame. Applicable to the H264 and HEVC encoders.

That seems ambiguous at best. What does it mean the "currently encoded frame" ?
The OUTPUT and CAPTURE queue can be holding multiple frames. For "per frame"
accurate reporting, I feel like we'd need something like Hans' read-only
requests proposal [0]. Its basically a mechanism that let you attach request FD
to capture buffer, so that supported controls can be saved per v4l2-buffer and
read later on.

https://patches.linaro.org/project/linux-media/patch/20210610113615.785359-12-hverkuil-cisco@xs4all.nl/

If this isn't what you wanted, we'll need a better definition. It might be
helpful to explain what this is used for.

Nicolas

> +
>  .. raw:: latex
>  
>      \normalsize
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 8696eb1cdd61..88e86e4e539d 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -972,6 +972,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES:		return "Use LTR Frames";
>  	case V4L2_CID_FWHT_I_FRAME_QP:				return "FWHT I-Frame QP Value";
>  	case V4L2_CID_FWHT_P_FRAME_QP:				return "FWHT P-Frame QP Value";
> +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:			return "Average QP value";
>  
>  	/* VPX controls */
>  	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		return "VPX Number of Partitions";
> @@ -1507,6 +1508,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  		*max = 0xffffffffffffLL;
>  		*step = 1;
>  		break;
> +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:
> +		*type = V4L2_CTRL_TYPE_INTEGER;
> +		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
> +		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 99c3f5e99da7..974fd254e573 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -898,6 +898,8 @@ enum v4l2_mpeg_video_av1_level {
>  	V4L2_MPEG_VIDEO_AV1_LEVEL_7_3 = 23
>  };
>  
> +#define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
> +
>  /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
>  #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
>  #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE		(V4L2_CID_CODEC_CX2341X_BASE+0)
Ming Qian(OSS) April 7, 2024, 2:43 a.m. UTC | #2
Hi Nicolas,

On 4/5/24 02:14, Nicolas Dufresne wrote:
> Hi,
> 
> Le vendredi 29 mars 2024 à 18:23 +0900, Ming Qian a écrit :
>> Add a control V4L2_CID_MPEG_VIDEO_AVERAGE_QP to report the average qp
>> value of current encoded frame.
>>
>> Signed-off-by: Ming Qian <ming.qian@nxp.com>
>> ---
>>   Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++++
>>   drivers/media/v4l2-core/v4l2-ctrls-defs.c                 | 5 +++++
>>   include/uapi/linux/v4l2-controls.h                        | 2 ++
>>   3 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index 2a165ae063fb..cef20b3f54ca 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -1653,6 +1653,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>>       Quantization parameter for a P frame for FWHT. Valid range: from 1
>>       to 31.
>>   
>> +``V4L2_CID_MPEG_VIDEO_AVERAGE_QP (integer)``
>> +    This read-only control returns the average qp value of the currently
>> +    encoded frame. Applicable to the H264 and HEVC encoders.
> 
> That seems ambiguous at best. What does it mean the "currently encoded frame" ?
> The OUTPUT and CAPTURE queue can be holding multiple frames. For "per frame"
> accurate reporting, I feel like we'd need something like Hans' read-only
> requests proposal [0]. Its basically a mechanism that let you attach request FD
> to capture buffer, so that supported controls can be saved per v4l2-buffer and
> read later on.
> 
> https://patches.linaro.org/project/linux-media/patch/20210610113615.785359-12-hverkuil-cisco@xs4all.nl/
> 
> If this isn't what you wanted, we'll need a better definition. It might be
> helpful to explain what this is used for.
> 
> Nicolas
> 

Yes, I want to report the qp value for every frame.
I thought the request FD is only used for stateless decoder, but I want
to add a read-only ctrl for the stateful encoder. So I checked the
defined read-only ctrls, I think it's similar with
V4L2_CID_MPEG_VIDEO_DEC_PTS.
(https://linuxtv.org/downloads/v4l-dvb-apis/userspace-api/v4l/ext-ctrls-codec.html?highlight=v4l2_cid_mpeg_video_dec_pts)

then back to your question about the "currently encoded frame", it's the
last dequeued capture buffer of the encoder, the capture queue can hold
multiple frames, each frame will have a qp value in this case, and this
ctrl only report the qp value of the last dequeued frame, when user has
dequeued an encoded frame from the capture queue, he can get the ctrl
value of V4L2_CID_MPEG_VIDEO_AVERAGE_QP immediately to get the qp value
of the currently dequeued frame. If user doesn't care about this
parameter, he doesn't need to do anything, it's just the same as before.
so I think this ctrl is backward compatible.

Maybe the request FD is a better and more intuitive way to suggest a
one-to-one correspondence between ctrl and frame. I'm just not sure if
it just applies to the stateless decoder. I did find any stateful
decoder or encoder to use them.
If we use the request FD for this stateful encoder, I'm not sure if it
will break the original flow.

best regards,
Ming

>> +
>>   .. raw:: latex
>>   
>>       \normalsize
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
>> index 8696eb1cdd61..88e86e4e539d 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
>> @@ -972,6 +972,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>   	case V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES:		return "Use LTR Frames";
>>   	case V4L2_CID_FWHT_I_FRAME_QP:				return "FWHT I-Frame QP Value";
>>   	case V4L2_CID_FWHT_P_FRAME_QP:				return "FWHT P-Frame QP Value";
>> +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:			return "Average QP value";
>>   
>>   	/* VPX controls */
>>   	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		return "VPX Number of Partitions";
>> @@ -1507,6 +1508,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>   		*max = 0xffffffffffffLL;
>>   		*step = 1;
>>   		break;
>> +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:
>> +		*type = V4L2_CTRL_TYPE_INTEGER;
>> +		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
>> +		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 99c3f5e99da7..974fd254e573 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -898,6 +898,8 @@ enum v4l2_mpeg_video_av1_level {
>>   	V4L2_MPEG_VIDEO_AV1_LEVEL_7_3 = 23
>>   };
>>   
>> +#define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
>> +
>>   /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
>>   #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
>>   #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE		(V4L2_CID_CODEC_CX2341X_BASE+0)
>
Hans Verkuil April 24, 2024, 10:03 a.m. UTC | #3
On 29/03/2024 10:23, Ming Qian wrote:
> Add a control V4L2_CID_MPEG_VIDEO_AVERAGE_QP to report the average qp
> value of current encoded frame.
> 
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
>  Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++++
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c                 | 5 +++++
>  include/uapi/linux/v4l2-controls.h                        | 2 ++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index 2a165ae063fb..cef20b3f54ca 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -1653,6 +1653,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>      Quantization parameter for a P frame for FWHT. Valid range: from 1
>      to 31.
>  
> +``V4L2_CID_MPEG_VIDEO_AVERAGE_QP (integer)``
> +    This read-only control returns the average qp value of the currently
> +    encoded frame. Applicable to the H264 and HEVC encoders.

qp -> QP

Why is this applicable to H264/HEVC only? I think it is fine for any codec.

This needs to document that the value applies to the last dequeued buffer
(VIDIOC_DQBUF).

> +
>  .. raw:: latex
>  
>      \normalsize
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 8696eb1cdd61..88e86e4e539d 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -972,6 +972,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES:		return "Use LTR Frames";
>  	case V4L2_CID_FWHT_I_FRAME_QP:				return "FWHT I-Frame QP Value";
>  	case V4L2_CID_FWHT_P_FRAME_QP:				return "FWHT P-Frame QP Value";
> +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:			return "Average QP value";

value -> Value

Also move it up two lines so that it follows V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES
rather than FWHT controls.

>  
>  	/* VPX controls */
>  	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		return "VPX Number of Partitions";
> @@ -1507,6 +1508,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  		*max = 0xffffffffffffLL;
>  		*step = 1;
>  		break;
> +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:
> +		*type = V4L2_CTRL_TYPE_INTEGER;
> +		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;

Drop the volatile flag, this isn't a volatile value.

> +		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 99c3f5e99da7..974fd254e573 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -898,6 +898,8 @@ enum v4l2_mpeg_video_av1_level {
>  	V4L2_MPEG_VIDEO_AV1_LEVEL_7_3 = 23
>  };
>  
> +#define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
> +
>  /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
>  #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
>  #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE		(V4L2_CID_CODEC_CX2341X_BASE+0)

Regards,

	Hans
Nicolas Dufresne May 1, 2024, 7:55 p.m. UTC | #4
Le mercredi 24 avril 2024 à 12:03 +0200, Hans Verkuil a écrit :
> On 29/03/2024 10:23, Ming Qian wrote:
> > Add a control V4L2_CID_MPEG_VIDEO_AVERAGE_QP to report the average qp
> > value of current encoded frame.
> > 
> > Signed-off-by: Ming Qian <ming.qian@nxp.com>
> > ---
> >  Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++++
> >  drivers/media/v4l2-core/v4l2-ctrls-defs.c                 | 5 +++++
> >  include/uapi/linux/v4l2-controls.h                        | 2 ++
> >  3 files changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > index 2a165ae063fb..cef20b3f54ca 100644
> > --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> > @@ -1653,6 +1653,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> >      Quantization parameter for a P frame for FWHT. Valid range: from 1
> >      to 31.
> >  
> > +``V4L2_CID_MPEG_VIDEO_AVERAGE_QP (integer)``
> > +    This read-only control returns the average qp value of the currently
> > +    encoded frame. Applicable to the H264 and HEVC encoders.
> 
> qp -> QP
> 
> Why is this applicable to H264/HEVC only? I think it is fine for any codec.

Same question, though we should document the range, which will differ per-
codecs. We should also document to always use the specified range, rather then a
HW custom range. This way we don't endup with issues we have hit with other ill-
defined controls.

Nicolas

> 
> This needs to document that the value applies to the last dequeued buffer
> (VIDIOC_DQBUF).
> 
> > +
> >  .. raw:: latex
> >  
> >      \normalsize
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > index 8696eb1cdd61..88e86e4e539d 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> > @@ -972,6 +972,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> >  	case V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES:		return "Use LTR Frames";
> >  	case V4L2_CID_FWHT_I_FRAME_QP:				return "FWHT I-Frame QP Value";
> >  	case V4L2_CID_FWHT_P_FRAME_QP:				return "FWHT P-Frame QP Value";
> > +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:			return "Average QP value";
> 
> value -> Value
> 
> Also move it up two lines so that it follows V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES
> rather than FWHT controls.
> 
> >  
> >  	/* VPX controls */
> >  	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		return "VPX Number of Partitions";
> > @@ -1507,6 +1508,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> >  		*max = 0xffffffffffffLL;
> >  		*step = 1;
> >  		break;
> > +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:
> > +		*type = V4L2_CTRL_TYPE_INTEGER;
> > +		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
> 
> Drop the volatile flag, this isn't a volatile value.
> 
> > +		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 99c3f5e99da7..974fd254e573 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -898,6 +898,8 @@ enum v4l2_mpeg_video_av1_level {
> >  	V4L2_MPEG_VIDEO_AV1_LEVEL_7_3 = 23
> >  };
> >  
> > +#define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
> > +
> >  /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
> >  #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
> >  #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE		(V4L2_CID_CODEC_CX2341X_BASE+0)
> 
> Regards,
> 
> 	Hans
>
Ming Qian(OSS) May 6, 2024, 2:28 a.m. UTC | #5
Hi Nicolas,

> Le mercredi 24 avril 2024 à 12:03 +0200, Hans Verkuil a écrit :
>> On 29/03/2024 10:23, Ming Qian wrote:
>>> Add a control V4L2_CID_MPEG_VIDEO_AVERAGE_QP to report the average qp
>>> value of current encoded frame.
>>>
>>> Signed-off-by: Ming Qian <ming.qian@nxp.com>
>>> ---
>>>   Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 4 ++++
>>>   drivers/media/v4l2-core/v4l2-ctrls-defs.c                 | 5 +++++
>>>   include/uapi/linux/v4l2-controls.h                        | 2 ++
>>>   3 files changed, 11 insertions(+)
>>>
>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> index 2a165ae063fb..cef20b3f54ca 100644
>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> @@ -1653,6 +1653,10 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>>>       Quantization parameter for a P frame for FWHT. Valid range: from 1
>>>       to 31.
>>>   
>>> +``V4L2_CID_MPEG_VIDEO_AVERAGE_QP (integer)``
>>> +    This read-only control returns the average qp value of the currently
>>> +    encoded frame. Applicable to the H264 and HEVC encoders.
>>
>> qp -> QP
>>
>> Why is this applicable to H264/HEVC only? I think it is fine for any codec.
> 
> Same question, though we should document the range, which will differ per-
> codecs. We should also document to always use the specified range, rather then a
> HW custom range. This way we don't endup with issues we have hit with other ill-
> defined controls.
> 
> Nicolas

Thanks for the tip, I will add a description of the value range in V3
patch.


> 
>>
>> This needs to document that the value applies to the last dequeued buffer
>> (VIDIOC_DQBUF).
>>
>>> +
>>>   .. raw:: latex
>>>   
>>>       \normalsize
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
>>> index 8696eb1cdd61..88e86e4e539d 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
>>> @@ -972,6 +972,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>>   	case V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES:		return "Use LTR Frames";
>>>   	case V4L2_CID_FWHT_I_FRAME_QP:				return "FWHT I-Frame QP Value";
>>>   	case V4L2_CID_FWHT_P_FRAME_QP:				return "FWHT P-Frame QP Value";
>>> +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:			return "Average QP value";
>>
>> value -> Value
>>
>> Also move it up two lines so that it follows V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES
>> rather than FWHT controls.
>>
>>>   
>>>   	/* VPX controls */
>>>   	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		return "VPX Number of Partitions";
>>> @@ -1507,6 +1508,10 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>>>   		*max = 0xffffffffffffLL;
>>>   		*step = 1;
>>>   		break;
>>> +	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:
>>> +		*type = V4L2_CTRL_TYPE_INTEGER;
>>> +		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
>>
>> Drop the volatile flag, this isn't a volatile value.
>>
>>> +		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 99c3f5e99da7..974fd254e573 100644
>>> --- a/include/uapi/linux/v4l2-controls.h
>>> +++ b/include/uapi/linux/v4l2-controls.h
>>> @@ -898,6 +898,8 @@ enum v4l2_mpeg_video_av1_level {
>>>   	V4L2_MPEG_VIDEO_AV1_LEVEL_7_3 = 23
>>>   };
>>>   
>>> +#define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
>>> +
>>>   /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
>>>   #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
>>>   #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE		(V4L2_CID_CODEC_CX2341X_BASE+0)
>>
>> Regards,
>>
>> 	Hans
>>
>
diff mbox series

Patch

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index 2a165ae063fb..cef20b3f54ca 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -1653,6 +1653,10 @@  enum v4l2_mpeg_video_h264_hierarchical_coding_type -
     Quantization parameter for a P frame for FWHT. Valid range: from 1
     to 31.
 
+``V4L2_CID_MPEG_VIDEO_AVERAGE_QP (integer)``
+    This read-only control returns the average qp value of the currently
+    encoded frame. Applicable to the H264 and HEVC encoders.
+
 .. raw:: latex
 
     \normalsize
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index 8696eb1cdd61..88e86e4e539d 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -972,6 +972,7 @@  const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES:		return "Use LTR Frames";
 	case V4L2_CID_FWHT_I_FRAME_QP:				return "FWHT I-Frame QP Value";
 	case V4L2_CID_FWHT_P_FRAME_QP:				return "FWHT P-Frame QP Value";
+	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:			return "Average QP value";
 
 	/* VPX controls */
 	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		return "VPX Number of Partitions";
@@ -1507,6 +1508,10 @@  void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		*max = 0xffffffffffffLL;
 		*step = 1;
 		break;
+	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:
+		*type = V4L2_CTRL_TYPE_INTEGER;
+		*flags |= V4L2_CTRL_FLAG_VOLATILE | V4L2_CTRL_FLAG_READ_ONLY;
+		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 99c3f5e99da7..974fd254e573 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -898,6 +898,8 @@  enum v4l2_mpeg_video_av1_level {
 	V4L2_MPEG_VIDEO_AV1_LEVEL_7_3 = 23
 };
 
+#define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
+
 /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
 #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
 #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE		(V4L2_CID_CODEC_CX2341X_BASE+0)