| Message ID | 20200303215903.46180-2-wangcao@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] libavcodec/libaomenc.c: Add a libaom command-line opton 'tune' | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Wed, Mar 04, 2020 at 05:59:03 +0800, Wang Cao wrote: > Signed-off-by: Wang Cao <wangcao@google.com> > --- > doc/encoders.texi | 39 +++++++++++++++++++++++++++++++++++++++ > libavcodec/libaomenc.c | 38 ++++++++++++++++++++++++++++++++++++++ Again, I suggest bumping MICRO once more. > +@item superres_denominator > +The denominator for superres to use when @option{superres-mode} is @option{fixed}. Valid value > +ranges from 8 to 16. > + > +@item superres_kf_denominator > +The denominator for superres to use on key frames is fixed when > +@option{superres-mode} is @option{fixed}. Valid value ranges from 8 to 16. I believe "is fixed " is not needed in this sentence, or even confusing. > [AOME_SET_TUNING] = "AOME_SET_TUNING", > + [AV1E_SET_ENABLE_SUPERRES] = "AV1E_SET_ENABLE_SUPERRES", The other '=' in this block are aligned. > + if (ctx->superres_mode >= 0) > + enccfg.rc_superres_mode = ctx->superres_mode; > + if (ctx->superres_qthresh > 0) > + enccfg.rc_superres_qthresh = ctx->superres_qthresh; > + if (ctx->superres_kf_qthresh > 0) > + enccfg.rc_superres_kf_qthresh = ctx->superres_kf_qthresh; > + if (ctx->superres_denominator >= 0) > + enccfg.rc_superres_denominator = ctx->superres_denominator; > + if (ctx->superres_kf_denominator >= 0) > + enccfg.rc_superres_kf_denominator = ctx->superres_kf_denominator; It looks like indentation is off - ffmpeg uses four spaces. (Is this struct zero-initialized / memset()'d?) > { "ssim", "SSIM as distortion metric", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, "tune"}, > + { "enable-superres", "Enable super-resolution mode", OFFSET(enable_superres), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, > + { "superres-mode", "Select super-resultion mode", OFFSET(superres_mode), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 4, VE, "superres_mode"}, > + { "none", "No frame superres allowed", 0, AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 0, VE, "superres_mode"}, > + { "fixed", "All frames are coded at the specified scale and super-resolved", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, "superres_mode"}, > + { "random", "All frames are coded at a random scale and super-resolved.", 0, AV_OPT_TYPE_CONST, {.i64 = 2}, 0, 0, VE, "superres_mode"}, > + { "qthresh", "Superres scale for a frame is determined based on q_index", 0, AV_OPT_TYPE_CONST, {.i64 = 3}, 0, 0, VE, "superres_mode"}, > + { "auto", "Automatically select superres for appropriate frames", 0, AV_OPT_TYPE_CONST, {.i64 = 4}, 0, 0, VE, "superres_mode"}, Several remarks: - The "none" entry should also be aligned, just like the entry "fixed" and following... (starting at "0, AV_OPT_TYPE_CONST", if you see what I mean). - Is "auto" a value given by the library? I'm asking because we tend to use "-1" for our internal "auto", and use that as a default, if desired. (From looking at libaom, they indeed use 4 for "auto".) - Can you directly use the enumerations provided as SUPERRES_MODE by libaom, or are they not public? - If you cannot reuse said enum (and its resulting range [-1, SUPERRES_MODES - 1]), you should define your own as enum SuperresModes { AOM_SUPERRES_MODE_NONE, AOM_SUPERRES_MODE_FIXED, ...., AOM_SUPERRES_MODE_NB }, use these in the options definition above, and set the allowed range to [-1, AOM_SUPERRES_MODE_NB - 1]. Cheers, Moritz
On Wed, Mar 4, 2020 at 1:38 AM Moritz Barsnick <barsnick@gmx.net> wrote: > On Wed, Mar 04, 2020 at 05:59:03 +0800, Wang Cao wrote: > > Signed-off-by: Wang Cao <wangcao@google.com> > > --- > > doc/encoders.texi | 39 +++++++++++++++++++++++++++++++++++++++ > > libavcodec/libaomenc.c | 38 ++++++++++++++++++++++++++++++++++++++ > > Again, I suggest bumping MICRO once more. > Done. > > +@item superres_denominator > > +The denominator for superres to use when @option{superres-mode} is > @option{fixed}. Valid value > > +ranges from 8 to 16. > > + > > +@item superres_kf_denominator > > +The denominator for superres to use on key frames is fixed when > > +@option{superres-mode} is @option{fixed}. Valid value ranges from 8 to > 16. > > I believe "is fixed " is not needed in this sentence, or even > confusing. > Done. > > [AOME_SET_TUNING] = "AOME_SET_TUNING", > > + [AV1E_SET_ENABLE_SUPERRES] = "AV1E_SET_ENABLE_SUPERRES", > > The other '=' in this block are aligned. > > > + if (ctx->superres_mode >= 0) > > + enccfg.rc_superres_mode = ctx->superres_mode; > > + if (ctx->superres_qthresh > 0) > > + enccfg.rc_superres_qthresh = ctx->superres_qthresh; > > + if (ctx->superres_kf_qthresh > 0) > > + enccfg.rc_superres_kf_qthresh = ctx->superres_kf_qthresh; > > + if (ctx->superres_denominator >= 0) > > + enccfg.rc_superres_denominator = ctx->superres_denominator; > > + if (ctx->superres_kf_denominator >= 0) > > + enccfg.rc_superres_kf_denominator = ctx->superres_kf_denominator; > > It looks like indentation is off - ffmpeg uses four spaces. > > (Is this struct zero-initialized / memset()'d?) Yes it is initialized before through aom_codec_enc_config_default > > > { "ssim", "SSIM as distortion metric", 0, > AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, "tune"}, > > + { "enable-superres", "Enable super-resolution mode", > OFFSET(enable_superres), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, > > + { "superres-mode", "Select super-resultion mode", > OFFSET(superres_mode), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 4, VE, > "superres_mode"}, > > + { "none", "No frame superres allowed", 0, > AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 0, VE, "superres_mode"}, > > + { "fixed", "All frames are coded at the specified scale > and super-resolved", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, > "superres_mode"}, > > + { "random", "All frames are coded at a random scale and > super-resolved.", 0, AV_OPT_TYPE_CONST, {.i64 = 2}, 0, 0, VE, > "superres_mode"}, > > + { "qthresh", "Superres scale for a frame is determined > based on q_index", 0, AV_OPT_TYPE_CONST, {.i64 = 3}, 0, 0, VE, > "superres_mode"}, > > + { "auto", "Automatically select superres for appropriate > frames", 0, AV_OPT_TYPE_CONST, {.i64 = 4}, 0, 0, VE, > "superres_mode"}, > > Several remarks: > - The "none" entry should also be aligned, just like the entry "fixed" > and following... (starting at "0, AV_OPT_TYPE_CONST", if you see > what I mean). > - Is "auto" a value given by the library? I'm asking because > we tend to use "-1" for our internal "auto", and use that as a > default, if desired. > (From looking at libaom, they indeed use 4 for "auto".) > - Can you directly use the enumerations provided as SUPERRES_MODE by > libaom, or are they not public? > - If you cannot reuse said enum (and its resulting range > [-1, SUPERRES_MODES - 1]), you should define your own as enum > SuperresModes { AOM_SUPERRES_MODE_NONE, AOM_SUPERRES_MODE_FIXED, ...., > AOM_SUPERRES_MODE_NB }, use these in the options definition above, > and set the allowed range to [-1, AOM_SUPERRES_MODE_NB - 1]. > > This is a good approach to improve the readability. Done. > Cheers, > Moritz > Please find the changes in the updated patch. Thanks!
diff --git a/doc/encoders.texi b/doc/encoders.texi index 4215f237bd..ac00c305d5 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1608,6 +1608,45 @@ Enable the use of global motion for block prediction. Default is true. Enable block copy mode for intra block prediction. This mode is useful for screen content. Default is true. +@item enable-superres (@emph{boolean}) +Enable super-resolution during the encoding process. + +@item superres-mode (@emph{mode}) +Select super-resultion mode + +@table @option +@item none (@emph{0}) +No frame superres allowed + +@item fixed (@emph{1}) +All frames are coded at the specified scale and super-resolved + +@item random (@emph{2}) +All frames are coded at a random scale and super-resolved. + +@item qthresh (@emph{3}) +Superres scale for a frame is determined based on q_index + +@item auto (@emph{4}) +Automatically select superres for appropriate frames +@end table + +@item superres_denominator +The denominator for superres to use when @option{superres-mode} is @option{fixed}. Valid value +ranges from 8 to 16. + +@item superres_kf_denominator +The denominator for superres to use on key frames is fixed when +@option{superres-mode} is @option{fixed}. Valid value ranges from 8 to 16. + +@item superres_qthresh +The q level threshold after which superres is used when @option{superres-mode} is +@option{qthresh}. Valid value ranges from 1 to 63. + +@item superres_kf_qthresh +The q level threshold after which superres is used for key frames when +@option{superres-mode} is @option{qthresh}. Valid value ranges from 1 to 63. + @end table @section libkvazaar diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 7fd624c470..f1578db5e6 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -95,6 +95,12 @@ typedef struct AOMEncoderContext { int enable_restoration; int usage; int tune; + int enable_superres; + int superres_mode; + int superres_denominator; + int superres_qthresh; + int superres_kf_denominator; + int superres_kf_qthresh; } AOMContext; static const char *const ctlidstr[] = { @@ -134,6 +140,7 @@ static const char *const ctlidstr[] = { #endif [AV1E_SET_ENABLE_CDEF] = "AV1E_SET_ENABLE_CDEF", [AOME_SET_TUNING] = "AOME_SET_TUNING", + [AV1E_SET_ENABLE_SUPERRES] = "AV1E_SET_ENABLE_SUPERRES", }; static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc) @@ -203,6 +210,13 @@ static av_cold void dump_enc_cfg(AVCodecContext *avctx, width, "tile_width_count:", cfg->tile_width_count, width, "tile_height_count:", cfg->tile_height_count); av_log(avctx, level, "\n"); + av_log(avctx, level, "super resolution settings\n" + "%*s%u\n %*s%u\n %*s%u\n %*s%u\n %*s%u\n ", + width, "rc_superres_mode:", cfg->rc_superres_mode, + width, "rc_superres_denominator:",cfg->rc_superres_denominator, + width, "rc_superres_qthresh:", cfg->rc_superres_qthresh, + width, "rc_superres_kf_denominator:", cfg->rc_superres_kf_denominator, + width, "rc_superres_kf_qthresh:", cfg->rc_superres_kf_qthresh); } static void coded_frame_add(void *list, struct FrameListData *cx_frame) @@ -545,6 +559,17 @@ static av_cold int aom_init(AVCodecContext *avctx, return AVERROR(EINVAL); } + if (ctx->superres_mode >= 0) + enccfg.rc_superres_mode = ctx->superres_mode; + if (ctx->superres_qthresh > 0) + enccfg.rc_superres_qthresh = ctx->superres_qthresh; + if (ctx->superres_kf_qthresh > 0) + enccfg.rc_superres_kf_qthresh = ctx->superres_kf_qthresh; + if (ctx->superres_denominator >= 0) + enccfg.rc_superres_denominator = ctx->superres_denominator; + if (ctx->superres_kf_denominator >= 0) + enccfg.rc_superres_kf_denominator = ctx->superres_kf_denominator; + dump_enc_cfg(avctx, &enccfg); enccfg.g_w = avctx->width; @@ -687,6 +712,8 @@ static av_cold int aom_init(AVCodecContext *avctx, // codec control failures are currently treated only as warnings av_log(avctx, AV_LOG_DEBUG, "aom_codec_control\n"); codecctl_int(avctx, AOME_SET_CPUUSED, ctx->cpu_used); + if (ctx->enable_superres >= 0) + codecctl_int(avctx, AV1E_SET_ENABLE_SUPERRES, ctx->enable_superres); if (ctx->auto_alt_ref >= 0) codecctl_int(avctx, AOME_SET_ENABLEAUTOALTREF, ctx->auto_alt_ref); if (ctx->arnr_max_frames >= 0) @@ -1103,6 +1130,17 @@ static const AVOption options[] = { { "tune", "The metric that encoder tunes for", OFFSET(tune), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 1, VE, "tune"}, { "psnr", "PSNR as distortion metric", 0, AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 0, VE, "tune"}, { "ssim", "SSIM as distortion metric", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, "tune"}, + { "enable-superres", "Enable super-resolution mode", OFFSET(enable_superres), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE}, + { "superres-mode", "Select super-resultion mode", OFFSET(superres_mode), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 4, VE, "superres_mode"}, + { "none", "No frame superres allowed", 0, AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 0, VE, "superres_mode"}, + { "fixed", "All frames are coded at the specified scale and super-resolved", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, "superres_mode"}, + { "random", "All frames are coded at a random scale and super-resolved.", 0, AV_OPT_TYPE_CONST, {.i64 = 2}, 0, 0, VE, "superres_mode"}, + { "qthresh", "Superres scale for a frame is determined based on q_index", 0, AV_OPT_TYPE_CONST, {.i64 = 3}, 0, 0, VE, "superres_mode"}, + { "auto", "Automatically select superres for appropriate frames", 0, AV_OPT_TYPE_CONST, {.i64 = 4}, 0, 0, VE, "superres_mode"}, + { "superres-denominator", "The denominator for superres to use.", OFFSET(superres_denominator), AV_OPT_TYPE_INT, {.i64 = 8}, 8, 16, VE}, + { "superres-qthresh", "The q level threshold after which superres is used", OFFSET(superres_qthresh), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 63, VE}, + { "superres-kf-denominator", "The denominator for superres to use on key frames.", OFFSET(superres_kf_denominator), AV_OPT_TYPE_INT, {.i64 = 8}, 8, 16, VE}, + { "superres-kf-qthresh", "The q level threshold after which superres is used for key frames.", OFFSET(superres_kf_qthresh), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 63, VE}, { NULL }, };
Signed-off-by: Wang Cao <wangcao@google.com> --- doc/encoders.texi | 39 +++++++++++++++++++++++++++++++++++++++ libavcodec/libaomenc.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+)