[FFmpeg-devel,V1,3/4] lavc/libxavs2: Rename option speed_level to preset

Submitted by Jun Zhao on Dec. 30, 2018, 8:39 a.m.

Details

Message ID 1546159173-27597-4-git-send-email-mypopydev@gmail.com
State New
Headers show

Commit Message

Jun Zhao Dec. 30, 2018, 8:39 a.m.
preset used in more in line with similar encoders.

Signed-off-by: Jun Zhao <mypopydev@gmail.com>
---
 libavcodec/libxavs2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Mark Thompson Dec. 30, 2018, 6:06 p.m.
On 30/12/2018 08:39, Jun Zhao wrote:
> preset used in more in line with similar encoders.
> 
> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> ---
>  libavcodec/libxavs2.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/libavcodec/libxavs2.c b/libavcodec/libxavs2.c
> index 41c0b63..2ade923 100644
> --- a/libavcodec/libxavs2.c
> +++ b/libavcodec/libxavs2.c
> @@ -256,7 +256,7 @@ static const AVOption options[] = {
>      { "qp"              ,   "Quantization parameter"  ,                 OFFSET(qp)              , AV_OPT_TYPE_INT, {.i64 = 34 },  1,      63,  VE },
>      { "max_qp"          ,   "max qp for rate control" ,                 OFFSET(max_qp)          , AV_OPT_TYPE_INT, {.i64 = 55 },  0,      63,  VE },
>      { "min_qp"          ,   "min qp for rate control" ,                 OFFSET(min_qp)          , AV_OPT_TYPE_INT, {.i64 = 20 },  0,      63,  VE },
> -    { "speed_level"     ,   "Speed level, higher is better but slower", OFFSET(preset_level)    , AV_OPT_TYPE_INT, {.i64 =  0 },  0,       9,  VE },
> +    { "preset"     ,        "Set the encoding preset, higher is better but slower", OFFSET(preset_level)    , AV_OPT_TYPE_INT, {.i64 =  0 },  0,       9,  VE },
>      { "log_level"       ,   "log level: -1: none, 0: error, 1: warning, 2: info, 3: debug", OFFSET(log_level)    , AV_OPT_TYPE_INT, {.i64 =  0 },  -1,       3,  VE },
>      { "xavs2-params"    ,   "set the xavs2 configuration using a :-separated list of key=value parameters", OFFSET(xavs2_opts), AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
>      { NULL },
> 

Seems unhelpful?  The explicit "speed_level" option is much clearer about what it does.

The name "preset" comes from it generally being a combination of individual options which together invoke some effect appropriate to the name.  E.g. in libwebp each of the presets configures the encoder appropriately for various different named styles of input (such as "photo" or "drawing"), which in libx264 each contains a set of options which combine to change the speed of the encoder in a way matching the named level (such as "slow" or "veryfast").

Given that, it seems nonsensical to set numbered presets - if you set it to 3, how would the encoder act threely?

(Also note that this named option was in the 4.1 release, so you may need to keep the old option with a deprecation period, etc.)

- Mark

Patch hide | download patch | download mbox

diff --git a/libavcodec/libxavs2.c b/libavcodec/libxavs2.c
index 41c0b63..2ade923 100644
--- a/libavcodec/libxavs2.c
+++ b/libavcodec/libxavs2.c
@@ -256,7 +256,7 @@  static const AVOption options[] = {
     { "qp"              ,   "Quantization parameter"  ,                 OFFSET(qp)              , AV_OPT_TYPE_INT, {.i64 = 34 },  1,      63,  VE },
     { "max_qp"          ,   "max qp for rate control" ,                 OFFSET(max_qp)          , AV_OPT_TYPE_INT, {.i64 = 55 },  0,      63,  VE },
     { "min_qp"          ,   "min qp for rate control" ,                 OFFSET(min_qp)          , AV_OPT_TYPE_INT, {.i64 = 20 },  0,      63,  VE },
-    { "speed_level"     ,   "Speed level, higher is better but slower", OFFSET(preset_level)    , AV_OPT_TYPE_INT, {.i64 =  0 },  0,       9,  VE },
+    { "preset"     ,        "Set the encoding preset, higher is better but slower", OFFSET(preset_level)    , AV_OPT_TYPE_INT, {.i64 =  0 },  0,       9,  VE },
     { "log_level"       ,   "log level: -1: none, 0: error, 1: warning, 2: info, 3: debug", OFFSET(log_level)    , AV_OPT_TYPE_INT, {.i64 =  0 },  -1,       3,  VE },
     { "xavs2-params"    ,   "set the xavs2 configuration using a :-separated list of key=value parameters", OFFSET(xavs2_opts), AV_OPT_TYPE_STRING, { 0 }, 0, 0, VE },
     { NULL },