[<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