linux-kernel - Re: [PATCH v2 1/2] media: v4l2-ctrls: add encoder maximum bitrate control

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] [day] [month] [year] [list]

Message-ID: <7322a22591ab18a664c9b6fa07957a163fceaa2d.camel@ndufresne.ca>
Date: Fri, 09 Feb 2024 10:10:17 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Sachin Kumar Garg <quic_sachinku@...cinc.com>, hverkuil-cisco@...all.nl,
  Mauro Carvalho Chehab <mchehab@...nel.org>, Stanimir Varbanov
 <stanimir.k.varbanov@...il.com>,  Vikash Garodia
 <quic_vgarodia@...cinc.com>, Andy Gross <agross@...nel.org>, Bjorn
 Andersson <andersson@...nel.org>,  Konrad Dybcio <konrad.dybcio@...aro.org>
Cc: Bryan O'Donoghue <bryan.odonoghue@...aro.org>, 
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v2 1/2] media: v4l2-ctrls: add encoder maximum bitrate
 control

Hi Scahin,

Le mardi 30 janvier 2024 à 16:53 +0530, Sachin Kumar Garg a écrit :
> Introduce V4L2_MPEG_VIDEO_BITRATE_MODE_MBR rate control to
> limit the frame level maximum bit rate.
> Encoder will choose appropriate quantization parameter and
> do the smart bit allocation to set the frame maximum bitrate
> level as per the Bitrate value configured.
> 
> Signed-off-by: Sachin Kumar Garg <quic_sachinku@...cinc.com>
> ---
>  Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 2 ++
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c                 | 1 +
>  include/uapi/linux/v4l2-controls.h                        | 1 +
>  3 files changed, 4 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..05ef4a70e3f5 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -576,6 +576,8 @@ enum v4l2_mpeg_video_bitrate_mode -
>        - Constant bitrate
>      * - ``V4L2_MPEG_VIDEO_BITRATE_MODE_CQ``
>        - Constant quality
> +    * - ``V4L2_MPEG_VIDEO_BITRATE_MODE_MBR``
> +      - Maximum bitrate

I'm afraid for this one your documentation is too short. I believe your commit
message helps, but this is not what our uAPI users will read.

My understanding is that this feature is a form of constant quality (smart bit
allocation) but with a maximum rate guaranty. Using a specific mode (rather then
a constraint on top of a constant quality mode) is a Qualcomm specific design. I
think presets are generally easier to use, so I kind of like it. What is missing
(arguably all these modes documentation are also missing it) is the rate
observation window. Would be nice to check if there is a way to specify that (or
even configure it, if so add a cross reference).

So I'd like to see some proper documentation for this one, remember that V4L2
documentation is also a specification and will serve to ensure drivers conforms
to the preset expectations.

regards,
Nicolas

>  
>  
>  
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 8696eb1cdd61..e0597b61ffb9 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -154,6 +154,7 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		"Variable Bitrate",
>  		"Constant Bitrate",
>  		"Constant Quality",
> +		"Maximum Bitrate",
>  		NULL
>  	};
>  	static const char * const mpeg_stream_type[] = {
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 99c3f5e99da7..7c74d6c417d1 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -393,6 +393,7 @@ enum v4l2_mpeg_video_bitrate_mode {
>  	V4L2_MPEG_VIDEO_BITRATE_MODE_VBR = 0,
>  	V4L2_MPEG_VIDEO_BITRATE_MODE_CBR = 1,
>  	V4L2_MPEG_VIDEO_BITRATE_MODE_CQ  = 2,
> +	V4L2_MPEG_VIDEO_BITRATE_MODE_MBR = 3,
>  };
>  #define V4L2_CID_MPEG_VIDEO_BITRATE		(V4L2_CID_CODEC_BASE+207)
>  #define V4L2_CID_MPEG_VIDEO_BITRATE_PEAK	(V4L2_CID_CODEC_BASE+208)