From: "\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: 徐福隆 <839789740@qq.com>, "Rick Kern" <kernrj@gmail.com>
Subject: Re: [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: replace VT_H264Profile with avctx profile
Date: Mon, 22 May 2023 11:11:08 +0800
Message-ID: <tencent_64E4D50A2753389F16444D0D206ACDC14F07@qq.com> (raw)
In-Reply-To: <tencent_30FD136B75D06906CEAC454F130E2841F305@qq.com>
> On May 22, 2023, at 11:05, zhilizhao(赵志立) <quinkblack@foxmail.com> wrote:
>
>> On May 21, 2023, at 22:41, xufuji456 <839789740@qq.com> wrote:
>>
>> For compatibility with constrained_baseline in the future,
>> replace VT_H264Profile/VT_HEVCProfile with avctx->profile.
>>
>> Signed-off-by: xufuji456 <839789740@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-request@ffmpeg.org with subject "unsubscribe".
>
>> On May 21, 2023, at 22:41, xufuji456 <839789740@qq.com> wrote:
>>
>> For compatibility with constrained_baseline in the future,
>> replace VT_H264Profile/VT_HEVCProfile with avctx->profile.
>>
>> Signed-off-by: xufuji456 <839789740@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-request@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-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2023-05-22 3:11 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-21 14:41 xufuji456
2023-05-22 3:05 ` "zhilizhao(赵志立)"
2023-05-22 3:11 ` "zhilizhao(赵志立)" [this message]
2023-05-22 4:17 ` [FFmpeg-devel] =?gb18030?b?u9i4tKO6ICBbUEFUQ0hdIGF2Y29kZWMvdmlk?= =?gb18030?q?eotoolboxenc=3A_replace_VT=5FH264Profile_with_avctx_profile?= =?gb18030?B?0Oy4o8Kh?=
2023-05-24 13:37 ` [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: replace VT_H264Profile with avctx profile Rick Kern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=tencent_64E4D50A2753389F16444D0D206ACDC14F07@qq.com \
--to=quinkblack@foxmail.com \
--cc=839789740@qq.com \
--cc=ffmpeg-devel@ffmpeg.org \
--cc=kernrj@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git