Re: [PATCH 1/4] media: v4l2-ctrl: Add control to enable h.264 constrained intra prediction

From: Hans Verkuil <hverkuil@xs4all.nl>
To: Philipp Zabel <p.zabel@pengutronix.de>, linux-media@vger.kernel.org
Cc: Hans Verkuil <hans.verkuil@cisco.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>
Subject: Re: [PATCH 1/4] media: v4l2-ctrl: Add control to enable h.264 constrained intra prediction
Date: Thu, 10 Jan 2019 11:37:38 +0100	[thread overview]
Message-ID: <5bb5c9b8-703e-75df-6bf9-5d1845cedec0@xs4all.nl> (raw)
In-Reply-To: <20190108171313.1750-1-p.zabel@pengutronix.de>

On 01/08/19 18:13, Philipp Zabel wrote:
> Allow to enable h.264 constrained intra prediction (macroblocks using
> intra prediction modes are not allowed to use residual data and decoded
> samples of neighboring macroblocks coded using inter prediction modes).
> This control directly corresponds to the constrained_intra_pred_flag
> field in the h.264 picture parameter set.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  Documentation/media/uapi/v4l/extended-controls.rst | 4 ++++
>  drivers/media/v4l2-core/v4l2-ctrls.c               | 2 ++
>  include/uapi/linux/v4l2-controls.h                 | 1 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> index af4273aa5e85..235d0c293983 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -1154,6 +1154,10 @@ enum v4l2_mpeg_video_h264_entropy_mode -
>  ``V4L2_CID_MPEG_VIDEO_H264_8X8_TRANSFORM (boolean)``
>      Enable 8X8 transform for H264. Applicable to the H264 encoder.
>  
> +``V4L2_CID_MPEG_VIDEO_H264_CONSTRAINED_INTRA_PREDICTION (boolean)``
> +    Enable constrained intra prediction for H264. Applicable to the H264
> +    encoder.
> +
>  ``V4L2_CID_MPEG_VIDEO_CYCLIC_INTRA_REFRESH_MB (integer)``
>      Cyclic intra macroblock refresh. This is the number of continuous
>      macroblocks refreshed every frame. Each frame a successive set of
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index e3bd441fa29a..1f2fd279f37d 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -825,6 +825,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER:return "H264 Number of HC Layers";
>  	case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP:
>  								return "H264 Set QP Value for HC Layers";
> +	case V4L2_CID_MPEG_VIDEO_H264_CONSTRAINED_INTRA_PREDICTION:
> +								return "H264 Constrained Intra Prediction";

This string is too long. The one above it ("H264 Set QP Value for HC Layers") has exactly 31
characters, which is the maximum. Perhaps abbreviating "Prediction" by "Pred" will work.

>  	case V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP:		return "MPEG4 I-Frame QP Value";
>  	case V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP:		return "MPEG4 P-Frame QP Value";
>  	case V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP:		return "MPEG4 B-Frame QP Value";
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 3dcfc6148f99..fd65c710b144 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -533,6 +533,7 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type {
>  };
>  #define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER	(V4L2_CID_MPEG_BASE+381)
>  #define V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER_QP	(V4L2_CID_MPEG_BASE+382)
> +#define V4L2_CID_MPEG_VIDEO_H264_CONSTRAINED_INTRA_PREDICTION	(V4L2_CID_MPEG_BASE+383)
>  #define V4L2_CID_MPEG_VIDEO_MPEG4_I_FRAME_QP	(V4L2_CID_MPEG_BASE+400)
>  #define V4L2_CID_MPEG_VIDEO_MPEG4_P_FRAME_QP	(V4L2_CID_MPEG_BASE+401)
>  #define V4L2_CID_MPEG_VIDEO_MPEG4_B_FRAME_QP	(V4L2_CID_MPEG_BASE+402)
> 

Regards,

	Hans