| 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: <36d644aa-5143-4a5a-a01e-a67792288ba3@oss.nxp.com>
Date: Wed, 21 May 2025 09:23:56 +0800
From: "Ming Qian(OSS)" <ming.qian@....nxp.com>
To: Nicolas Dufresne <nicolas@...fresne.ca>, 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 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];
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