Re: [PATCH 3/5] media: Rename stateful codec control macros

[Hans Verkuil]

On 12/11/2020 12:57, Ezequiel Garcia wrote:
> For historical reasons, stateful codec controls are named
> as {}_MPEG_{}. While we can't at this point sanely
> change all control IDs (such as V4L2_CID_MPEG_VIDEO_VP8_FRAME_HEADER),
> we can least change the more meaningful macros such as classes
> macros.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> ---
>  .../userspace-api/media/v4l/dev-mem2mem.rst   |   2 +-
>  .../media/v4l/ext-ctrls-codec.rst             |   4 +-
>  .../media/v4l/extended-controls.rst           |   8 +-
>  .../media/v4l/vidioc-g-ext-ctrls.rst          |   6 +-
>  drivers/media/common/cx2341x.c                |   4 +-
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c  |   2 +-
>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c  |   2 +-
>  drivers/media/v4l2-core/v4l2-ctrls.c          |   4 +-
>  include/media/fwht-ctrls.h                    |   2 +-
>  include/media/h264-ctrls.h                    |  16 +-
>  include/media/hevc-ctrls.h                    |  10 +-
>  include/media/mpeg2-ctrls.h                   |   4 +-
>  include/media/vp8-ctrls.h                     |   2 +-
>  include/uapi/linux/v4l2-controls.h            | 354 +++++++++---------
>  14 files changed, 212 insertions(+), 208 deletions(-)
> 

<snip>

> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 7035f4fb182c..53122ee42988 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -54,7 +54,7 @@
>  
>  /* Control classes */
>  #define V4L2_CTRL_CLASS_USER		0x00980000	/* Old-style 'user' controls */
> -#define V4L2_CTRL_CLASS_MPEG		0x00990000	/* MPEG-compression controls */
> +#define V4L2_CTRL_CLASS_CODEC		0x00990000	/* Stateful codec controls */
>  #define V4L2_CTRL_CLASS_CAMERA		0x009a0000	/* Camera class controls */
>  #define V4L2_CTRL_CLASS_FM_TX		0x009b0000	/* FM Modulator controls */
>  #define V4L2_CTRL_CLASS_FLASH		0x009c0000	/* Camera flash controls */
> @@ -66,6 +66,10 @@
>  #define V4L2_CTRL_CLASS_RF_TUNER	0x00a20000	/* RF tuner controls */
>  #define V4L2_CTRL_CLASS_DETECT		0x00a30000	/* Detection controls */
>  
> +#ifndef __KERNEL__
> +#define V4L2_CTRL_CLASS_MPEG V4L2_CTRL_CLASS_CODEC	/* MPEG-compression controls (Legacy) */
> +#endif
> +
>  /* User-class control IDs */
>  
>  #define V4L2_CID_BASE			(V4L2_CTRL_CLASS_USER | 0x900)
> @@ -208,11 +212,11 @@ enum v4l2_colorfx {
>  /* The MPEG controls are applicable to all codec controls
>   * and the 'MPEG' part of the define is historical */
>  
> -#define V4L2_CID_MPEG_BASE			(V4L2_CTRL_CLASS_MPEG | 0x900)
> -#define V4L2_CID_MPEG_CLASS			(V4L2_CTRL_CLASS_MPEG | 1)

These two old defines should remain as aliases of the new defines for legacy
purposes under #ifndef __KERNEL__.

> +#define V4L2_CID_CODEC_BASE			(V4L2_CTRL_CLASS_CODEC | 0x900)
> +#define V4L2_CID_CODEC_CLASS			(V4L2_CTRL_CLASS_CODEC | 1)

<snip>

> @@ -775,7 +779,7 @@ enum v4l2_mpeg_video_frame_skip_mode {
>  };
>  
>  /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
> -#define V4L2_CID_MPEG_CX2341X_BASE				(V4L2_CTRL_CLASS_MPEG | 0x1000)
> +#define V4L2_CID_MPEG_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)

Rename this as well to V4L2_CID_CODEC_CX2341X_BASE (but still keep the old alias).

>  #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE		(V4L2_CID_MPEG_CX2341X_BASE+0)
>  enum v4l2_mpeg_cx2341x_video_spatial_filter_mode {
>  	V4L2_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE_MANUAL = 0,
> @@ -816,7 +820,7 @@ enum v4l2_mpeg_cx2341x_video_median_filter_type {
>  #define V4L2_CID_MPEG_CX2341X_STREAM_INSERT_NAV_PACKETS		(V4L2_CID_MPEG_CX2341X_BASE+11)
>  
>  /*  MPEG-class control IDs specific to the Samsung MFC 5.1 driver as defined by V4L2 */
> -#define V4L2_CID_MPEG_MFC51_BASE				(V4L2_CTRL_CLASS_MPEG | 0x1100)
> +#define V4L2_CID_MPEG_MFC51_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1100)

Ditto.

>  
>  #define V4L2_CID_MPEG_MFC51_VIDEO_DECODER_H264_DISPLAY_DELAY		(V4L2_CID_MPEG_MFC51_BASE+0)
>  #define V4L2_CID_MPEG_MFC51_VIDEO_DECODER_H264_DISPLAY_DELAY_ENABLE	(V4L2_CID_MPEG_MFC51_BASE+1)
> 

I think it is best to add the #ifndef __KERNEL__ part containing the aliases at the
end of this header.

Regards,

	Hans