[FFmpeg-devel] [PATCH v2 1/2] avcodec/libaomenc.c: Add a libaom command-line option 'tune'

[James Zern]

Hi,

On Thu, Mar 5, 2020 at 6:38 PM Wang Cao <doubleecao at gmail.com> wrote:
>
> Signed-off-by: Wang Cao <wangcao at google.com>
> ---
> Made changes according to the review.
> - Bump the MICRO version of libavcodec.
> - Use enum for 'tune' option consts for better consistency
>
>  doc/encoders.texi      | 11 +++++++++++
>  libavcodec/libaomenc.c |  7 +++++++
>  libavcodec/version.h   |  2 +-
>  3 files changed, 19 insertions(+), 1 deletion(-)
>

Just some cosmetics.

> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index e23b6b32fe..0a74ecce9b 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -1508,6 +1508,17 @@ Complexity-based.
>  Cyclic refresh.
>  @end table
>
> + at item tune (@emph{tune})
> +Set the distortion metric the encoder is tune with. Default is @code{psnr}.
> +

...the encoder is tuned with...

> + at table @samp
> + at item psnr (@emph{0})
> +PSNR as distortion metric
> +

Given the description above and the name of the options the extra text
may be redundant.

> [...]
> @@ -1096,6 +1100,9 @@ static const AVOption options[] = {
>      { "usage",           "Quality and compression efficiency vs speed tradeof", OFFSET(usage), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, VE, "usage"},
>      { "good",            "Good quality",      0, AV_OPT_TYPE_CONST, {.i64 = 0 /* AOM_USAGE_GOOD_QUALITY */}, 0, 0, VE, "usage"},
>      { "realtime",        "Realtime encoding", 0, AV_OPT_TYPE_CONST, {.i64 = 1 /* AOM_USAGE_REALTIME */},     0, 0, VE, "usage"},
> +    { "tune",            "The metric that encoder tunes for. Automatically choosen by encoder by default", OFFSET(tune), AV_OPT_TYPE_INT, {.i64 = -1}, -1, AOM_TUNE_SSIM, VE, "tune"},

...that the encoder tunes...chosen by the encoder...

> +    { "psnr",            "PSNR as distortion metric",         0, AV_OPT_TYPE_CONST, {.i64 = AOM_TUNE_PSNR}, 0, 0, VE, "tune"},
> +    { "ssim",            "SSIM as distortion metric",         0, AV_OPT_TYPE_CONST, {.i64 = AOM_TUNE_SSIM}, 0, 0, VE, "tune"},

Like in the other table the extra descriptions may be redundant, they
could be NULL.