It's my mistake that forget to remove H264_PROF_AUTO. I will fix that. Any other suggestions about the profile_options? Thanks.
------------------ ???????? ------------------ ??????: ""zhilizhao(??????)"" <quinkbl...@foxmail.com>; ????????: 2023??5??22??(??????) ????11:11 ??????: "FFmpeg development discussions and patches"<ffmpeg-devel@ffmpeg.org>; ????: "??????"<839789...@qq.com>;"Rick Kern"<ker...@gmail.com>; ????: Re: [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: replace VT_H264Profile with avctx profile > On May 22, 2023, at 11:05, zhilizhao(??????) <quinkbl...@foxmail.com> wrote: > >> On May 21, 2023, at 22:41, xufuji456 <839789...@qq.com> wrote: >> >> For compatibility with constrained_baseline in the future, >> replace VT_H264Profile/VT_HEVCProfile with avctx->profile. >> >> Signed-off-by: xufuji456 <839789...@qq.com> >> --- >> libavcodec/videotoolboxenc.c | 55 +++++++++++------------------------- >> 1 file changed, 16 insertions(+), 39 deletions(-) >> >> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c >> index b017c90c36..4966ab36ae 100644 >> --- a/libavcodec/videotoolboxenc.c >> +++ b/libavcodec/videotoolboxenc.c >> @@ -190,28 +190,12 @@ static void loadVTEncSymbols(void){ >> "EnableLowLatencyRateControl"); >> } >> >> -typedef enum VT_H264Profile { >> - H264_PROF_AUTO, >> - H264_PROF_BASELINE, >> - H264_PROF_MAIN, >> - H264_PROF_HIGH, >> - H264_PROF_EXTENDED, >> - H264_PROF_COUNT >> -} VT_H264Profile; >> - >> typedef enum VTH264Entropy{ >> VT_ENTROPY_NOT_SET, >> VT_CAVLC, >> VT_CABAC >> } VTH264Entropy; >> >> -typedef enum VT_HEVCProfile { >> - HEVC_PROF_AUTO, >> - HEVC_PROF_MAIN, >> - HEVC_PROF_MAIN10, >> - HEVC_PROF_COUNT >> -} VT_HEVCProfile; >> - >> static const uint8_t start_code[] = { 0, 0, 0, 1 }; >> >> typedef struct ExtraSEI { >> @@ -730,18 +714,13 @@ static bool get_vt_h264_profile_level(AVCodecContext *avctx, >> VTEncContext *vtctx = avctx->priv_data; >> int64_t profile = vtctx->profile; >> >> - if (profile == H264_PROF_AUTO && vtctx->level) { >> - //Need to pick a profile if level is not auto-selected. >> - profile = vtctx->has_b_frames ? H264_PROF_MAIN : H264_PROF_BASELINE; >> - } >> - >> *profile_level_val = NULL; >> >> switch (profile) { >> case H264_PROF_AUTO: >> return true; Isn??t it failed to build since H264_PROF_AUTO isn??t defined? >> >> - case H264_PROF_BASELINE: >> + case FF_PROFILE_H264_BASELINE: >> switch (vtctx->level) { >> case 0: *profile_level_val = >> compat_keys.kVTProfileLevel_H264_Baseline_AutoLevel; break; >> @@ -763,7 +742,7 @@ static bool get_vt_h264_profile_level(AVCodecContext *avctx, >> } >> break; >> >> - case H264_PROF_MAIN: >> + case FF_PROFILE_H264_MAIN: >> switch (vtctx->level) { >> case 0: *profile_level_val = >> compat_keys.kVTProfileLevel_H264_Main_AutoLevel; break; >> @@ -782,7 +761,7 @@ static bool get_vt_h264_profile_level(AVCodecContext *avctx, >> } >> break; >> >> - case H264_PROF_HIGH: >> + case FF_PROFILE_H264_HIGH: >> switch (vtctx->level) { >> case 0: *profile_level_val = >> compat_keys.kVTProfileLevel_H264_High_AutoLevel; break; >> @@ -805,7 +784,7 @@ static bool get_vt_h264_profile_level(AVCodecContext *avctx, >> compat_keys.kVTProfileLevel_H264_High_5_2; break; >> } >> break; >> - case H264_PROF_EXTENDED: >> + case FF_PROFILE_H264_EXTENDED: >> switch (vtctx->level) { >> case 0: *profile_level_val = >> compat_keys.kVTProfileLevel_H264_Extended_AutoLevel; break; >> @@ -838,13 +817,11 @@ static bool get_vt_hevc_profile_level(AVCodecContext *avctx, >> *profile_level_val = NULL; >> >> switch (profile) { >> - case HEVC_PROF_AUTO: >> - return true; >> - case HEVC_PROF_MAIN: >> + case FF_PROFILE_HEVC_MAIN: >> *profile_level_val = >> compat_keys.kVTProfileLevel_HEVC_Main_AutoLevel; >> break; >> - case HEVC_PROF_MAIN10: >> + case FF_PROFILE_HEVC_MAIN_10: >> *profile_level_val = >> compat_keys.kVTProfileLevel_HEVC_Main10_AutoLevel; >> break; >> @@ -1515,12 +1492,12 @@ static int vtenc_configure_encoder(AVCodecContext *avctx) >> vtctx->get_param_set_func = CMVideoFormatDescriptionGetH264ParameterSetAtIndex; >> >> vtctx->has_b_frames = avctx->max_b_frames > 0; >> - if(vtctx->has_b_frames && vtctx->profile == H264_PROF_BASELINE){ >> + if(vtctx->has_b_frames && vtctx->profile == FF_PROFILE_H264_BASELINE) { >> av_log(avctx, AV_LOG_WARNING, "Cannot use B-frames with baseline profile. Output will not contain B-frames.\n"); >> vtctx->has_b_frames = 0; >> } >> >> - if (vtctx->entropy == VT_CABAC && vtctx->profile == H264_PROF_BASELINE) { >> + if (vtctx->entropy == VT_CABAC && vtctx->profile == FF_PROFILE_H264_BASELINE) { >> av_log(avctx, AV_LOG_WARNING, "CABAC entropy requires 'main' or 'high' profile, but baseline was requested. Encode will not use CABAC entropy.\n"); >> vtctx->entropy = VT_ENTROPY_NOT_SET; >> } >> @@ -2756,11 +2733,11 @@ static const enum AVPixelFormat prores_pix_fmts[] = { >> >> #define OFFSET(x) offsetof(VTEncContext, x) >> static const AVOption h264_options[] = { >> - { "profile", "Profile", OFFSET(profile), AV_OPT_TYPE_INT64, { .i64 = H264_PROF_AUTO }, H264_PROF_AUTO, H264_PROF_COUNT, VE, "profile" }, >> - { "baseline", "Baseline Profile", 0, AV_OPT_TYPE_CONST, { .i64 = H264_PROF_BASELINE }, INT_MIN, INT_MAX, VE, "profile" }, >> - { "main", "Main Profile", 0, AV_OPT_TYPE_CONST, { .i64 = H264_PROF_MAIN }, INT_MIN, INT_MAX, VE, "profile" }, >> - { "high", "High Profile", 0, AV_OPT_TYPE_CONST, { .i64 = H264_PROF_HIGH }, INT_MIN, INT_MAX, VE, "profile" }, >> - { "extended", "Extend Profile", 0, AV_OPT_TYPE_CONST, { .i64 = H264_PROF_EXTENDED }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "profile", "Profile", OFFSET(profile), AV_OPT_TYPE_INT64, { .i64 = FF_PROFILE_H264_BASELINE }, FF_PROFILE_H264_BASELINE, FF_PROFILE_H264_HIGH, VE, "profile" }, >> + { "baseline", "Baseline Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_BASELINE }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "main", "Main Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_MAIN }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "extended", "Extend Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_EXTENDED }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "high", "High Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_HIGH }, INT_MIN, INT_MAX, VE, "profile" }, > > Firstly, it breaks use case which set the option by number like -profile 1. > > Secondly, it changes the default profile to baseline. I don??t think it??s a > good idea. > >> >> { "level", "Level", OFFSET(level), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 52, VE, "level" }, >> { "1.3", "Level 1.3, only available with Baseline Profile", 0, AV_OPT_TYPE_CONST, { .i64 = 13 }, INT_MIN, INT_MAX, VE, "level" }, >> @@ -2811,9 +2788,9 @@ const FFCodec ff_h264_videotoolbox_encoder = { >> }; >> >> static const AVOption hevc_options[] = { >> - { "profile", "Profile", OFFSET(profile), AV_OPT_TYPE_INT64, { .i64 = HEVC_PROF_AUTO }, HEVC_PROF_AUTO, HEVC_PROF_COUNT, VE, "profile" }, >> - { "main", "Main Profile", 0, AV_OPT_TYPE_CONST, { .i64 = HEVC_PROF_MAIN }, INT_MIN, INT_MAX, VE, "profile" }, >> - { "main10", "Main10 Profile", 0, AV_OPT_TYPE_CONST, { .i64 = HEVC_PROF_MAIN10 }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "profile", "Profile", OFFSET(profile), AV_OPT_TYPE_INT64, { .i64 = FF_PROFILE_HEVC_MAIN }, FF_PROFILE_HEVC_MAIN, FF_PROFILE_HEVC_MAIN_10, VE, "profile" }, >> + { "main", "Main Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_HEVC_MAIN }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "main10", "Main10 Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_HEVC_MAIN_10 }, INT_MIN, INT_MAX, VE, "profile" }, >> >> { "alpha_quality", "Compression quality for the alpha channel", OFFSET(alpha_quality), AV_OPT_TYPE_DOUBLE, { .dbl = 0.0 }, 0.0, 1.0, VE }, >> >> -- >> 2.32.0 (Apple Git-132) >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > >> On May 21, 2023, at 22:41, xufuji456 <839789...@qq.com> wrote: >> >> For compatibility with constrained_baseline in the future, >> replace VT_H264Profile/VT_HEVCProfile with avctx->profile. >> >> Signed-off-by: xufuji456 <839789...@qq.com> >> --- >> libavcodec/videotoolboxenc.c | 55 +++++++++++------------------------- >> 1 file changed, 16 insertions(+), 39 deletions(-) >> >> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c >> index b017c90c36..4966ab36ae 100644 >> --- a/libavcodec/videotoolboxenc.c >> +++ b/libavcodec/videotoolboxenc.c >> @@ -190,28 +190,12 @@ static void loadVTEncSymbols(void){ >> "EnableLowLatencyRateControl"); >> } >> >> -typedef enum VT_H264Profile { >> - H264_PROF_AUTO, >> - H264_PROF_BASELINE, >> - H264_PROF_MAIN, >> - H264_PROF_HIGH, >> - H264_PROF_EXTENDED, >> - H264_PROF_COUNT >> -} VT_H264Profile; >> - >> typedef enum VTH264Entropy{ >> VT_ENTROPY_NOT_SET, >> VT_CAVLC, >> VT_CABAC >> } VTH264Entropy; >> >> -typedef enum VT_HEVCProfile { >> - HEVC_PROF_AUTO, >> - HEVC_PROF_MAIN, >> - HEVC_PROF_MAIN10, >> - HEVC_PROF_COUNT >> -} VT_HEVCProfile; >> - >> static const uint8_t start_code[] = { 0, 0, 0, 1 }; >> >> typedef struct ExtraSEI { >> @@ -730,18 +714,13 @@ static bool get_vt_h264_profile_level(AVCodecContext *avctx, >> VTEncContext *vtctx = avctx->priv_data; >> int64_t profile = vtctx->profile; >> >> - if (profile == H264_PROF_AUTO && vtctx->level) { >> - //Need to pick a profile if level is not auto-selected. >> - profile = vtctx->has_b_frames ? H264_PROF_MAIN : H264_PROF_BASELINE; >> - } >> - >> *profile_level_val = NULL; >> >> switch (profile) { >> case H264_PROF_AUTO: >> return true; >> >> - case H264_PROF_BASELINE: >> + case FF_PROFILE_H264_BASELINE: >> switch (vtctx->level) { >> case 0: *profile_level_val = >> compat_keys.kVTProfileLevel_H264_Baseline_AutoLevel; break; >> @@ -763,7 +742,7 @@ static bool get_vt_h264_profile_level(AVCodecContext *avctx, >> } >> break; >> >> - case H264_PROF_MAIN: >> + case FF_PROFILE_H264_MAIN: >> switch (vtctx->level) { >> case 0: *profile_level_val = >> compat_keys.kVTProfileLevel_H264_Main_AutoLevel; break; >> @@ -782,7 +761,7 @@ static bool get_vt_h264_profile_level(AVCodecContext *avctx, >> } >> break; >> >> - case H264_PROF_HIGH: >> + case FF_PROFILE_H264_HIGH: >> switch (vtctx->level) { >> case 0: *profile_level_val = >> compat_keys.kVTProfileLevel_H264_High_AutoLevel; break; >> @@ -805,7 +784,7 @@ static bool get_vt_h264_profile_level(AVCodecContext *avctx, >> compat_keys.kVTProfileLevel_H264_High_5_2; break; >> } >> break; >> - case H264_PROF_EXTENDED: >> + case FF_PROFILE_H264_EXTENDED: >> switch (vtctx->level) { >> case 0: *profile_level_val = >> compat_keys.kVTProfileLevel_H264_Extended_AutoLevel; break; >> @@ -838,13 +817,11 @@ static bool get_vt_hevc_profile_level(AVCodecContext *avctx, >> *profile_level_val = NULL; >> >> switch (profile) { >> - case HEVC_PROF_AUTO: >> - return true; >> - case HEVC_PROF_MAIN: >> + case FF_PROFILE_HEVC_MAIN: >> *profile_level_val = >> compat_keys.kVTProfileLevel_HEVC_Main_AutoLevel; >> break; >> - case HEVC_PROF_MAIN10: >> + case FF_PROFILE_HEVC_MAIN_10: >> *profile_level_val = >> compat_keys.kVTProfileLevel_HEVC_Main10_AutoLevel; >> break; >> @@ -1515,12 +1492,12 @@ static int vtenc_configure_encoder(AVCodecContext *avctx) >> vtctx->get_param_set_func = CMVideoFormatDescriptionGetH264ParameterSetAtIndex; >> >> vtctx->has_b_frames = avctx->max_b_frames > 0; >> - if(vtctx->has_b_frames && vtctx->profile == H264_PROF_BASELINE){ >> + if(vtctx->has_b_frames && vtctx->profile == FF_PROFILE_H264_BASELINE) { >> av_log(avctx, AV_LOG_WARNING, "Cannot use B-frames with baseline profile. Output will not contain B-frames.\n"); >> vtctx->has_b_frames = 0; >> } >> >> - if (vtctx->entropy == VT_CABAC && vtctx->profile == H264_PROF_BASELINE) { >> + if (vtctx->entropy == VT_CABAC && vtctx->profile == FF_PROFILE_H264_BASELINE) { >> av_log(avctx, AV_LOG_WARNING, "CABAC entropy requires 'main' or 'high' profile, but baseline was requested. Encode will not use CABAC entropy.\n"); >> vtctx->entropy = VT_ENTROPY_NOT_SET; >> } >> @@ -2756,11 +2733,11 @@ static const enum AVPixelFormat prores_pix_fmts[] = { >> >> #define OFFSET(x) offsetof(VTEncContext, x) >> static const AVOption h264_options[] = { >> - { "profile", "Profile", OFFSET(profile), AV_OPT_TYPE_INT64, { .i64 = H264_PROF_AUTO }, H264_PROF_AUTO, H264_PROF_COUNT, VE, "profile" }, >> - { "baseline", "Baseline Profile", 0, AV_OPT_TYPE_CONST, { .i64 = H264_PROF_BASELINE }, INT_MIN, INT_MAX, VE, "profile" }, >> - { "main", "Main Profile", 0, AV_OPT_TYPE_CONST, { .i64 = H264_PROF_MAIN }, INT_MIN, INT_MAX, VE, "profile" }, >> - { "high", "High Profile", 0, AV_OPT_TYPE_CONST, { .i64 = H264_PROF_HIGH }, INT_MIN, INT_MAX, VE, "profile" }, >> - { "extended", "Extend Profile", 0, AV_OPT_TYPE_CONST, { .i64 = H264_PROF_EXTENDED }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "profile", "Profile", OFFSET(profile), AV_OPT_TYPE_INT64, { .i64 = FF_PROFILE_H264_BASELINE }, FF_PROFILE_H264_BASELINE, FF_PROFILE_H264_HIGH, VE, "profile" }, >> + { "baseline", "Baseline Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_BASELINE }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "main", "Main Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_MAIN }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "extended", "Extend Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_EXTENDED }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "high", "High Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_H264_HIGH }, INT_MIN, INT_MAX, VE, "profile" }, > > Firstly, it breaks use case which set the option by number like -profile 1. > > Secondly, it changes the default profile to baseline. I don??t think it??s a > good idea. > >> >> { "level", "Level", OFFSET(level), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 52, VE, "level" }, >> { "1.3", "Level 1.3, only available with Baseline Profile", 0, AV_OPT_TYPE_CONST, { .i64 = 13 }, INT_MIN, INT_MAX, VE, "level" }, >> @@ -2811,9 +2788,9 @@ const FFCodec ff_h264_videotoolbox_encoder = { >> }; >> >> static const AVOption hevc_options[] = { >> - { "profile", "Profile", OFFSET(profile), AV_OPT_TYPE_INT64, { .i64 = HEVC_PROF_AUTO }, HEVC_PROF_AUTO, HEVC_PROF_COUNT, VE, "profile" }, >> - { "main", "Main Profile", 0, AV_OPT_TYPE_CONST, { .i64 = HEVC_PROF_MAIN }, INT_MIN, INT_MAX, VE, "profile" }, >> - { "main10", "Main10 Profile", 0, AV_OPT_TYPE_CONST, { .i64 = HEVC_PROF_MAIN10 }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "profile", "Profile", OFFSET(profile), AV_OPT_TYPE_INT64, { .i64 = FF_PROFILE_HEVC_MAIN }, FF_PROFILE_HEVC_MAIN, FF_PROFILE_HEVC_MAIN_10, VE, "profile" }, >> + { "main", "Main Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_HEVC_MAIN }, INT_MIN, INT_MAX, VE, "profile" }, >> + { "main10", "Main10 Profile", 0, AV_OPT_TYPE_CONST, { .i64 = FF_PROFILE_HEVC_MAIN_10 }, INT_MIN, INT_MAX, VE, "profile" }, >> >> { "alpha_quality", "Compression quality for the alpha channel", OFFSET(alpha_quality), AV_OPT_TYPE_DOUBLE, { .dbl = 0.0 }, 0.0, 1.0, VE }, >> >> -- >> 2.32.0 (Apple Git-132) >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".