[FFmpeg-devel] 回复: [PATCH] avcodec/videotoolboxenc: replace VT_H264Profile with avctx profile

徐福隆 839789740 at qq.com
Mon May 22 07:17:07 EEST 2023


It's my mistake that forget to remove H264_PROF_AUTO. I will fix that.
Any other suggestions about the profile_options? Thanks.


------------------ 原始邮件 ------------------
发件人:                                                                                                                        ""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?

>> 
>> -        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".
> 
>> 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