| 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
| ||
|
Message-ID: <0f751cf4a8998b6c6b9046d4e1cb25646709f75a.camel@ndufresne.ca>
Date: Wed, 21 May 2025 14:39:51 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: "Ming Qian(OSS)" <ming.qian@....nxp.com>, mchehab@...nel.org,
hverkuil-cisco@...all.nl
Cc: shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
festevam@...il.com, linux-imx@....com, xiahong.bao@....com,
eagle.zhou@....com, imx@...ts.linux.dev, linux-media@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v4] media: amphion: Add H264 and HEVC profile and level
control
Hi,
Le mercredi 21 mai 2025 à 09:23 +0800, Ming Qian(OSS) a écrit :
>
> Hi Nicolas,
>
> On 2025/5/21 5:04, Nicolas Dufresne wrote:
> > Hi Ming,
> >
> > got one final question below...
> >
> > Le lundi 12 mai 2025 à 10:01 +0800, ming.qian@....nxp.com a écrit :
> > > From: Ming Qian <ming.qian@....nxp.com>
> > >
> > > For format H264 and HEVC, the firmware can report the parsed profile idc
> > > and level idc to driver, these information may be useful.
> > > Implement the H264 and HEVC profile and level control to report them.
> > >
> > > Signed-off-by: Ming Qian <ming.qian@....nxp.com>
> > > ---
> > > v4
> > > - Fix a building warning reported by kernel test robot
> > >
> > > v3
> > > - Check H264 Constrained Baseline Profile
> > > - Check H264 Level 1b
> > > - Remove support for V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH
> > > - Remove support for V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH
> > >
> > > v2
> > > - Add support for V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE
> > > ---
> > > drivers/media/platform/amphion/vdec.c | 59 +++++++++
> > > drivers/media/platform/amphion/vpu_defs.h | 12 ++
> > > drivers/media/platform/amphion/vpu_helpers.c | 123 +++++++++++++++++++
> > > drivers/media/platform/amphion/vpu_helpers.h | 7 ++
> > > drivers/media/platform/amphion/vpu_malone.c | 5 +-
> > > 5 files changed, 205 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
> > > index 85d518823159..55067d9405c2 100644
> > > --- a/drivers/media/platform/amphion/vdec.c
> > > +++ b/drivers/media/platform/amphion/vdec.c
> > > @@ -232,6 +232,35 @@ 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) |
> > > + (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
> > > + (1 << V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED) |
> > > + (1 << V4L2_MPEG_VIDEO_H264_PROFILE_HIGH)),
> > > + V4L2_MPEG_VIDEO_H264_PROFILE_MAIN);
> > > +
> > > + 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 +1195,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));
> > > + vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_H264_LEVEL,
> > > + vpu_get_h264_v4l2_level(hdr));
> > > + break;
> > > + case V4L2_PIX_FMT_HEVC:
> > > + vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_HEVC_PROFILE,
> > > + vpu_get_hevc_v4l2_profile(hdr));
> > > + vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
> > > + vpu_get_hevc_v4l2_level(hdr));
> > > + 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 +1247,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..f56245ae2205 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;
> > > @@ -147,6 +148,17 @@ struct vpu_dec_codec_info {
> > > u32 mbi_size;
> > > u32 dcp_size;
> > > u32 stride;
> > > + union {
> > > + struct {
> > > + u32 constraint_set5_flag : 1;
> > > + u32 constraint_set4_flag : 1;
> > > + u32 constraint_set3_flag : 1;
> > > + u32 constraint_set2_flag : 1;
> > > + u32 constraint_set1_flag : 1;
> > > + u32 constraint_set0_flag : 1;
> > > + };
> > > + u32 constraint_set_flags;
> >
> > Shouldn't this be u8 all over to match the fact the firmware reports this over a single byte ?
> >
> > Nicolas
> >
>
> firmware use u32 to transmit message data:
> struct vpu_rpc_event_header {
> u32 index;
> u32 id;
> u32 num;
> };
>
> struct vpu_rpc_event {
> struct vpu_rpc_event_header hdr;
> u32 data[128];
> };
>
> And in amphion vpu decoder firmware 1.10.0, it increase the data number of
> SEQ_HDR_FOUND event from 28 to 29. the constraint_set_flags is stored in
> the data[28], so although only 6 bits are used, u32 is still used here.
>
> if (pkt->hdr.num > 28)
> info->constraint_set_flags = pkt->data[28];
Ack, I should have checked the definition instead of assuming. Patch queued.
regards
Nicolas
>
> Regards,
> Ming
>
> > > + };
> > > };
> > >
> > > struct vpu_dec_pic_info {
> > > diff --git a/drivers/media/platform/amphion/vpu_helpers.c b/drivers/media/platform/amphion/vpu_helpers.c
> > > index d12310af9ebc..886d5632388e 100644
> > > --- a/drivers/media/platform/amphion/vpu_helpers.c
> > > +++ b/drivers/media/platform/amphion/vpu_helpers.c
> > > @@ -509,3 +509,126 @@ 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}
> > > +};
> > > +
> > > +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(struct vpu_dec_codec_info *hdr)
> > > +{
> > > + if (!hdr)
> > > + return 0;
> > > +
> > > + /*
> > > + * In H.264 Document section A.2.1.1 Constrained Baseline profile
> > > + * Conformance of a bitstream to the Constrained Baseline profile is indicated by
> > > + * profile_idc being equal to 66 with constraint_set1_flag being equal to 1.
> > > + */
> > > + if (hdr->profile_idc == 66 && hdr->constraint_set1_flag)
> > > + return V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE;
> > > +
> > > + return vpu_find_v4l2_id(hdr->profile_idc, h264_profiles, ARRAY_SIZE(h264_profiles));
> > > +}
> > > +
> > > +u32 vpu_get_h264_v4l2_level(struct vpu_dec_codec_info *hdr)
> > > +{
> > > + if (!hdr)
> > > + return 0;
> > > +
> > > + /*
> > > + * In H.264 Document section 7.4.2.1.1 Sequence parameter set data semantics
> > > + * If profile_idc is equal to 66, 77, or 88 and level_idc is equal to 11,
> > > + * constraint_set3_flag equal to 1 indicates that the coded video sequence
> > > + * obeys all constraints specified in Annex A for level 1b
> > > + * and constraint_set3_flag equal to 0 indicates that the coded video sequence
> > > + * obeys all constraints specified in Annex A for level 1.1.
> > > + */
> > > + if (hdr->level_idc == 11 && hdr->constraint_set3_flag &&
> > > + (hdr->profile_idc == 66 || hdr->profile_idc == 77 || hdr->profile_idc == 88))
> > > + return V4L2_MPEG_VIDEO_H264_LEVEL_1B;
> > > +
> > > + return vpu_find_v4l2_id(hdr->level_idc, h264_levels, ARRAY_SIZE(h264_levels));
> > > +}
> > > +
> > > +u32 vpu_get_hevc_v4l2_profile(struct vpu_dec_codec_info *hdr)
> > > +{
> > > + if (!hdr)
> > > + return 0;
> > > +
> > > + return vpu_find_v4l2_id(hdr->profile_idc, hevc_profiles, ARRAY_SIZE(hevc_profiles));
> > > +}
> > > +
> > > +u32 vpu_get_hevc_v4l2_level(struct vpu_dec_codec_info *hdr)
> > > +{
> > > + if (!hdr)
> > > + return 0;
> > > +
> > > + return vpu_find_v4l2_id(hdr->level_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..6c0554ccf3b3 100644
> > > --- a/drivers/media/platform/amphion/vpu_helpers.h
> > > +++ b/drivers/media/platform/amphion/vpu_helpers.h
> > > @@ -6,6 +6,8 @@
> > > #ifndef _AMPHION_VPU_HELPERS_H
> > > #define _AMPHION_VPU_HELPERS_H
> > >
> > > +#include "vpu_defs.h"
> > > +
> > > struct vpu_pair {
> > > u32 src;
> > > u32 dst;
> > > @@ -70,4 +72,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(struct vpu_dec_codec_info *hdr);
> > > +u32 vpu_get_h264_v4l2_level(struct vpu_dec_codec_info *hdr);
> > > +u32 vpu_get_hevc_v4l2_profile(struct vpu_dec_codec_info *hdr);
> > > +u32 vpu_get_hevc_v4l2_level(struct vpu_dec_codec_info *hdr);
> > > #endif
> > > diff --git a/drivers/media/platform/amphion/vpu_malone.c b/drivers/media/platform/amphion/vpu_malone.c
> > > index feca7d4220ed..ba688566dffd 100644
> > > --- a/drivers/media/platform/amphion/vpu_malone.c
> > > +++ b/drivers/media/platform/amphion/vpu_malone.c
> > > @@ -908,7 +908,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;
> > > + 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];
> > > @@ -925,6 +926,8 @@ static void vpu_malone_unpack_seq_hdr(struct vpu_rpc_event *pkt,
> > > info->pixfmt = V4L2_PIX_FMT_NV12M_10BE_8L128;
> > > else
> > > info->pixfmt = V4L2_PIX_FMT_NV12M_8L128;
> > > + if (pkt->hdr.num > 28)
> > > + info->constraint_set_flags = pkt->data[28];
> > > if (info->frame_rate.numerator && info->frame_rate.denominator) {
> > > unsigned long n, d;
> > >
> > >
> > > base-commit: b64b134942c8cf4801ea288b3fd38b509aedec21
Powered by blists - more mailing lists