[FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: replace VT_H264Profile with avctx profile
Rick Kern
kernrj at gmail.com
Wed May 24 16:37:51 EEST 2023
On Mon, May 22, 2023 at 12:17 AM 徐福隆 <839789740 at qq.com> wrote:
> It's my mistake that forget to remove H264_PROF_AUTO. I will fix that.
> Any other suggestions about the profile_options? Thanks.
>
Nothing else from me - just the default profile selection behavior
zhilizhao mentioned.
>
> ------------------ 原始邮件 ------------------
> *发件人:* ""zhilizhao(赵志立)"" <quinkblack at foxmail.com>;
> *发送时间:* 2023年5月22日(星期一) 中午11:11
> *收件人:* "FFmpeg development discussions and patches"<
> ffmpeg-devel at ffmpeg.org>;
> *抄送:* "徐福隆"<839789740 at qq.com>;"Rick Kern"<kernrj at gmail.com>;
> *主题:* Re: [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: replace
> VT_H264Profile with avctx profile
>
>
>
> > On May 22, 2023, at 11:05, zhilizhao(赵志立) <quinkblack at foxmail.com>
> wrote:
> >
> >> On May 21, 2023, at 22:41, xufuji456 <839789740 at qq.com> wrote:
> >>
> >> For compatibility with constrained_baseline in the future,
> >> replace VT_H264Profile/VT_HEVCProfile with avctx->profile.
> >>
> >> Signed-off-by: xufuji456 <839789740 at 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?
>
Please be sure to compile and test before submitting a patch.
>
> >>
> >> - 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.
>
Only the string representation of the profiles are documented. The mapping
from integer to profile isn't. Moving to the global constants is also less
confusing for users if profiles are consistent across encoders, and less
confusing for other maintainers looking at this code.
> >
> > Secondly, it changes the default profile to baseline. I don’t think it’s
> a
> > good idea.
>
I agree. We still need the default setting to be "auto" because we need to
know when the user doesn't specify a value. In this case, it uses a sane
default, main or baseline profile, depending on whether B-frames are
present or not.
> >
> >>
> >> { "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 at ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >
> >> On May 21, 2023, at 22:41, xufuji456 <839789740 at qq.com> wrote:
> >>
> >> For compatibility with constrained_baseline in the future,
> >> replace VT_H264Profile/VT_HEVCProfile with avctx->profile.
> >>
> >> Signed-off-by: xufuji456 <839789740 at 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 at ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
More information about the ffmpeg-devel
mailing list