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: <74eb0589ae54855db1d2024591d501989e30027a.camel@ndufresne.ca>
Date: Tue, 22 Apr 2025 16:15:25 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Ming Qian <ming.qian@....nxp.com>, mchehab@...nel.org, 
	hverkuil-cisco@...all.nl
Cc: shawnguo@...nel.org, robh+dt@...nel.org, s.hauer@...gutronix.de, 
	kernel@...gutronix.de, festevam@...il.com, linux-imx@....com,
 xiahong.bao@....com, 	eagle.zhou@....com, tao.jiang_2@....com,
 imx@...ts.linux.dev, 	linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org, 	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] media: amphion: Add H264 and HEVC profile and level
 control

Hi Ming,

Le mardi 10 décembre 2024 à 18:33 +0900, Ming Qian a écrit :
> From: Ming Qian <ming.qian@....com>
> 
> For format H264 and HEVC, the firmware can report the parsed profile idc
> and level idc to driver, the information may be useful.
> Implement the H264 and HEVC profile and level control to report them.
> 
> Signed-off-by: Ming Qian <ming.qian@....com>
> ---
> v2
> -- add support for V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE
> 
>  drivers/media/platform/amphion/vdec.c        | 61 +++++++++++++
>  drivers/media/platform/amphion/vpu_defs.h    |  1 +
>  drivers/media/platform/amphion/vpu_helpers.c | 93 ++++++++++++++++++++
>  drivers/media/platform/amphion/vpu_helpers.h |  5 ++
>  drivers/media/platform/amphion/vpu_malone.c  |  3 +-
>  5 files changed, 162 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
> index ca8f7319503a..61d5598ee6a1 100644
> --- a/drivers/media/platform/amphion/vdec.c
> +++ b/drivers/media/platform/amphion/vdec.c
> @@ -232,6 +232,37 @@ static int vdec_ctrl_init(struct vpu_inst *inst)
>  			  V4L2_CID_MPEG_VIDEO_DEC_DISPLAY_DELAY_ENABLE,
>  			  0, 1, 1, 0);
>  
> +	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, NULL,
> +			       V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> +			       V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH,
> +			       ~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
> +				 (1 << V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) |

You've added it here, but you will never report it, see my comment
below.

> +				 (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
> +				 (1 << V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED) |
> +				 (1 << V4L2_MPEG_VIDEO_H264_PROFILE_HIGH) |
> +				 (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH) |
> +				 (1 << V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH)),

Shall we advertise multiview and stereo ? My impression is that we lack
a mechanism to actually signal the stereo layout, or which view each
buffers came from. I'm thinking, you can can't test it, we should just
fail on these ?

> +			       V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE);
> +
> +	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, NULL,
> +			       V4L2_CID_MPEG_VIDEO_H264_LEVEL,
> +			       V4L2_MPEG_VIDEO_H264_LEVEL_6_2,
> +			       0,
> +			       V4L2_MPEG_VIDEO_H264_LEVEL_4_0);
> +
> +	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, NULL,
> +			       V4L2_CID_MPEG_VIDEO_HEVC_PROFILE,
> +			       V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10,
> +			       ~((1 << V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN) |
> +				 (1 << V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10)),
> +			       V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN);
> +
> +	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, NULL,
> +			       V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
> +			       V4L2_MPEG_VIDEO_HEVC_LEVEL_6_2,
> +			       0,
> +			       V4L2_MPEG_VIDEO_HEVC_LEVEL_4);
> +
>  	ctrl = v4l2_ctrl_new_std(&inst->ctrl_handler, &vdec_ctrl_ops,
>  				 V4L2_CID_MIN_BUFFERS_FOR_CAPTURE, 1, 32, 1, 2);
>  	if (ctrl)
> @@ -1166,6 +1197,35 @@ static void vdec_clear_slots(struct vpu_inst *inst)
>  	}
>  }
>  
> +static void vdec_update_v4l2_ctrl(struct vpu_inst *inst, u32 id, u32 val)
> +{
> +	struct v4l2_ctrl *ctrl = v4l2_ctrl_find(&inst->ctrl_handler, id);
> +
> +	if (ctrl)
> +		v4l2_ctrl_s_ctrl(ctrl, val);
> +}
> +
> +static void vdec_update_v4l2_profile_level(struct vpu_inst *inst, struct vpu_dec_codec_info *hdr)
> +{
> +	switch (inst->out_format.pixfmt) {
> +	case V4L2_PIX_FMT_H264:
> +	case V4L2_PIX_FMT_H264_MVC:
> +		vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> +				      vpu_get_h264_v4l2_profile(hdr->profile_idc));
> +		vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_H264_LEVEL,
> +				      vpu_get_h264_v4l2_level(hdr->level_idc));
> +		break;
> +	case V4L2_PIX_FMT_HEVC:
> +		vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_HEVC_PROFILE,
> +				      vpu_get_hevc_v4l2_profile(hdr->profile_idc));
> +		vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
> +				      vpu_get_hevc_v4l2_level(hdr->level_idc));
> +		break;
> +	default:
> +		return;
> +	}
> +}
> +
>  static void vdec_event_seq_hdr(struct vpu_inst *inst, struct vpu_dec_codec_info *hdr)
>  {
>  	struct vdec_t *vdec = inst->priv;
> @@ -1189,6 +1249,7 @@ static void vdec_event_seq_hdr(struct vpu_inst *inst, struct vpu_dec_codec_info
>  	vdec_init_crop(inst);
>  	vdec_init_mbi(inst);
>  	vdec_init_dcp(inst);
> +	vdec_update_v4l2_profile_level(inst, hdr);
>  	if (!vdec->seq_hdr_found) {
>  		vdec->seq_tag = vdec->codec_info.tag;
>  		if (vdec->is_source_changed) {
> diff --git a/drivers/media/platform/amphion/vpu_defs.h b/drivers/media/platform/amphion/vpu_defs.h
> index 428d988cf2f7..606f9d61a265 100644
> --- a/drivers/media/platform/amphion/vpu_defs.h
> +++ b/drivers/media/platform/amphion/vpu_defs.h
> @@ -134,6 +134,7 @@ struct vpu_dec_codec_info {
>  	u32 decoded_height;
>  	struct v4l2_fract frame_rate;
>  	u32 dsp_asp_ratio;
> +	u32 profile_idc;
>  	u32 level_idc;
>  	u32 bit_depth_luma;
>  	u32 bit_depth_chroma;
> diff --git a/drivers/media/platform/amphion/vpu_helpers.c b/drivers/media/platform/amphion/vpu_helpers.c
> index d12310af9ebc..108b75ceb4ae 100644
> --- a/drivers/media/platform/amphion/vpu_helpers.c
> +++ b/drivers/media/platform/amphion/vpu_helpers.c
> @@ -509,3 +509,96 @@ const char *vpu_codec_state_name(enum vpu_codec_state state)
>  	}
>  	return "<unknown>";
>  }
> +
> +struct codec_id_mapping {
> +	u32 id;
> +	u32 v4l2_id;
> +};
> +
> +static struct codec_id_mapping h264_profiles[] = {
> +	{66,  V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE},
> +	{77,  V4L2_MPEG_VIDEO_H264_PROFILE_MAIN},
> +	{88,  V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED},
> +	{100, V4L2_MPEG_VIDEO_H264_PROFILE_HIGH},
> +	{118, V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH},
> +	{128, V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH}
> +};
> +
> +static struct codec_id_mapping h264_levels[] = {
> +	{10,  V4L2_MPEG_VIDEO_H264_LEVEL_1_0},
> +	{9,   V4L2_MPEG_VIDEO_H264_LEVEL_1B},
> +	{11,  V4L2_MPEG_VIDEO_H264_LEVEL_1_1},
> +	{12,  V4L2_MPEG_VIDEO_H264_LEVEL_1_2},
> +	{13,  V4L2_MPEG_VIDEO_H264_LEVEL_1_3},
> +	{20,  V4L2_MPEG_VIDEO_H264_LEVEL_2_0},
> +	{21,  V4L2_MPEG_VIDEO_H264_LEVEL_2_1},
> +	{22,  V4L2_MPEG_VIDEO_H264_LEVEL_2_2},
> +	{30,  V4L2_MPEG_VIDEO_H264_LEVEL_3_0},
> +	{31,  V4L2_MPEG_VIDEO_H264_LEVEL_3_1},
> +	{32,  V4L2_MPEG_VIDEO_H264_LEVEL_3_2},
> +	{40,  V4L2_MPEG_VIDEO_H264_LEVEL_4_0},
> +	{41,  V4L2_MPEG_VIDEO_H264_LEVEL_4_1},
> +	{42,  V4L2_MPEG_VIDEO_H264_LEVEL_4_2},
> +	{50,  V4L2_MPEG_VIDEO_H264_LEVEL_5_0},
> +	{51,  V4L2_MPEG_VIDEO_H264_LEVEL_5_1},
> +	{52,  V4L2_MPEG_VIDEO_H264_LEVEL_5_2},
> +	{60,  V4L2_MPEG_VIDEO_H264_LEVEL_6_0},
> +	{61,  V4L2_MPEG_VIDEO_H264_LEVEL_6_1},
> +	{62,  V4L2_MPEG_VIDEO_H264_LEVEL_6_2}
> +};
> +
> +static struct codec_id_mapping hevc_profiles[] = {
> +	{1,   V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN},
> +	{2,   V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10}
> +};
> +
> +static struct codec_id_mapping hevc_levels[] = {
> +	{30,  V4L2_MPEG_VIDEO_HEVC_LEVEL_1},
> +	{60,  V4L2_MPEG_VIDEO_HEVC_LEVEL_2},
> +	{63,  V4L2_MPEG_VIDEO_HEVC_LEVEL_2_1},
> +	{90,  V4L2_MPEG_VIDEO_HEVC_LEVEL_3},
> +	{93,  V4L2_MPEG_VIDEO_HEVC_LEVEL_3_1},
> +	{120, V4L2_MPEG_VIDEO_HEVC_LEVEL_4},
> +	{123, V4L2_MPEG_VIDEO_HEVC_LEVEL_4_1},
> +	{150, V4L2_MPEG_VIDEO_HEVC_LEVEL_5},
> +	{153, V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1},
> +	{156, V4L2_MPEG_VIDEO_HEVC_LEVEL_5_2},
> +	{180, V4L2_MPEG_VIDEO_HEVC_LEVEL_6},
> +	{183, V4L2_MPEG_VIDEO_HEVC_LEVEL_6_1},
> +	{186, V4L2_MPEG_VIDEO_HEVC_LEVEL_6_2}
> +};
> +
> +static u32 vpu_find_v4l2_id(u32 id, struct codec_id_mapping *array, u32 array_sz)
> +{
> +	u32 i;
> +
> +	if (!array || !array_sz)
> +		return 0;
> +
> +	for (i = 0; i < array_sz; i++) {
> +		if (id == array[i].id)
> +			return array[i].v4l2_id;
> +	}
> +
> +	return 0;
> +}
> +
> +u32 vpu_get_h264_v4l2_profile(u32 idc)
> +{
> +	return vpu_find_v4l2_id(idc, h264_profiles, ARRAY_SIZE(h264_profiles));
> +}
> +
> +u32 vpu_get_h264_v4l2_level(u32 idc)
> +{
> +	return vpu_find_v4l2_id(idc, h264_levels, ARRAY_SIZE(h264_levels));
> +}
> +
> +u32 vpu_get_hevc_v4l2_profile(u32 idc)
> +{
> +	return vpu_find_v4l2_id(idc, hevc_profiles, ARRAY_SIZE(hevc_profiles));
> +}
> +
> +u32 vpu_get_hevc_v4l2_level(u32 idc)
> +{
> +	return vpu_find_v4l2_id(idc, hevc_levels, ARRAY_SIZE(hevc_levels));
> +}
> diff --git a/drivers/media/platform/amphion/vpu_helpers.h b/drivers/media/platform/amphion/vpu_helpers.h
> index 0eaddb07190d..dc5fb1ca2d33 100644
> --- a/drivers/media/platform/amphion/vpu_helpers.h
> +++ b/drivers/media/platform/amphion/vpu_helpers.h
> @@ -70,4 +70,9 @@ int vpu_color_get_default(u32 primaries, u32 *ptransfers, u32 *pmatrix, u32 *pfu
>  
>  int vpu_find_dst_by_src(struct vpu_pair *pairs, u32 cnt, u32 src);
>  int vpu_find_src_by_dst(struct vpu_pair *pairs, u32 cnt, u32 dst);
> +
> +u32 vpu_get_h264_v4l2_profile(u32 idc);
> +u32 vpu_get_h264_v4l2_level(u32 idc);
> +u32 vpu_get_hevc_v4l2_profile(u32 idc);
> +u32 vpu_get_hevc_v4l2_level(u32 idc);
>  #endif
> diff --git a/drivers/media/platform/amphion/vpu_malone.c b/drivers/media/platform/amphion/vpu_malone.c
> index 4769c053c6c2..5c6b2a841b6f 100644
> --- a/drivers/media/platform/amphion/vpu_malone.c
> +++ b/drivers/media/platform/amphion/vpu_malone.c
> @@ -889,7 +889,8 @@ static void vpu_malone_unpack_seq_hdr(struct vpu_rpc_event *pkt,
>  	info->frame_rate.numerator = 1000;
>  	info->frame_rate.denominator = pkt->data[8];
>  	info->dsp_asp_ratio = pkt->data[9];
> -	info->level_idc = pkt->data[10];
> +	info->profile_idc = (pkt->data[10] >> 8) & 0xff;

The data should normally also include the sps_constraint_set1_flag,
which differentiate baseline from constrained-baseline. I would also be
very surprised if the decoders supports ASO/FMO.

Nicolas

> +	info->level_idc = pkt->data[10] & 0xff;
>  	info->bit_depth_luma = pkt->data[13];
>  	info->bit_depth_chroma = pkt->data[14];
>  	info->chroma_fmt = pkt->data[15];

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ