On Wed, Mar 04, 2020 at 05:59:03 +0800, Wang Cao wrote: > Signed-off-by: Wang Cao <wang...@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 _______________________________________________ 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".