[FFmpeg-devel] [PATCH] Add support for the new key & value API in libaom.
This key & value API can greatly help with users who wants to try libaom-av1 specific options that are not supported by ffmpeg options. As was previously discussed in this thread: https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658. The commit that added the API to libaom: https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257 The libaom issue tracker: https://bugs.chromium.org/p/aomedia/issues/detail?id=2875 Signed-off-by: Bohan Li --- doc/encoders.texi | 11 +++ libavcodec/libaomenc.c | 30 ++ 2 files changed, 41 insertions(+) diff --git a/doc/encoders.texi b/doc/encoders.texi index c2ba7d3e6f..9fab512892 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true. @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0) Enable smooth interintra mode. Default is true. +@item libaom-params +Set libaom options using a list of @var{key}=@var{value} pairs separated +by ":". For a list of supported options, see @command{aomenc --help} under the +section "AV1 Specific Options". + +For example to specify libaom encoding options with @option{-libaom-params}: + +@example +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params tune=psnr:enable-tpl-model=1 output.mp4 +@end example + @end table @section libsvtav1 diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 342d0883e4..c7a87e01cd 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { int enable_diff_wtd_comp; int enable_dist_wtd_comp; int enable_dual_filter; +AVDictionary *extra_params; } AOMContext; static const char *const ctlidstr[] = { @@ -318,6 +319,25 @@ static av_cold int codecctl_int(AVCodecContext *avctx, return 0; } +static av_cold int codec_set_option(AVCodecContext *avctx, +const char* key, +const char* value) +{ +AOMContext *ctx = avctx->priv_data; +int width = -30; +int res; + +av_log(avctx, AV_LOG_DEBUG, " %*s: %s\n", width, key, value); + +res = aom_codec_set_option(&ctx->encoder, key, value); +if (res != AOM_CODEC_OK) { +log_encoder_error(avctx, key); +return AVERROR(EINVAL); +} + +return 0; +} + static av_cold int aom_free(AVCodecContext *avctx) { AOMContext *ctx = avctx->priv_data; @@ -874,6 +894,15 @@ static av_cold int aom_init(AVCodecContext *avctx, codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc); #endif +#if AOM_ENCODER_ABI_VERSION >= 23 +{ + AVDictionaryEntry *en = NULL; + while ((en = av_dict_get(ctx->extra_params, "", en, AV_DICT_IGNORE_SUFFIX))) { +codec_set_option(avctx, en->key, en->value); + } +} +#endif + // provide dummy value to initialize wrapper, values will be updated each _encode() aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1, (unsigned char*)1); @@ -1299,6 +1328,7 @@ static const AVOption options[] = { { "enable-masked-comp", "Enable masked compound", OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-interintra-comp", "Enable interintra compound", OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-smooth-interintra", "Enable smooth interintra mode", OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, +{ "libaom-params","Extra parameters for libaom", OFFSET(extra_params), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE }, { NULL }, }; -- 2.30.0.365.g02bc693789-goog ___ 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".
Re: [FFmpeg-devel] [PATCH] Add support for the new key & value API in libaom.
Thanks for the review! Indeed the buf string does not play any role here. I have removed it in the new patch. Bohan On Thu, Feb 4, 2021 at 1:02 PM Bohan Li wrote: > This key & value API can greatly help with users who wants to try > libaom-av1 specific options that are not supported by ffmpeg options. > > As was previously discussed in this thread: > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658. > > The commit that added the API to libaom: > https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257 > > The libaom issue tracker: > https://bugs.chromium.org/p/aomedia/issues/detail?id=2875 > > Signed-off-by: Bohan Li > --- > doc/encoders.texi | 11 +++ > libavcodec/libaomenc.c | 30 ++ > 2 files changed, 41 insertions(+) > > diff --git a/doc/encoders.texi b/doc/encoders.texi > index c2ba7d3e6f..9fab512892 100644 > --- a/doc/encoders.texi > +++ b/doc/encoders.texi > @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true. > @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= > v2.0.0) > Enable smooth interintra mode. Default is true. > > +@item libaom-params > +Set libaom options using a list of @var{key}=@var{value} pairs separated > +by ":". For a list of supported options, see @command{aomenc --help} > under the > +section "AV1 Specific Options". > + > +For example to specify libaom encoding options with > @option{-libaom-params}: > + > +@example > +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params > tune=psnr:enable-tpl-model=1 output.mp4 > +@end example > + > @end table > > @section libsvtav1 > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > index 342d0883e4..c7a87e01cd 100644 > --- a/libavcodec/libaomenc.c > +++ b/libavcodec/libaomenc.c > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { > int enable_diff_wtd_comp; > int enable_dist_wtd_comp; > int enable_dual_filter; > +AVDictionary *extra_params; > } AOMContext; > > static const char *const ctlidstr[] = { > @@ -318,6 +319,25 @@ static av_cold int codecctl_int(AVCodecContext *avctx, > return 0; > } > > +static av_cold int codec_set_option(AVCodecContext *avctx, > +const char* key, > +const char* value) > +{ > +AOMContext *ctx = avctx->priv_data; > +int width = -30; > +int res; > + > +av_log(avctx, AV_LOG_DEBUG, " %*s: %s\n", width, key, value); > + > +res = aom_codec_set_option(&ctx->encoder, key, value); > +if (res != AOM_CODEC_OK) { > +log_encoder_error(avctx, key); > +return AVERROR(EINVAL); > +} > + > +return 0; > +} > + > static av_cold int aom_free(AVCodecContext *avctx) > { > AOMContext *ctx = avctx->priv_data; > @@ -874,6 +894,15 @@ static av_cold int aom_init(AVCodecContext *avctx, > codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc); > #endif > > +#if AOM_ENCODER_ABI_VERSION >= 23 > +{ > + AVDictionaryEntry *en = NULL; > + while ((en = av_dict_get(ctx->extra_params, "", en, > AV_DICT_IGNORE_SUFFIX))) { > +codec_set_option(avctx, en->key, en->value); > + } > +} > +#endif > + > // provide dummy value to initialize wrapper, values will be updated > each _encode() > aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1, > (unsigned char*)1); > @@ -1299,6 +1328,7 @@ static const AVOption options[] = { > { "enable-masked-comp", "Enable masked compound", > OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > { "enable-interintra-comp", "Enable interintra compound", > OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > { "enable-smooth-interintra", "Enable smooth interintra mode", > OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > +{ "libaom-params","Extra parameters for libaom", > OFFSET(extra_params), AV_OPT_TYPE_DICT, { > 0 },0, 0, VE }, > { NULL }, > }; > > -- > 2.30.0.365.g02bc693789-goog > > ___ 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".
Re: [FFmpeg-devel] [PATCH] Add support for the new key & value API in libaom.
Gentle ping :) On Thu, Feb 4, 2021 at 1:45 PM Bohan Li wrote: > Thanks for the review! Indeed the buf string does not play any role here. > I have removed it in the new patch. > > Bohan > > On Thu, Feb 4, 2021 at 1:02 PM Bohan Li wrote: > >> This key & value API can greatly help with users who wants to try >> libaom-av1 specific options that are not supported by ffmpeg options. >> >> As was previously discussed in this thread: >> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658. >> >> The commit that added the API to libaom: >> https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257 >> >> The libaom issue tracker: >> https://bugs.chromium.org/p/aomedia/issues/detail?id=2875 >> >> Signed-off-by: Bohan Li >> --- >> doc/encoders.texi | 11 +++ >> libavcodec/libaomenc.c | 30 ++ >> 2 files changed, 41 insertions(+) >> >> diff --git a/doc/encoders.texi b/doc/encoders.texi >> index c2ba7d3e6f..9fab512892 100644 >> --- a/doc/encoders.texi >> +++ b/doc/encoders.texi >> @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true. >> @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= >> v2.0.0) >> Enable smooth interintra mode. Default is true. >> >> +@item libaom-params >> +Set libaom options using a list of @var{key}=@var{value} pairs separated >> +by ":". For a list of supported options, see @command{aomenc --help} >> under the >> +section "AV1 Specific Options". >> + >> +For example to specify libaom encoding options with >> @option{-libaom-params}: >> + >> +@example >> +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params >> tune=psnr:enable-tpl-model=1 output.mp4 >> +@end example >> + >> @end table >> >> @section libsvtav1 >> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c >> index 342d0883e4..c7a87e01cd 100644 >> --- a/libavcodec/libaomenc.c >> +++ b/libavcodec/libaomenc.c >> @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { >> int enable_diff_wtd_comp; >> int enable_dist_wtd_comp; >> int enable_dual_filter; >> +AVDictionary *extra_params; >> } AOMContext; >> >> static const char *const ctlidstr[] = { >> @@ -318,6 +319,25 @@ static av_cold int codecctl_int(AVCodecContext >> *avctx, >> return 0; >> } >> >> +static av_cold int codec_set_option(AVCodecContext *avctx, >> +const char* key, >> +const char* value) >> +{ >> +AOMContext *ctx = avctx->priv_data; >> +int width = -30; >> +int res; >> + >> +av_log(avctx, AV_LOG_DEBUG, " %*s: %s\n", width, key, value); >> + >> +res = aom_codec_set_option(&ctx->encoder, key, value); >> +if (res != AOM_CODEC_OK) { >> +log_encoder_error(avctx, key); >> +return AVERROR(EINVAL); >> +} >> + >> +return 0; >> +} >> + >> static av_cold int aom_free(AVCodecContext *avctx) >> { >> AOMContext *ctx = avctx->priv_data; >> @@ -874,6 +894,15 @@ static av_cold int aom_init(AVCodecContext *avctx, >> codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, >> ctx->enable_intrabc); >> #endif >> >> +#if AOM_ENCODER_ABI_VERSION >= 23 >> +{ >> + AVDictionaryEntry *en = NULL; >> + while ((en = av_dict_get(ctx->extra_params, "", en, >> AV_DICT_IGNORE_SUFFIX))) { >> +codec_set_option(avctx, en->key, en->value); >> + } >> +} >> +#endif >> + >> // provide dummy value to initialize wrapper, values will be updated >> each _encode() >> aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1, >> (unsigned char*)1); >> @@ -1299,6 +1328,7 @@ static const AVOption options[] = { >> { "enable-masked-comp", "Enable masked compound", >> OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, >> {.i64 = -1}, -1, 1, VE}, >> { "enable-interintra-comp", "Enable interintra compound", >> OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, >> {.i64 = -1}, -1, 1, VE}, >> { "enable-smooth-interintra", "Enable smooth interintra mode", >>OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, >> {.i64 = -1}, -1, 1, VE}, >> +{ "libaom-params","Extra parameters for libaom", >>OFFSET(extra_params), AV_OPT_TYPE_DICT, >> { 0 },0, 0, VE }, >> { NULL }, >> }; >> >> -- >> 2.30.0.365.g02bc693789-goog >> >> ___ 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".
Re: [FFmpeg-devel] [PATCH] Add support for the new key & value API in libaom.
Thank you very much for the reviews and the fixes, Jan! I went over the fixup commit and I do agree with the patches you proposed. It looks good to me. Just to make sure I don't misunderstand, should I submit a new patch with the modifications you mentioned, or leave the patch as is and you would modify and apply it? Thanks a lot! Bohan On Mon, Feb 8, 2021 at 3:47 PM Jan Ekström wrote: > On Thu, Feb 4, 2021 at 11:02 PM Bohan Li > wrote: > > > > This key & value API can greatly help with users who wants to try > > libaom-av1 specific options that are not supported by ffmpeg options. > > > > Excellent! Thank you for moving this forward :) . > > I noticed various things which I will also notice here, but made a possible > fixup commit on a branch: > https://github.com/jeeb/ffmpeg/commits/libaom_key_value_api > > If you think that is OK (in case I didn't mess anything up), I can > trim the commit message a bit and we should be done with this :) . > > > As was previously discussed in this thread: > > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658. > > > > The commit that added the API to libaom: > > https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257 > > > > The libaom issue tracker: > > https://bugs.chromium.org/p/aomedia/issues/detail?id=2875 > > > > Signed-off-by: Bohan Li > > --- > > doc/encoders.texi | 11 +++ > > libavcodec/libaomenc.c | 30 ++ > > 2 files changed, 41 insertions(+) > > > > diff --git a/doc/encoders.texi b/doc/encoders.texi > > index c2ba7d3e6f..9fab512892 100644 > > --- a/doc/encoders.texi > > +++ b/doc/encoders.texi > > @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true. > > @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= > v2.0.0) > > Enable smooth interintra mode. Default is true. > > > > +@item libaom-params > > +Set libaom options using a list of @var{key}=@var{value} pairs separated > > +by ":". For a list of supported options, see @command{aomenc --help} > under the > > +section "AV1 Specific Options". > > + > > +For example to specify libaom encoding options with > @option{-libaom-params}: > > + > > +@example > > +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params > tune=psnr:enable-tpl-model=1 output.mp4 > > +@end example > > + > > @end table > > > > @section libsvtav1 > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > > index 342d0883e4..c7a87e01cd 100644 > > --- a/libavcodec/libaomenc.c > > +++ b/libavcodec/libaomenc.c > > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { > > int enable_diff_wtd_comp; > > int enable_dist_wtd_comp; > > int enable_dual_filter; > > +AVDictionary *extra_params; > > } AOMContext; > > > > static const char *const ctlidstr[] = { > > @@ -318,6 +319,25 @@ static av_cold int codecctl_int(AVCodecContext > *avctx, > > return 0; > > } > > > > +static av_cold int codec_set_option(AVCodecContext *avctx, > > +const char* key, > > +const char* value) > > +{ > > +AOMContext *ctx = avctx->priv_data; > > +int width = -30; > > +int res; > > + > > +av_log(avctx, AV_LOG_DEBUG, " %*s: %s\n", width, key, value); > > + > > My guess this was a left-over from some debugging. I think the width > and debug log line can be removed if log_encoder_error gives one a > useful error in the log? > > > +res = aom_codec_set_option(&ctx->encoder, key, value); > > +if (res != AOM_CODEC_OK) { > > +log_encoder_error(avctx, key); > > +return AVERROR(EINVAL); > > AVERROR_EXTERNAL seems to be utilized when something fails inside an > external library. To be fair, in this case maybe if we had separate > paths for AOM_CODEC_INVALID_PARAM and AOM_CODEC_ERROR we could utilize > EINVAL for one of them? But if we just handle both of them then > AVERROR_EXTERNAL might match both? > > > +} > > + > > +return 0; > > +} > > + > > static av_cold int aom_free(AVCodecContext *avctx) > > { > > AOMContext *ctx = avctx->priv_data; > > @@ -874,6 +894,15 @@ static av_cold int aom_init(AVCodecContext *avctx, > > codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, > ctx->enable_intrabc); > > #endif > > > > +#if AOM_ENCODER_ABI_VERSION >= 23 > > +{ >
[FFmpeg-devel] [PATCH v2] Add support for the new key & value API in libaom.
This key & value API can greatly help with users who wants to try libaom-av1 specific options that are not supported by ffmpeg options. As was previously discussed in this thread: https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658. The commit that added the API to libaom: https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257 The libaom issue tracker: https://bugs.chromium.org/p/aomedia/issues/detail?id=2875 Signed-off-by: Bohan Li --- doc/encoders.texi | 11 +++ libavcodec/libaomenc.c | 18 ++ 2 files changed, 29 insertions(+) diff --git a/doc/encoders.texi b/doc/encoders.texi index c2ba7d3e6f..8fb573c416 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true. @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0) Enable smooth interintra mode. Default is true. +@item aom-params +Set libaom options using a list of @var{key}=@var{value} pairs separated +by ":". For a list of supported options, see @command{aomenc --help} under the +section "AV1 Specific Options". + +For example to specify libaom encoding options with @option{-aom-params}: + +@example +ffmpeg -i input -c:v libaom-av1 -b:v 500K -aom-params tune=psnr:enable-tpl-model=1 output.mp4 +@end example + @end table @section libsvtav1 diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 342d0883e4..9a26b5f9ef 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { int enable_diff_wtd_comp; int enable_dist_wtd_comp; int enable_dual_filter; +AVDictionary *aom_params; } AOMContext; static const char *const ctlidstr[] = { @@ -874,6 +875,20 @@ static av_cold int aom_init(AVCodecContext *avctx, codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc); #endif +#if AOM_ENCODER_ABI_VERSION >= 23 +{ +AVDictionaryEntry *en = NULL; + +while ((en = av_dict_get(ctx->aom_params, "", en, AV_DICT_IGNORE_SUFFIX))) { +int ret = aom_codec_set_option(&ctx->encoder, en->key, en->value); +if (ret != AOM_CODEC_OK) { +log_encoder_error(avctx, en->key); +return AVERROR_EXTERNAL; +} +} +} +#endif + // provide dummy value to initialize wrapper, values will be updated each _encode() aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1, (unsigned char*)1); @@ -1299,6 +1314,9 @@ static const AVOption options[] = { { "enable-masked-comp", "Enable masked compound", OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-interintra-comp", "Enable interintra compound", OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-smooth-interintra", "Enable smooth interintra mode", OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, +#if AOM_ENCODER_ABI_VERSION >= 23 +{ "aom-params", "Set libaom options using a :-separated list of key=value pairs", OFFSET(aom_params), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE }, +#endif { NULL }, }; -- 2.30.0.478.g8a0d178c01-goog ___ 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".
Re: [FFmpeg-devel] [PATCH v2] Add support for the new key & value API in libaom.
Thanks for the info! Just uploaded the patch v2 as suggested. Best, Bohan On Mon, Feb 8, 2021 at 8:04 PM Bohan Li wrote: > This key & value API can greatly help with users who wants to try > libaom-av1 specific options that are not supported by ffmpeg options. > > As was previously discussed in this thread: > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658. > > The commit that added the API to libaom: > https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257 > > The libaom issue tracker: > https://bugs.chromium.org/p/aomedia/issues/detail?id=2875 > > Signed-off-by: Bohan Li > --- > doc/encoders.texi | 11 +++ > libavcodec/libaomenc.c | 18 ++ > 2 files changed, 29 insertions(+) > > diff --git a/doc/encoders.texi b/doc/encoders.texi > index c2ba7d3e6f..8fb573c416 100644 > --- a/doc/encoders.texi > +++ b/doc/encoders.texi > @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true. > @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= > v2.0.0) > Enable smooth interintra mode. Default is true. > > +@item aom-params > +Set libaom options using a list of @var{key}=@var{value} pairs separated > +by ":". For a list of supported options, see @command{aomenc --help} > under the > +section "AV1 Specific Options". > + > +For example to specify libaom encoding options with @option{-aom-params}: > + > +@example > +ffmpeg -i input -c:v libaom-av1 -b:v 500K -aom-params > tune=psnr:enable-tpl-model=1 output.mp4 > +@end example > + > @end table > > @section libsvtav1 > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > index 342d0883e4..9a26b5f9ef 100644 > --- a/libavcodec/libaomenc.c > +++ b/libavcodec/libaomenc.c > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { > int enable_diff_wtd_comp; > int enable_dist_wtd_comp; > int enable_dual_filter; > +AVDictionary *aom_params; > } AOMContext; > > static const char *const ctlidstr[] = { > @@ -874,6 +875,20 @@ static av_cold int aom_init(AVCodecContext *avctx, > codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc); > #endif > > +#if AOM_ENCODER_ABI_VERSION >= 23 > +{ > +AVDictionaryEntry *en = NULL; > + > +while ((en = av_dict_get(ctx->aom_params, "", en, > AV_DICT_IGNORE_SUFFIX))) { > +int ret = aom_codec_set_option(&ctx->encoder, en->key, > en->value); > +if (ret != AOM_CODEC_OK) { > +log_encoder_error(avctx, en->key); > +return AVERROR_EXTERNAL; > +} > +} > +} > +#endif > + > // provide dummy value to initialize wrapper, values will be updated > each _encode() > aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1, > (unsigned char*)1); > @@ -1299,6 +1314,9 @@ static const AVOption options[] = { > { "enable-masked-comp", "Enable masked compound", > OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > { "enable-interintra-comp", "Enable interintra compound", > OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > { "enable-smooth-interintra", "Enable smooth interintra mode", > OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > +#if AOM_ENCODER_ABI_VERSION >= 23 > +{ "aom-params", "Set libaom options using a > :-separated list of key=value pairs", OFFSET(aom_params), AV_OPT_TYPE_DICT, > { 0 }, 0, 0, VE }, > +#endif > { NULL }, > }; > > -- > 2.30.0.478.g8a0d178c01-goog > > ___ 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".
Re: [FFmpeg-devel] [PATCH v2] Add support for the new key & value API in libaom.
Hi Jan, Yes the modified patch looks good to me. Please let me know if there is anything needed from my end. Thank you very much! Bohan On Tue, Feb 9, 2021, 12:01 PM Jan Ekström wrote: > On Tue, Feb 9, 2021 at 6:05 AM Bohan Li > wrote: > > > > This key & value API can greatly help with users who wants to try > > libaom-av1 specific options that are not supported by ffmpeg options. > > > > Looks good to me code-wise at this point. Thanks for noticing the > small issues I had created due to not testing the modifications I did > late in the night :) . > > I have tested this patch with both c1d42fe6615c96fc929257~1 (last > revision without this feature) and current libaom master, and both > compiled correctly. The error message from the library looks good > enough (in my case omochi=unyoon led to an "unknown option" error), > and the option is not available in the wrapper if the libaom version > built against didn't have it. > > So two things I did: > - Adjust the author to match the name & e-mail in the Signed-off-by > (the mailing list changes the From field as it attempts to rewrite > fields when sending the messages out, so if you just apply the patch > from f.ex. Patchwork you get the wrong one). > - Rewrote the commit message. > > https://github.com/jeeb/ffmpeg/commits/libaom_key_value_api_v2 > > Please tell if there are any issues with this from your side. If not, > I think we can start pulling this in :) . > > Jan > ___ > 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".
[FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Signed-off-by: Bohan Li --- libavcodec/libaomenc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 2b0581b15a..a5d9843ae2 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { int enable_diff_wtd_comp; int enable_dist_wtd_comp; int enable_dual_filter; +int enable_keyframe_filtering; } AOMContext; static const char *const ctlidstr[] = { @@ -192,6 +193,7 @@ static const char *const ctlidstr[] = { [AV1E_SET_REDUCED_REFERENCE_SET]= "AV1E_SET_REDUCED_REFERENCE_SET", [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS", +[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = "AV1E_SET_ENABLE_KEYFRAME_FILTRING" #endif }; @@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx, codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, ctx->enable_onesided_comp); if (ctx->enable_smooth_interintra >= 0) codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, ctx->enable_smooth_interintra); +if (ctx->enable_keyframe_filtering >= 0) +codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, ctx->enable_keyframe_filtering); #endif codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh); @@ -1261,6 +1265,7 @@ static const AVOption options[] = { { "enable-masked-comp", "Enable masked compound", OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-interintra-comp", "Enable interintra compound", OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-smooth-interintra", "Enable smooth interintra mode", OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, +{ "enable-keyframe-filtering","Enable keyframe filtering type", OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT, {.i64 = -1}, -1, 3, VE}, { NULL }, }; -- 2.29.0.rc1.297.gfa9743e501-goog ___ 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] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Add the option to use -enable-keyframe-filtering with libaom-av1 codec. The option controls the encoder behavior on performing temporal filtering on keyframes. Signed-off-by: Bohan Li --- libavcodec/libaomenc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 2b0581b15a..77c25770a4 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { int enable_diff_wtd_comp; int enable_dist_wtd_comp; int enable_dual_filter; +int enable_keyframe_filtering; } AOMContext; static const char *const ctlidstr[] = { @@ -192,6 +193,7 @@ static const char *const ctlidstr[] = { [AV1E_SET_REDUCED_REFERENCE_SET]= "AV1E_SET_REDUCED_REFERENCE_SET", [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS", +[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = "AV1E_SET_ENABLE_KEYFRAME_FILTERING" #endif }; @@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx, codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, ctx->enable_onesided_comp); if (ctx->enable_smooth_interintra >= 0) codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, ctx->enable_smooth_interintra); +if (ctx->enable_keyframe_filtering >= 0) +codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, ctx->enable_keyframe_filtering); #endif codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh); @@ -1261,6 +1265,7 @@ static const AVOption options[] = { { "enable-masked-comp", "Enable masked compound", OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-interintra-comp", "Enable interintra compound", OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-smooth-interintra", "Enable smooth interintra mode", OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, +{ "enable-keyframe-filtering","Keyframe filtering type", OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT, {.i64 = -1}, -1, 3, VE}, { NULL }, }; -- 2.29.0.rc1.297.gfa9743e501-goog ___ 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Thank you for the prompt response! I'll fix the typo, add documentation and re-submit. Regarding the last comment, the enable-keyframe-filtering parameter was a boolean, but recently there is one more option added to libaom (--enable-keyframe-filtering=2), so I thought it would be better to use AV_OPT_TYPE_INT here so people who build libaom from source could use that option. Please let me know if there are any concerns with it. Thanks and best regards, Bohan On Mon, Oct 26, 2020 at 2:07 PM James Almer wrote: > On 10/26/2020 6:01 PM, Bohan Li wrote: > > Signed-off-by: Bohan Li > > --- > > libavcodec/libaomenc.c | 5 + > > 1 file changed, 5 insertions(+) > > Missing documentation. > > > > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > > index 2b0581b15a..a5d9843ae2 100644 > > --- a/libavcodec/libaomenc.c > > +++ b/libavcodec/libaomenc.c > > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { > > int enable_diff_wtd_comp; > > int enable_dist_wtd_comp; > > int enable_dual_filter; > > +int enable_keyframe_filtering; > > } AOMContext; > > > > static const char *const ctlidstr[] = { > > @@ -192,6 +193,7 @@ static const char *const ctlidstr[] = { > > [AV1E_SET_REDUCED_REFERENCE_SET]= > "AV1E_SET_REDUCED_REFERENCE_SET", > > [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = > "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", > > [AV1E_SET_ENABLE_REF_FRAME_MVS] = > "AV1E_SET_ENABLE_REF_FRAME_MVS", > > +[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = > "AV1E_SET_ENABLE_KEYFRAME_FILTRING" > > FILTRING typo. > > > #endif > > }; > > > > @@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx, > > codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, > ctx->enable_onesided_comp); > > if (ctx->enable_smooth_interintra >= 0) > > codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, > ctx->enable_smooth_interintra); > > +if (ctx->enable_keyframe_filtering >= 0) > > +codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, > ctx->enable_keyframe_filtering); > > #endif > > > > codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh); > > @@ -1261,6 +1265,7 @@ static const AVOption options[] = { > > { "enable-masked-comp", "Enable masked compound", > OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > > { "enable-interintra-comp", "Enable interintra compound", > OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > > { "enable-smooth-interintra", "Enable smooth interintra mode", >OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > > +{ "enable-keyframe-filtering","Enable keyframe filtering > type",OFFSET(enable_keyframe_filtering), > AV_OPT_TYPE_INT, {.i64 = -1}, -1, 3, VE}, > > It's a boolean, so use AV_OPT_TYPE_BOOL. > > > { NULL }, > > }; > > > > > > ___ > 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".
[FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Add the option to use -enable-keyframe-filtering with libaom-av1 codec. The option controls the encoder behavior on performing temporal filtering on keyframes. Signed-off-by: Bohan Li --- doc/encoders.texi | 13 + libavcodec/libaomenc.c | 5 + 2 files changed, 18 insertions(+) diff --git a/doc/encoders.texi b/doc/encoders.texi index 0b1c69e982..8914546694 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1685,6 +1685,19 @@ Enable interintra compound. Default is true. @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0) Enable smooth interintra mode. Default is true. +@item enable-keyframe-filtering (Requires libaom >= v2.0.0) +Filtering type for key frames. Possible values: +@table @samp +@item @emph{-1} +Use the default in libaom (default). +@item @emph{0} +Do not filter key frames. +@item @emph{1} +Filter key frames but do not apply overlays. +@item @emph{2} +Filter key frames and apply overlays to them (experimental). +@end table + @end table @section libsvtav1 diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 2b0581b15a..77c25770a4 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { int enable_diff_wtd_comp; int enable_dist_wtd_comp; int enable_dual_filter; +int enable_keyframe_filtering; } AOMContext; static const char *const ctlidstr[] = { @@ -192,6 +193,7 @@ static const char *const ctlidstr[] = { [AV1E_SET_REDUCED_REFERENCE_SET]= "AV1E_SET_REDUCED_REFERENCE_SET", [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS", +[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = "AV1E_SET_ENABLE_KEYFRAME_FILTERING" #endif }; @@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx, codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, ctx->enable_onesided_comp); if (ctx->enable_smooth_interintra >= 0) codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, ctx->enable_smooth_interintra); +if (ctx->enable_keyframe_filtering >= 0) +codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, ctx->enable_keyframe_filtering); #endif codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh); @@ -1261,6 +1265,7 @@ static const AVOption options[] = { { "enable-masked-comp", "Enable masked compound", OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-interintra-comp", "Enable interintra compound", OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-smooth-interintra", "Enable smooth interintra mode", OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, +{ "enable-keyframe-filtering","Keyframe filtering type", OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT, {.i64 = -1}, -1, 3, VE}, { NULL }, }; -- 2.29.0.rc1.297.gfa9743e501-goog ___ 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Thanks a lot for the explanation! I'll add the documentation accordingly. Best, Bohan On Mon, Oct 26, 2020 at 2:46 PM James Almer wrote: > On 10/26/2020 6:21 PM, Bohan Li wrote: > > > Regarding the last comment, the enable-keyframe-filtering parameter was a > > boolean, but recently there is one more option added to libaom > > (--enable-keyframe-filtering=2), so I thought it would be better to use > > AV_OPT_TYPE_INT here so people who build libaom from source could use > that > > option. > > > > Please let me know if there are any concerns with it. > > No, if it was extended then it's correct as INT. > > > Add the option to use -enable-keyframe-filtering with libaom-av1 > > codec. The option controls the encoder behavior on performing > > temporal filtering on keyframes. > > By missing documentation i meant an entry in doc/encoder.texi > Look for the libaom-av1 section and add the new option at the end. > > This blurb in the commit message is ok and can stay, too. > > > > > Signed-off-by: Bohan Li > > --- > > libavcodec/libaomenc.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > > index 2b0581b15a..77c25770a4 100644 > > --- a/libavcodec/libaomenc.c > > +++ b/libavcodec/libaomenc.c > > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { > > int enable_diff_wtd_comp; > > int enable_dist_wtd_comp; > > int enable_dual_filter; > > +int enable_keyframe_filtering; > > } AOMContext; > > > > static const char *const ctlidstr[] = { > > @@ -192,6 +193,7 @@ static const char *const ctlidstr[] = { > > [AV1E_SET_REDUCED_REFERENCE_SET]= > "AV1E_SET_REDUCED_REFERENCE_SET", > > [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = > "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", > > [AV1E_SET_ENABLE_REF_FRAME_MVS] = > "AV1E_SET_ENABLE_REF_FRAME_MVS", > > +[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = > "AV1E_SET_ENABLE_KEYFRAME_FILTERING" > > #endif > > }; > > > > @@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx, > > codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, > ctx->enable_onesided_comp); > > if (ctx->enable_smooth_interintra >= 0) > > codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, > ctx->enable_smooth_interintra); > > +if (ctx->enable_keyframe_filtering >= 0) > > +codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, > ctx->enable_keyframe_filtering); > > #endif > > > > codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh); > > @@ -1261,6 +1265,7 @@ static const AVOption options[] = { > > { "enable-masked-comp", "Enable masked compound", > OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > > { "enable-interintra-comp", "Enable interintra compound", > OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > > { "enable-smooth-interintra", "Enable smooth interintra mode", >OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > > +{ "enable-keyframe-filtering","Keyframe filtering type", >OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT, > {.i64 = -1}, -1, 3, VE}, > > { NULL }, > > }; > > > > > > ___ > 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Thanks for the comment, Mark. Indeed this may not be helpful for people who did not know the background of such parameters. I added more details to it so people could understand the trade-off behind it better. Will re-submit the patch soon. Best, Bohan On Tue, Oct 27, 2020 at 1:26 PM Mark Thompson wrote: > On 26/10/2020 22:04, Bohan Li wrote: > > Add the option to use -enable-keyframe-filtering with libaom-av1 > > codec. The option controls the encoder behavior on performing > > temporal filtering on keyframes. > > > > Signed-off-by: Bohan Li > > --- > > doc/encoders.texi | 13 + > > libavcodec/libaomenc.c | 5 + > > 2 files changed, 18 insertions(+) > > > > diff --git a/doc/encoders.texi b/doc/encoders.texi > > index 0b1c69e982..8914546694 100644 > > --- a/doc/encoders.texi > > +++ b/doc/encoders.texi > > @@ -1685,6 +1685,19 @@ Enable interintra compound. Default is true. > > @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= > v2.0.0) > > Enable smooth interintra mode. Default is true. > > > > +@item enable-keyframe-filtering (Requires libaom >= v2.0.0) > > +Filtering type for key frames. Possible values: > > +@table @samp > > +@item @emph{-1} > > +Use the default in libaom (default). > > +@item @emph{0} > > +Do not filter key frames. > > +@item @emph{1} > > +Filter key frames but do not apply overlays. > > +@item @emph{2} > > +Filter key frames and apply overlays to them (experimental). > > +@end table > > This documentation does not seem helpful. Suppose I am a normal end-user; > what effect does this option have on the output and under what > circumstances would I set it? If never, why is the option in the user > manual? > > (I would like to ask the same question of some of the options above this > one as well.) > > > + > > @end table > > > > @section libsvtav1 > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > > index 2b0581b15a..77c25770a4 100644 > > --- a/libavcodec/libaomenc.c > > +++ b/libavcodec/libaomenc.c > > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { > > int enable_diff_wtd_comp; > > int enable_dist_wtd_comp; > > int enable_dual_filter; > > +int enable_keyframe_filtering; > > } AOMContext; > > > > static const char *const ctlidstr[] = { > > @@ -192,6 +193,7 @@ static const char *const ctlidstr[] = { > > [AV1E_SET_REDUCED_REFERENCE_SET]= > "AV1E_SET_REDUCED_REFERENCE_SET", > > [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = > "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", > > [AV1E_SET_ENABLE_REF_FRAME_MVS] = > "AV1E_SET_ENABLE_REF_FRAME_MVS", > > +[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = > "AV1E_SET_ENABLE_KEYFRAME_FILTERING" > > #endif > > }; > > > > @@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx, > > codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, > ctx->enable_onesided_comp); > > if (ctx->enable_smooth_interintra >= 0) > > codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, > ctx->enable_smooth_interintra); > > +if (ctx->enable_keyframe_filtering >= 0) > > +codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, > ctx->enable_keyframe_filtering); > > #endif > > > > codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh); > > @@ -1261,6 +1265,7 @@ static const AVOption options[] = { > > { "enable-masked-comp", "Enable masked compound", > OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > > { "enable-interintra-comp", "Enable interintra compound", > OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, > {.i64 = -1}, -1, 1, VE}, > > { "enable-smooth-interintra", "Enable smooth interintra > mode", OFFSET(enable_smooth_interintra), > AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, > > +{ "enable-keyframe-filtering","Keyframe filtering type", >OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT, > {.i64 = -1}, -1, 3, VE}, > > { NULL }, > > }; > > > > > > - Mark > ___ > 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".
[FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Add the option to use -enable-keyframe-filtering with libaom-av1 codec. The option controls the encoder behavior on performing temporal filtering on keyframes. Signed-off-by: Bohan Li --- doc/encoders.texi | 17 + libavcodec/libaomenc.c | 5 + 2 files changed, 22 insertions(+) diff --git a/doc/encoders.texi b/doc/encoders.texi index 0b1c69e982..c80a520a20 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1685,6 +1685,23 @@ Enable interintra compound. Default is true. @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0) Enable smooth interintra mode. Default is true. +@item enable-keyframe-filtering (Requires libaom >= v2.0.0) +Filtering type for key frames. Temporal filtering of key frames improves +compression efficiency, but may introduce key frames that are blurred at times. +Adding an overlay to filtered frames mitigates the blurring issue, but could +potentially confuse video players when performing random access. +Possible values: +@table @samp +@item @emph{-1} +Use the default in libaom (default). +@item @emph{0} +Do not filter key frames. +@item @emph{1} +Filter key frames but do not apply overlays. +@item @emph{2} +Filter key frames and apply overlays to them (experimental). +@end table + @end table @section libsvtav1 diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 2b0581b15a..77c25770a4 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { int enable_diff_wtd_comp; int enable_dist_wtd_comp; int enable_dual_filter; +int enable_keyframe_filtering; } AOMContext; static const char *const ctlidstr[] = { @@ -192,6 +193,7 @@ static const char *const ctlidstr[] = { [AV1E_SET_REDUCED_REFERENCE_SET]= "AV1E_SET_REDUCED_REFERENCE_SET", [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS", +[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = "AV1E_SET_ENABLE_KEYFRAME_FILTERING" #endif }; @@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx, codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, ctx->enable_onesided_comp); if (ctx->enable_smooth_interintra >= 0) codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, ctx->enable_smooth_interintra); +if (ctx->enable_keyframe_filtering >= 0) +codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, ctx->enable_keyframe_filtering); #endif codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh); @@ -1261,6 +1265,7 @@ static const AVOption options[] = { { "enable-masked-comp", "Enable masked compound", OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-interintra-comp", "Enable interintra compound", OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-smooth-interintra", "Enable smooth interintra mode", OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, +{ "enable-keyframe-filtering","Keyframe filtering type", OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT, {.i64 = -1}, -1, 3, VE}, { NULL }, }; -- 2.29.0.rc2.309.g374f81d7ae-goog ___ 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Hi Jan, Thanks for the suggestion! I believe that is a good idea. I am not super familiar with this part, though, so will probably need to propose this to the Aomedia issue tracker. Meanwhile I think it is also useful to expose useful options (like this one, enable-keyframe-filtering, which is discussed quite a few times in the community as far as I know) to the users as well. So I think we should still apply this patch, and if once the mentioned API is working, we could maybe re-consider which ones are more needed. Best, Bohan On Tue, Oct 27, 2020 at 2:19 PM Jan Ekström wrote: > On Tue, Oct 27, 2020 at 10:38 PM Bohan Li > wrote: > > > > Add the option to use -enable-keyframe-filtering with libaom-av1 > > codec. The option controls the encoder behavior on performing > > temporal filtering on keyframes. > > > > Hi, > > Looking at the amount of options etc which you want to expose to > FFmpeg users (or just utilize yourselves via FFmpeg), and by the > feeling that we're duplicating a lot of option parsing that aomenc CLI > is already doing, I wonder if it would be more useful for libaom to > provide a key=value API? This then could be exposed to FFmpeg users > with an option similar to {x264,x265,rav1e}-params. > > This way each time libaom adds new options, they don't explicitly need > to be added to the wrapper. They can just be utilized. > > Best regards, > Jan > ___ > 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Gentle ping :) Bohan On Tue, Oct 27, 2020 at 4:13 PM Bohan Li wrote: > Hi Jan, > > Thanks for the suggestion! I believe that is a good idea. I am not super > familiar with this part, though, so will probably need to propose this to > the Aomedia issue tracker. > Meanwhile I think it is also useful to expose useful options (like this > one, enable-keyframe-filtering, which is discussed quite a few times in the > community as far as I know) to the users as well. So I think we should > still apply this patch, and if once the mentioned API is working, we could > maybe re-consider which ones are more needed. > > Best, > Bohan > > On Tue, Oct 27, 2020 at 2:19 PM Jan Ekström wrote: > >> On Tue, Oct 27, 2020 at 10:38 PM Bohan Li >> wrote: >> > >> > Add the option to use -enable-keyframe-filtering with libaom-av1 >> > codec. The option controls the encoder behavior on performing >> > temporal filtering on keyframes. >> > >> >> Hi, >> >> Looking at the amount of options etc which you want to expose to >> FFmpeg users (or just utilize yourselves via FFmpeg), and by the >> feeling that we're duplicating a lot of option parsing that aomenc CLI >> is already doing, I wonder if it would be more useful for libaom to >> provide a key=value API? This then could be exposed to FFmpeg users >> with an option similar to {x264,x265,rav1e}-params. >> >> This way each time libaom adds new options, they don't explicitly need >> to be added to the wrapper. They can just be utilized. >> >> Best regards, >> Jan >> ___ >> 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Another ping :) Bohan On Fri, Oct 30, 2020 at 2:43 PM Bohan Li wrote: > Gentle ping :) > > Bohan > > On Tue, Oct 27, 2020 at 4:13 PM Bohan Li wrote: > >> Hi Jan, >> >> Thanks for the suggestion! I believe that is a good idea. I am not super >> familiar with this part, though, so will probably need to propose this to >> the Aomedia issue tracker. >> Meanwhile I think it is also useful to expose useful options (like this >> one, enable-keyframe-filtering, which is discussed quite a few times in the >> community as far as I know) to the users as well. So I think we should >> still apply this patch, and if once the mentioned API is working, we could >> maybe re-consider which ones are more needed. >> >> Best, >> Bohan >> >> On Tue, Oct 27, 2020 at 2:19 PM Jan Ekström wrote: >> >>> On Tue, Oct 27, 2020 at 10:38 PM Bohan Li >>> wrote: >>> > >>> > Add the option to use -enable-keyframe-filtering with libaom-av1 >>> > codec. The option controls the encoder behavior on performing >>> > temporal filtering on keyframes. >>> > >>> >>> Hi, >>> >>> Looking at the amount of options etc which you want to expose to >>> FFmpeg users (or just utilize yourselves via FFmpeg), and by the >>> feeling that we're duplicating a lot of option parsing that aomenc CLI >>> is already doing, I wonder if it would be more useful for libaom to >>> provide a key=value API? This then could be exposed to FFmpeg users >>> with an option similar to {x264,x265,rav1e}-params. >>> >>> This way each time libaom adds new options, they don't explicitly need >>> to be added to the wrapper. They can just be utilized. >>> >>> Best regards, >>> Jan >>> ___ >>> 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Thanks for the reply, Steven! Regarding JEEB’s comment, the suggestion was to add an api to the *libaom* library, not to ffmpeg. I do agree with the rationale, but when such an api would be available for ffmpeg to use is quite uncertain. In the meanwhile, I believe it is reasonable to add in this patch to expose the option to the users. I don’t think this is a “middle version”, since as mentioned, this option could be quite useful in certain scenarios and IMHO should be added in. Please let me know if there are any concerns on this. Thanks! Best, Bohan On Thu, Nov 5, 2020 at 6:35 PM Steven Liu wrote: > > > > 2020年11月6日 上午2:29,Bohan Li 写道: > > > > Another ping :) > I think maybe you should fix JEEB’s review comment, then will get > responses comments. > Not only apply a middle version. > > > > Bohan > > > > On Fri, Oct 30, 2020 at 2:43 PM Bohan Li wrote: > > > >> Gentle ping :) > >> > >> Bohan > >> > >> On Tue, Oct 27, 2020 at 4:13 PM Bohan Li wrote: > >> > >>> Hi Jan, > >>> > >>> Thanks for the suggestion! I believe that is a good idea. I am not > super > >>> familiar with this part, though, so will probably need to propose this > to > >>> the Aomedia issue tracker. > >>> Meanwhile I think it is also useful to expose useful options (like this > >>> one, enable-keyframe-filtering, which is discussed quite a few times > in the > >>> community as far as I know) to the users as well. So I think we should > >>> still apply this patch, and if once the mentioned API is working, we > could > >>> maybe re-consider which ones are more needed. > >>> > >>> Best, > >>> Bohan > >>> > >>> On Tue, Oct 27, 2020 at 2:19 PM Jan Ekström wrote: > >>> > >>>> On Tue, Oct 27, 2020 at 10:38 PM Bohan Li > >>>> wrote: > >>>>> > >>>>> Add the option to use -enable-keyframe-filtering with libaom-av1 > >>>>> codec. The option controls the encoder behavior on performing > >>>>> temporal filtering on keyframes. > >>>>> > >>>> > >>>> Hi, > >>>> > >>>> Looking at the amount of options etc which you want to expose to > >>>> FFmpeg users (or just utilize yourselves via FFmpeg), and by the > >>>> feeling that we're duplicating a lot of option parsing that aomenc CLI > >>>> is already doing, I wonder if it would be more useful for libaom to > >>>> provide a key=value API? This then could be exposed to FFmpeg users > >>>> with an option similar to {x264,x265,rav1e}-params. > >>>> > >>>> This way each time libaom adds new options, they don't explicitly need > >>>> to be added to the wrapper. They can just be utilized. > >>>> > >>>> Best regards, > >>>> Jan > >>>> ___ > >>>> 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". > > Thanks > > Steven Liu > > ___ > 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Thanks a lot for the comment, Jan and Steven! Yes I very much agree that the number of options is quite large and this is not a great path to go for. But for the moment I still suggest that we apply this patch, while proposing to libaom for a key & value api. This option gives users the option to control the keyframe filtering method, which is quite essential to both objective and subjective quality. When turned on (default) the objective quality/compression efficiency is greatly improved, but at the cost of potential subjective quality loss at keyframes (because they are filtered and can appear blurry). Therefore users who do not want to have such artifacts may want to disable the option. This is actually a long-standing trade-off that libaom users have to (sometimes painfully) choose, and they cannot do so with ffmpeg right now. Also recently a new experimental option (=2) was added to libaom in order to mitigate the subjective loss. It would be nice to expose this option for people to be able to test it using ffmpeg with various decoders too. I've observed a few related discussions in the AV1 community (e.g. the reddit thread on the experimental option: https://www.reddit.com/r/AV1/comments/ic7ktu/aom_fixed_its_filtered_reference_frame_issues/ ). That said, I do understand the rationale and the reason why people might be opposed to adding another option, though I still suggest that we add the option for now, and when the API is available, we can make decisions and deprecate certain ones that are not necessary. Please let me know your thoughts on this, meanwhile I'll file a feature request to the aomedia bug tracker for the API. Thank you again for the comments! Best, Bohan On Fri, Nov 6, 2020 at 3:16 AM Jan Ekström wrote: > On Fri, Nov 6, 2020 at 11:46 AM Steven Liu wrote: > > > > > > > > > 2020年11月6日 下午4:42,Bohan Li 写道: > > > > > > Thanks for the reply, Steven! > > > > > > Regarding JEEB’s comment, the suggestion was to add an api to the > *libaom* library, not to ffmpeg. I do agree with the rationale, but when > such an api would be available for ffmpeg to use is quite uncertain. In the > meanwhile, I believe it is reasonable to add in this patch to expose the > option to the users. I don’t think this is a “middle version”, since as > mentioned, this option could be quite useful in certain scenarios and IMHO > should be added in. > > What about use av1_param arguments? As x264opts, x265opts? > > The problem with this is that libaom as far as I can tell doesn't have > an API like that. Thus each and every API user that needs to have > options defined as strings will have to *duplicate* those mappings > (which already are defined in the aomenc command line application, but > not exposed through the API). > > This has been the butt of jokes and reason for lamentations on IRC for > quite a while, but unfortunately nobody actually seems to have voiced > an opinion on this on the mailing list until now? > > Thus there generally speaking are only two ways forward for this: > 1. This is really spammy and unfortunate (and getting these removed > will be a PITA after we actually do get a key & value based API), but > we take the additional option in. > 2. This is apparently a low usage option and the amount of AVOptions > in this encoder is getting higher and higher, thus we deny the patch > until a key, value based API is provided. > > Personally I lean somewhat towards 2, but if the option is > needed/useful (use cases can be provided), then begrudgingly I could > go for 1. > > So far the lack of comments has mostly been regarding people not > having high enough stakes/care regarding this module, methinks? > > Jan > ___ > 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
I have added a feature request in the libaom issue tracker for the key & value api, although it may take some time for the api to land. Meanwhile I still suggest that we apply this patch to ffmpeg since as mentioned this option can be quite important for subjective quality control. Please let me know if there are any concerns. Thanks! Bohan On Fri, Nov 6, 2020 at 9:31 AM Bohan Li wrote: > Thanks a lot for the comment, Jan and Steven! Yes I very much agree that > the number of options is quite large and this is not a great path to go > for. But for the moment I still suggest that we apply this patch, while > proposing to libaom for a key & value api. > > This option gives users the option to control the keyframe filtering > method, which is quite essential to both objective and subjective quality. > When turned on (default) the objective quality/compression efficiency is > greatly improved, but at the cost of potential subjective quality loss at > keyframes (because they are filtered and can appear blurry). Therefore > users who do not want to have such artifacts may want to disable the > option. This is actually a long-standing trade-off that libaom users have > to (sometimes painfully) choose, and they cannot do so with ffmpeg right > now. Also recently a new experimental option (=2) was added to libaom in > order to mitigate the subjective loss. It would be nice to expose this > option for people to be able to test it using ffmpeg with various decoders > too. I've observed a few related discussions in the AV1 community (e.g. the > reddit thread on the experimental option: > https://www.reddit.com/r/AV1/comments/ic7ktu/aom_fixed_its_filtered_reference_frame_issues/ > ). > > That said, I do understand the rationale and the reason why people might > be opposed to adding another option, though I still suggest that we add the > option for now, and when the API is available, we can make decisions and > deprecate certain ones that are not necessary. Please let me know your > thoughts on this, meanwhile I'll file a feature request to the aomedia bug > tracker for the API. > > Thank you again for the comments! > Best, > Bohan > > On Fri, Nov 6, 2020 at 3:16 AM Jan Ekström wrote: > >> On Fri, Nov 6, 2020 at 11:46 AM Steven Liu wrote: >> > >> > >> > >> > > 2020年11月6日 下午4:42,Bohan Li 写道: >> > > >> > > Thanks for the reply, Steven! >> > > >> > > Regarding JEEB’s comment, the suggestion was to add an api to the >> *libaom* library, not to ffmpeg. I do agree with the rationale, but when >> such an api would be available for ffmpeg to use is quite uncertain. In the >> meanwhile, I believe it is reasonable to add in this patch to expose the >> option to the users. I don’t think this is a “middle version”, since as >> mentioned, this option could be quite useful in certain scenarios and IMHO >> should be added in. >> > What about use av1_param arguments? As x264opts, x265opts? >> >> The problem with this is that libaom as far as I can tell doesn't have >> an API like that. Thus each and every API user that needs to have >> options defined as strings will have to *duplicate* those mappings >> (which already are defined in the aomenc command line application, but >> not exposed through the API). >> >> This has been the butt of jokes and reason for lamentations on IRC for >> quite a while, but unfortunately nobody actually seems to have voiced >> an opinion on this on the mailing list until now? >> >> Thus there generally speaking are only two ways forward for this: >> 1. This is really spammy and unfortunate (and getting these removed >> will be a PITA after we actually do get a key & value based API), but >> we take the additional option in. >> 2. This is apparently a low usage option and the amount of AVOptions >> in this encoder is getting higher and higher, thus we deny the patch >> until a key, value based API is provided. >> >> Personally I lean somewhat towards 2, but if the option is >> needed/useful (use cases can be provided), then begrudgingly I could >> go for 1. >> >> So far the lack of comments has mostly been regarding people not >> having high enough stakes/care regarding this module, methinks? >> >> Jan >> ___ >> 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".
Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.
Gentle ping :) On Mon, Nov 9, 2020 at 11:00 AM Bohan Li wrote: > I have added a feature request in the libaom issue tracker for the key & > value api, although it may take some time for the api to land. Meanwhile I > still suggest that we apply this patch to ffmpeg since as mentioned this > option can be quite important for subjective quality control. > Please let me know if there are any concerns. > > Thanks! > Bohan > > On Fri, Nov 6, 2020 at 9:31 AM Bohan Li wrote: > >> Thanks a lot for the comment, Jan and Steven! Yes I very much agree that >> the number of options is quite large and this is not a great path to go >> for. But for the moment I still suggest that we apply this patch, while >> proposing to libaom for a key & value api. >> >> This option gives users the option to control the keyframe filtering >> method, which is quite essential to both objective and subjective quality. >> When turned on (default) the objective quality/compression efficiency is >> greatly improved, but at the cost of potential subjective quality loss at >> keyframes (because they are filtered and can appear blurry). Therefore >> users who do not want to have such artifacts may want to disable the >> option. This is actually a long-standing trade-off that libaom users have >> to (sometimes painfully) choose, and they cannot do so with ffmpeg right >> now. Also recently a new experimental option (=2) was added to libaom in >> order to mitigate the subjective loss. It would be nice to expose this >> option for people to be able to test it using ffmpeg with various decoders >> too. I've observed a few related discussions in the AV1 community (e.g. the >> reddit thread on the experimental option: >> https://www.reddit.com/r/AV1/comments/ic7ktu/aom_fixed_its_filtered_reference_frame_issues/ >> ). >> >> That said, I do understand the rationale and the reason why people might >> be opposed to adding another option, though I still suggest that we add the >> option for now, and when the API is available, we can make decisions and >> deprecate certain ones that are not necessary. Please let me know your >> thoughts on this, meanwhile I'll file a feature request to the aomedia bug >> tracker for the API. >> >> Thank you again for the comments! >> Best, >> Bohan >> >> On Fri, Nov 6, 2020 at 3:16 AM Jan Ekström wrote: >> >>> On Fri, Nov 6, 2020 at 11:46 AM Steven Liu wrote: >>> > >>> > >>> > >>> > > 2020年11月6日 下午4:42,Bohan Li 写道: >>> > > >>> > > Thanks for the reply, Steven! >>> > > >>> > > Regarding JEEB’s comment, the suggestion was to add an api to the >>> *libaom* library, not to ffmpeg. I do agree with the rationale, but when >>> such an api would be available for ffmpeg to use is quite uncertain. In the >>> meanwhile, I believe it is reasonable to add in this patch to expose the >>> option to the users. I don’t think this is a “middle version”, since as >>> mentioned, this option could be quite useful in certain scenarios and IMHO >>> should be added in. >>> > What about use av1_param arguments? As x264opts, x265opts? >>> >>> The problem with this is that libaom as far as I can tell doesn't have >>> an API like that. Thus each and every API user that needs to have >>> options defined as strings will have to *duplicate* those mappings >>> (which already are defined in the aomenc command line application, but >>> not exposed through the API). >>> >>> This has been the butt of jokes and reason for lamentations on IRC for >>> quite a while, but unfortunately nobody actually seems to have voiced >>> an opinion on this on the mailing list until now? >>> >>> Thus there generally speaking are only two ways forward for this: >>> 1. This is really spammy and unfortunate (and getting these removed >>> will be a PITA after we actually do get a key & value based API), but >>> we take the additional option in. >>> 2. This is apparently a low usage option and the amount of AVOptions >>> in this encoder is getting higher and higher, thus we deny the patch >>> until a key, value based API is provided. >>> >>> Personally I lean somewhat towards 2, but if the option is >>> needed/useful (use cases can be provided), then begrudgingly I could >>> go for 1. >>> >>> So far the lack of comments has mostly been regarding people not >>> having high enough stakes/care regarding this module, methinks? >>> >>> Jan >>> ___ >>> 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".
[FFmpeg-devel] [PATCH] Add support for the new key & value API in libaom.
This key & value API can greatly help with users who wants to try libaom-av1 specific options that are not supported by ffmpeg options. As was previously discussed in this thread: https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658. The commit that added the API to libaom: https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257 The libaom issue tracker: https://bugs.chromium.org/p/aomedia/issues/detail?id=2875 Signed-off-by: Bohan Li --- doc/encoders.texi | 10 ++ libavcodec/libaomenc.c | 32 2 files changed, 42 insertions(+) diff --git a/doc/encoders.texi b/doc/encoders.texi index c2ba7d3e6f..3f32303b3d 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1684,6 +1684,16 @@ Enable interintra compound. Default is true. @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0) Enable smooth interintra mode. Default is true. +@item libaom-params +Set libaom options using a list of @var{key}=@var{value} pairs separated +by ":". See @command{aomenc --help} for a list of options. + +For example to specify libaom encoding options with @option{-libaom-params}: + +@example +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params tune=psnr:enable-tpl-model=1 output.mp4 +@end example + @end table @section libsvtav1 diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 342d0883e4..b7c5a64417 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { int enable_diff_wtd_comp; int enable_dist_wtd_comp; int enable_dual_filter; +AVDictionary *extra_params; } AOMContext; static const char *const ctlidstr[] = { @@ -318,6 +319,27 @@ static av_cold int codecctl_int(AVCodecContext *avctx, return 0; } +static av_cold int codec_set_option(AVCodecContext *avctx, +const char* key, +const char* value) +{ +AOMContext *ctx = avctx->priv_data; +char buf[80]; +int width = -30; +int res; + +snprintf(buf, sizeof(buf), "%s:", key); +av_log(avctx, AV_LOG_DEBUG, " %*s%s: %s\n", width, buf, key, value); + +res = aom_codec_set_option(&ctx->encoder, key, value); +if (res != AOM_CODEC_OK) { +log_encoder_error(avctx, buf); +return AVERROR(EINVAL); +} + +return 0; +} + static av_cold int aom_free(AVCodecContext *avctx) { AOMContext *ctx = avctx->priv_data; @@ -874,6 +896,15 @@ static av_cold int aom_init(AVCodecContext *avctx, codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc); #endif +#if AOM_ENCODER_ABI_VERSION >= 23 +{ + AVDictionaryEntry *en = NULL; + while ((en = av_dict_get(ctx->extra_params, "", en, AV_DICT_IGNORE_SUFFIX))) { +codec_set_option(avctx, en->key, en->value); + } +} +#endif + // provide dummy value to initialize wrapper, values will be updated each _encode() aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1, (unsigned char*)1); @@ -1299,6 +1330,7 @@ static const AVOption options[] = { { "enable-masked-comp", "Enable masked compound", OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-interintra-comp", "Enable interintra compound", OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-smooth-interintra", "Enable smooth interintra mode", OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, +{ "libaom-params","Extra parameters for libaom", OFFSET(extra_params), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE }, { NULL }, }; -- 2.30.0.365.g02bc693789-goog ___ 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] [PATCH] Add support for the new key & value API in libaom.
This key & value API can greatly help with users who wants to try libaom-av1 specific options that are not supported by ffmpeg options. As was previously discussed in this thread: https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658. The commit that added the API to libaom: https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257 The libaom issue tracker: https://bugs.chromium.org/p/aomedia/issues/detail?id=2875 Signed-off-by: Bohan Li --- doc/encoders.texi | 10 ++ libavcodec/libaomenc.c | 32 2 files changed, 42 insertions(+) diff --git a/doc/encoders.texi b/doc/encoders.texi index c2ba7d3e6f..3f32303b3d 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1684,6 +1684,16 @@ Enable interintra compound. Default is true. @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0) Enable smooth interintra mode. Default is true. +@item libaom-params +Set libaom options using a list of @var{key}=@var{value} pairs separated +by ":". See @command{aomenc --help} for a list of options. + +For example to specify libaom encoding options with @option{-libaom-params}: + +@example +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params tune=psnr:enable-tpl-model=1 output.mp4 +@end example + @end table @section libsvtav1 diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 342d0883e4..b7c5a64417 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext { int enable_diff_wtd_comp; int enable_dist_wtd_comp; int enable_dual_filter; +AVDictionary *extra_params; } AOMContext; static const char *const ctlidstr[] = { @@ -318,6 +319,27 @@ static av_cold int codecctl_int(AVCodecContext *avctx, return 0; } +static av_cold int codec_set_option(AVCodecContext *avctx, +const char* key, +const char* value) +{ +AOMContext *ctx = avctx->priv_data; +char buf[80]; +int width = -30; +int res; + +snprintf(buf, sizeof(buf), "%s:", key); +av_log(avctx, AV_LOG_DEBUG, " %*s%s: %s\n", width, buf, key, value); + +res = aom_codec_set_option(&ctx->encoder, key, value); +if (res != AOM_CODEC_OK) { +log_encoder_error(avctx, buf); +return AVERROR(EINVAL); +} + +return 0; +} + static av_cold int aom_free(AVCodecContext *avctx) { AOMContext *ctx = avctx->priv_data; @@ -874,6 +896,15 @@ static av_cold int aom_init(AVCodecContext *avctx, codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc); #endif +#if AOM_ENCODER_ABI_VERSION >= 23 +{ + AVDictionaryEntry *en = NULL; + while ((en = av_dict_get(ctx->extra_params, "", en, AV_DICT_IGNORE_SUFFIX))) { +codec_set_option(avctx, en->key, en->value); + } +} +#endif + // provide dummy value to initialize wrapper, values will be updated each _encode() aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1, (unsigned char*)1); @@ -1299,6 +1330,7 @@ static const AVOption options[] = { { "enable-masked-comp", "Enable masked compound", OFFSET(enable_masked_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-interintra-comp", "Enable interintra compound", OFFSET(enable_interintra_comp), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, { "enable-smooth-interintra", "Enable smooth interintra mode", OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, +{ "libaom-params","Extra parameters for libaom", OFFSET(extra_params), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE }, { NULL }, }; -- 2.30.0.365.g02bc693789-goog ___ 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] [PATCH] Add unmet target level warning to libaom encoding.
When target levels are set, this patch checks whether they are satisfied by libaom. If not, a warning is shown. Otherwise the output levels are also logged. This patch applies basically the same approach used for libvpx. Signed-off-by: Bohan Li --- libavcodec/libaomenc.c | 64 ++ 1 file changed, 64 insertions(+) diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 054903e6e2..402bcb5198 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -198,6 +198,12 @@ static const char *const ctlidstr[] = { [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS", #endif +#ifdef AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX +[AV1E_GET_SEQ_LEVEL_IDX]= "AV1E_GET_SEQ_LEVEL_IDX", +#endif +#ifdef AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX +[AV1E_GET_TARGET_SEQ_LEVEL_IDX] = "AV1E_GET_TARGET_SEQ_LEVEL_IDX", +#endif }; static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc) @@ -323,10 +329,68 @@ static av_cold int codecctl_int(AVCodecContext *avctx, return 0; } +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) +static av_cold int codecctl_intp(AVCodecContext *avctx, +#ifdef UENUM1BYTE + aome_enc_control_id id, +#else + enum aome_enc_control_id id, +#endif + int* ptr) +{ +AOMContext *ctx = avctx->priv_data; +char buf[80]; +int width = -30; +int res; + +snprintf(buf, sizeof(buf), "%s:", ctlidstr[id]); +av_log(avctx, AV_LOG_DEBUG, " %*s%d\n", width, buf, *ptr); + +res = aom_codec_control(&ctx->encoder, id, ptr); +if (res != AOM_CODEC_OK) { +snprintf(buf, sizeof(buf), "Failed to set %s codec control", + ctlidstr[id]); +log_encoder_error(avctx, buf); +return AVERROR(EINVAL); +} + +return 0; +} +#endif + static av_cold int aom_free(AVCodecContext *avctx) { AOMContext *ctx = avctx->priv_data; +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) +if (!(avctx->flags & AV_CODEC_FLAG_PASS1)) { +int levels[32] = { 0 }; +int target_levels[32] = { 0 }; + +if (!codecctl_intp(avctx, AV1E_GET_SEQ_LEVEL_IDX, levels) && +!codecctl_intp(avctx, AV1E_GET_TARGET_SEQ_LEVEL_IDX, + target_levels)) { +for (int i = 0; i < 32; i++) { +if (levels[i] > target_levels[i]) { +// Warn when the target level was not met +av_log(avctx, AV_LOG_WARNING, + "Could not encode to target level %d.%d for " + "operating point %d. The output level is %d.%d.", + 2 + (target_levels[i] >> 2), target_levels[i] & 3, + i, 2 + (levels[i] >> 2), levels[i] & 3); +} else if (target_levels[i] < 31) { +// Log the encoded level if a target level was given +av_log(avctx, AV_LOG_INFO, + "Output level for operating point %d is %d.%d.", + i, 2 + (levels[i] >> 2), levels[i] & 3); +} +} +} +} +#endif + aom_codec_destroy(&ctx->encoder); av_freep(&ctx->twopass_stats.buf); av_freep(&avctx->stats_out); -- 2.36.0.rc0.470.gd361397f0d-goog ___ 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] [PATCH] avcodec/libaomenc: Add unmet target level warning
When target levels are set, this patch checks whether they are satisfied by libaom. If not, a warning is shown. Otherwise the output levels are also logged. This patch applies basically the same approach used for libvpx. Signed-off-by: Bohan Li --- libavcodec/libaomenc.c | 64 ++ 1 file changed, 64 insertions(+) diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 054903e6e2..402bcb5198 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -198,6 +198,12 @@ static const char *const ctlidstr[] = { [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS", #endif +#ifdef AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX +[AV1E_GET_SEQ_LEVEL_IDX]= "AV1E_GET_SEQ_LEVEL_IDX", +#endif +#ifdef AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX +[AV1E_GET_TARGET_SEQ_LEVEL_IDX] = "AV1E_GET_TARGET_SEQ_LEVEL_IDX", +#endif }; static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc) @@ -323,10 +329,68 @@ static av_cold int codecctl_int(AVCodecContext *avctx, return 0; } +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) +static av_cold int codecctl_intp(AVCodecContext *avctx, +#ifdef UENUM1BYTE + aome_enc_control_id id, +#else + enum aome_enc_control_id id, +#endif + int* ptr) +{ +AOMContext *ctx = avctx->priv_data; +char buf[80]; +int width = -30; +int res; + +snprintf(buf, sizeof(buf), "%s:", ctlidstr[id]); +av_log(avctx, AV_LOG_DEBUG, " %*s%d\n", width, buf, *ptr); + +res = aom_codec_control(&ctx->encoder, id, ptr); +if (res != AOM_CODEC_OK) { +snprintf(buf, sizeof(buf), "Failed to set %s codec control", + ctlidstr[id]); +log_encoder_error(avctx, buf); +return AVERROR(EINVAL); +} + +return 0; +} +#endif + static av_cold int aom_free(AVCodecContext *avctx) { AOMContext *ctx = avctx->priv_data; +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) +if (!(avctx->flags & AV_CODEC_FLAG_PASS1)) { +int levels[32] = { 0 }; +int target_levels[32] = { 0 }; + +if (!codecctl_intp(avctx, AV1E_GET_SEQ_LEVEL_IDX, levels) && +!codecctl_intp(avctx, AV1E_GET_TARGET_SEQ_LEVEL_IDX, + target_levels)) { +for (int i = 0; i < 32; i++) { +if (levels[i] > target_levels[i]) { +// Warn when the target level was not met +av_log(avctx, AV_LOG_WARNING, + "Could not encode to target level %d.%d for " + "operating point %d. The output level is %d.%d.", + 2 + (target_levels[i] >> 2), target_levels[i] & 3, + i, 2 + (levels[i] >> 2), levels[i] & 3); +} else if (target_levels[i] < 31) { +// Log the encoded level if a target level was given +av_log(avctx, AV_LOG_INFO, + "Output level for operating point %d is %d.%d.", + i, 2 + (levels[i] >> 2), levels[i] & 3); +} +} +} +} +#endif + aom_codec_destroy(&ctx->encoder); av_freep(&ctx->twopass_stats.buf); av_freep(&avctx->stats_out); -- 2.36.0.rc0.470.gd361397f0d-goog ___ 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] [PATCH v2] avcodec/libaomenc: Add unmet target level warning
When target levels are set, this patch checks whether they are satisfied by libaom. If not, a warning is shown. Otherwise the output levels are also logged. This patch applies basically the same approach used for libvpx. Signed-off-by: Bohan Li --- libavcodec/libaomenc.c | 64 ++ 1 file changed, 64 insertions(+) diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 054903e6e2..77be56fa51 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -198,6 +198,12 @@ static const char *const ctlidstr[] = { [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS", #endif +#ifdef AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX +[AV1E_GET_SEQ_LEVEL_IDX]= "AV1E_GET_SEQ_LEVEL_IDX", +#endif +#ifdef AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX +[AV1E_GET_TARGET_SEQ_LEVEL_IDX] = "AV1E_GET_TARGET_SEQ_LEVEL_IDX", +#endif }; static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc) @@ -323,10 +329,68 @@ static av_cold int codecctl_int(AVCodecContext *avctx, return 0; } +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) +static av_cold int codecctl_intp(AVCodecContext *avctx, +#ifdef UENUM1BYTE + aome_enc_control_id id, +#else + enum aome_enc_control_id id, +#endif + int* ptr) +{ +AOMContext *ctx = avctx->priv_data; +char buf[80]; +int width = -30; +int res; + +snprintf(buf, sizeof(buf), "%s:", ctlidstr[id]); +av_log(avctx, AV_LOG_DEBUG, " %*s%d\n", width, buf, *ptr); + +res = aom_codec_control(&ctx->encoder, id, ptr); +if (res != AOM_CODEC_OK) { +snprintf(buf, sizeof(buf), "Failed to set %s codec control", + ctlidstr[id]); +log_encoder_error(avctx, buf); +return AVERROR(EINVAL); +} + +return 0; +} +#endif + static av_cold int aom_free(AVCodecContext *avctx) { AOMContext *ctx = avctx->priv_data; +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) +if (!(avctx->flags & AV_CODEC_FLAG_PASS1)) { +int levels[32] = { 0 }; +int target_levels[32] = { 0 }; + +if (!codecctl_intp(avctx, AV1E_GET_SEQ_LEVEL_IDX, levels) && +!codecctl_intp(avctx, AV1E_GET_TARGET_SEQ_LEVEL_IDX, + target_levels)) { +for (int i = 0; i < 32; i++) { +if (levels[i] > target_levels[i]) { +// Warn when the target level was not met +av_log(avctx, AV_LOG_WARNING, + "Could not encode to target level %d.%d for " + "operating point %d. The output level is %d.%d.\n", + 2 + (target_levels[i] >> 2), target_levels[i] & 3, + i, 2 + (levels[i] >> 2), levels[i] & 3); +} else if (target_levels[i] < 31) { +// Log the encoded level if a target level was given +av_log(avctx, AV_LOG_INFO, + "Output level for operating point %d is %d.%d.\n", + i, 2 + (levels[i] >> 2), levels[i] & 3); +} +} +} +} +#endif + aom_codec_destroy(&ctx->encoder); av_freep(&ctx->twopass_stats.buf); av_freep(&avctx->stats_out); -- 2.36.0.rc0.470.gd361397f0d-goog ___ 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".
Re: [FFmpeg-devel] [PATCH v2] avcodec/libaomenc: Add unmet target level warning
Gentle ping on this :) On Tue, Apr 19, 2022 at 11:20 AM Bohan Li wrote: > When target levels are set, this patch checks whether they are > satisfied by libaom. If not, a warning is shown. Otherwise the output > levels are also logged. > > This patch applies basically the same approach used for libvpx. > > Signed-off-by: Bohan Li > --- > libavcodec/libaomenc.c | 64 ++ > 1 file changed, 64 insertions(+) > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > index 054903e6e2..77be56fa51 100644 > --- a/libavcodec/libaomenc.c > +++ b/libavcodec/libaomenc.c > @@ -198,6 +198,12 @@ static const char *const ctlidstr[] = { > [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = > "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", > [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS", > #endif > +#ifdef AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX > +[AV1E_GET_SEQ_LEVEL_IDX]= "AV1E_GET_SEQ_LEVEL_IDX", > +#endif > +#ifdef AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX > +[AV1E_GET_TARGET_SEQ_LEVEL_IDX] = "AV1E_GET_TARGET_SEQ_LEVEL_IDX", > +#endif > }; > > static av_cold void log_encoder_error(AVCodecContext *avctx, const char > *desc) > @@ -323,10 +329,68 @@ static av_cold int codecctl_int(AVCodecContext > *avctx, > return 0; > } > > +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ > +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) > +static av_cold int codecctl_intp(AVCodecContext *avctx, > +#ifdef UENUM1BYTE > + aome_enc_control_id id, > +#else > + enum aome_enc_control_id id, > +#endif > + int* ptr) > +{ > +AOMContext *ctx = avctx->priv_data; > +char buf[80]; > +int width = -30; > +int res; > + > +snprintf(buf, sizeof(buf), "%s:", ctlidstr[id]); > +av_log(avctx, AV_LOG_DEBUG, " %*s%d\n", width, buf, *ptr); > + > +res = aom_codec_control(&ctx->encoder, id, ptr); > +if (res != AOM_CODEC_OK) { > +snprintf(buf, sizeof(buf), "Failed to set %s codec control", > + ctlidstr[id]); > +log_encoder_error(avctx, buf); > +return AVERROR(EINVAL); > +} > + > +return 0; > +} > +#endif > + > static av_cold int aom_free(AVCodecContext *avctx) > { > AOMContext *ctx = avctx->priv_data; > > +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ > +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) > +if (!(avctx->flags & AV_CODEC_FLAG_PASS1)) { > +int levels[32] = { 0 }; > +int target_levels[32] = { 0 }; > + > +if (!codecctl_intp(avctx, AV1E_GET_SEQ_LEVEL_IDX, levels) && > +!codecctl_intp(avctx, AV1E_GET_TARGET_SEQ_LEVEL_IDX, > + target_levels)) { > +for (int i = 0; i < 32; i++) { > +if (levels[i] > target_levels[i]) { > +// Warn when the target level was not met > +av_log(avctx, AV_LOG_WARNING, > + "Could not encode to target level %d.%d for " > + "operating point %d. The output level is > %d.%d.\n", > + 2 + (target_levels[i] >> 2), target_levels[i] > & 3, > + i, 2 + (levels[i] >> 2), levels[i] & 3); > +} else if (target_levels[i] < 31) { > +// Log the encoded level if a target level was given > +av_log(avctx, AV_LOG_INFO, > + "Output level for operating point %d is > %d.%d.\n", > + i, 2 + (levels[i] >> 2), levels[i] & 3); > +} > +} > +} > +} > +#endif > + > aom_codec_destroy(&ctx->encoder); > av_freep(&ctx->twopass_stats.buf); > av_freep(&avctx->stats_out); > -- > 2.36.0.rc0.470.gd361397f0d-goog > > ___ 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".
Re: [FFmpeg-devel] [PATCH v2] avcodec/libaomenc: Add unmet target level warning
Another ping :) On Fri, Apr 29, 2022 at 2:46 PM Bohan Li wrote: > Gentle ping on this :) > > On Tue, Apr 19, 2022 at 11:20 AM Bohan Li wrote: > >> When target levels are set, this patch checks whether they are >> satisfied by libaom. If not, a warning is shown. Otherwise the output >> levels are also logged. >> >> This patch applies basically the same approach used for libvpx. >> >> Signed-off-by: Bohan Li >> --- >> libavcodec/libaomenc.c | 64 ++ >> 1 file changed, 64 insertions(+) >> >> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c >> index 054903e6e2..77be56fa51 100644 >> --- a/libavcodec/libaomenc.c >> +++ b/libavcodec/libaomenc.c >> @@ -198,6 +198,12 @@ static const char *const ctlidstr[] = { >> [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = >> "AV1E_SET_ENABLE_SMOOTH_INTERINTRA", >> [AV1E_SET_ENABLE_REF_FRAME_MVS] = >> "AV1E_SET_ENABLE_REF_FRAME_MVS", >> #endif >> +#ifdef AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX >> +[AV1E_GET_SEQ_LEVEL_IDX]= "AV1E_GET_SEQ_LEVEL_IDX", >> +#endif >> +#ifdef AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX >> +[AV1E_GET_TARGET_SEQ_LEVEL_IDX] = >> "AV1E_GET_TARGET_SEQ_LEVEL_IDX", >> +#endif >> }; >> >> static av_cold void log_encoder_error(AVCodecContext *avctx, const char >> *desc) >> @@ -323,10 +329,68 @@ static av_cold int codecctl_int(AVCodecContext >> *avctx, >> return 0; >> } >> >> +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ >> +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) >> +static av_cold int codecctl_intp(AVCodecContext *avctx, >> +#ifdef UENUM1BYTE >> + aome_enc_control_id id, >> +#else >> + enum aome_enc_control_id id, >> +#endif >> + int* ptr) >> +{ >> +AOMContext *ctx = avctx->priv_data; >> +char buf[80]; >> +int width = -30; >> +int res; >> + >> +snprintf(buf, sizeof(buf), "%s:", ctlidstr[id]); >> +av_log(avctx, AV_LOG_DEBUG, " %*s%d\n", width, buf, *ptr); >> + >> +res = aom_codec_control(&ctx->encoder, id, ptr); >> +if (res != AOM_CODEC_OK) { >> +snprintf(buf, sizeof(buf), "Failed to set %s codec control", >> + ctlidstr[id]); >> +log_encoder_error(avctx, buf); >> +return AVERROR(EINVAL); >> +} >> + >> +return 0; >> +} >> +#endif >> + >> static av_cold int aom_free(AVCodecContext *avctx) >> { >> AOMContext *ctx = avctx->priv_data; >> >> +#if defined(AOM_CTRL_AV1E_GET_SEQ_LEVEL_IDX) && \ >> +defined(AOM_CTRL_AV1E_GET_TARGET_SEQ_LEVEL_IDX) >> +if (!(avctx->flags & AV_CODEC_FLAG_PASS1)) { >> +int levels[32] = { 0 }; >> +int target_levels[32] = { 0 }; >> + >> +if (!codecctl_intp(avctx, AV1E_GET_SEQ_LEVEL_IDX, levels) && >> +!codecctl_intp(avctx, AV1E_GET_TARGET_SEQ_LEVEL_IDX, >> + target_levels)) { >> +for (int i = 0; i < 32; i++) { >> +if (levels[i] > target_levels[i]) { >> +// Warn when the target level was not met >> +av_log(avctx, AV_LOG_WARNING, >> + "Could not encode to target level %d.%d for " >> + "operating point %d. The output level is >> %d.%d.\n", >> + 2 + (target_levels[i] >> 2), target_levels[i] >> & 3, >> + i, 2 + (levels[i] >> 2), levels[i] & 3); >> +} else if (target_levels[i] < 31) { >> +// Log the encoded level if a target level was given >> +av_log(avctx, AV_LOG_INFO, >> + "Output level for operating point %d is >> %d.%d.\n", >> + i, 2 + (levels[i] >> 2), levels[i] & 3); >> +} >> +} >> +} >> +} >> +#endif >> + >> aom_codec_destroy(&ctx->encoder); >> av_freep(&ctx->twopass_stats.buf); >> av_freep(&avctx->stats_out); >> -- >> 2.36.0.rc0.470.gd361397f0d-goog >> >> ___ 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".
Re: [FFmpeg-devel] [PATCH v2] avcodec/libaomenc: Add unmet target level warning
Thanks for the reply, James! This is indeed not the intended behaviour, but it is due to libaom not initializing all the indices correctly. I've submitted a patch for review in libaom that fixes this, and after that patch the levels are only logged when a target level is given with this ffmpeg patch. Best, Bohan On Tue, May 24, 2022 at 5:43 PM James Zern wrote: > On Tue, May 17, 2022 at 12:45 PM James Zern wrote: > > > > On Tue, Apr 19, 2022 at 11:20 AM Bohan Li > > wrote: > > > > > > When target levels are set, this patch checks whether they are > > > satisfied by libaom. If not, a warning is shown. Otherwise the output > > > levels are also logged. > > > > > > This patch applies basically the same approach used for libvpx. > > > > > > Signed-off-by: Bohan Li > > > --- > > > libavcodec/libaomenc.c | 64 ++ > > > 1 file changed, 64 insertions(+) > > > > > > > lgtm. > > > +} else if (target_levels[i] < 31) { > > +// Log the encoded level if a target level was given > > +av_log(avctx, AV_LOG_INFO, > > + "Output level for operating point %d is > %d.%d.", > > + i, 2 + (levels[i] >> 2), levels[i] & 3); > > +} > > Actually this is a bit spammy. If there's only one operating point set > then I'd expect a single line output, but this seems to print all 32 > regardless. Is that expected? > ___ 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".