diff mbox series

[FFmpeg-devel] avdevice/caca: Allow to list multiple dither option types at once

Message ID AS8P250MB07449A8D2B956637AF02262B8F402@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel] avdevice/caca: Allow to list multiple dither option types at once | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Feb. 4, 2024, 5:36 p.m. UTC
This can be achieved by using AV_OPT_TYPE_FLAGS instead of
AV_OPT_TYPE_STRING. It also avoids strcmp() when accessing
the option.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavdevice/caca.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Comments

Stefano Sabatini Feb. 4, 2024, 11:47 p.m. UTC | #1
On date Sunday 2024-02-04 18:36:45 +0100, Andreas Rheinhardt wrote:
> This can be achieved by using AV_OPT_TYPE_FLAGS instead of
> AV_OPT_TYPE_STRING. It also avoids strcmp() when accessing
> the option.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavdevice/caca.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/libavdevice/caca.c b/libavdevice/caca.c
> index 6af1649137..c0e09cf6f7 100644
> --- a/libavdevice/caca.c
> +++ b/libavdevice/caca.c
> @@ -24,6 +24,13 @@
>  #include "libavformat/mux.h"
>  #include "avdevice.h"
>  

> +enum {
> +    SHOW_ALGORITHMS  = 1 << 0,
> +    SHOW_ANTIALIASES = 1 << 1,
> +    SHOW_CHARSETS    = 1 << 2,
> +    SHOW_COLORS      = 1 << 3,
> +};

nit: LIST_ for consistency?

> +
>  typedef struct CACAContext {
>      AVClass         *class;
>      AVFormatContext *ctx;
> @@ -38,7 +45,7 @@ typedef struct CACAContext {
>      char            *charset, *color;
>      char            *driver;
>  
> -    char            *list_dither;
> +    int             list_dither;
>      int             list_drivers;
>  } CACAContext;
>  
> @@ -99,21 +106,14 @@ static int caca_write_header(AVFormatContext *s)
>          return AVERROR_EXIT;
>      }
>      if (c->list_dither) {
> -        if (!strcmp(c->list_dither, "colors")) {
> +        if (c->list_dither & SHOW_COLORS)
>              list_dither_color(c);
> -        } else if (!strcmp(c->list_dither, "charsets")) {
> +        if (c->list_dither & SHOW_CHARSETS)
>              list_dither_charset(c);
> -        } else if (!strcmp(c->list_dither, "algorithms")) {
> +        if (c->list_dither & SHOW_ALGORITHMS)
>              list_dither_algorithm(c);
> -        } else if (!strcmp(c->list_dither, "antialiases")) {
> +        if (c->list_dither & SHOW_ANTIALIASES)
>              list_dither_antialias(c);
> -        } else {
> -            av_log(s, AV_LOG_ERROR,
> -                   "Invalid argument '%s', for 'list_dither' option\n"
> -                   "Argument must be one of 'algorithms, 'antialiases', 'charsets', 'colors'\n",
> -                   c->list_dither);
> -            return AVERROR(EINVAL);
> -        }
>          return AVERROR_EXIT;
>      }
>  
> @@ -205,11 +205,11 @@ static const AVOption options[] = {
>      { "charset",      "set charset used to render output", OFFSET(charset), AV_OPT_TYPE_STRING, {.str = "default" }, 0, 0, ENC },
>      { "color",        "set color used to render output",   OFFSET(color),   AV_OPT_TYPE_STRING, {.str = "default" }, 0, 0, ENC },
>      { "list_drivers", "list available drivers",  OFFSET(list_drivers), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, ENC },
> -    { "list_dither", "list available dither options", OFFSET(list_dither), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1, ENC, "list_dither" },
> -    { "algorithms",   NULL, 0, AV_OPT_TYPE_CONST, {.str = "algorithms"}, 0, 0, ENC, "list_dither" },
> -    { "antialiases",  NULL, 0, AV_OPT_TYPE_CONST, {.str = "antialiases"},0, 0, ENC, "list_dither" },
> -    { "charsets",     NULL, 0, AV_OPT_TYPE_CONST, {.str = "charsets"},   0, 0, ENC, "list_dither" },
> -    { "colors",       NULL, 0, AV_OPT_TYPE_CONST, {.str = "colors"},     0, 0, ENC, "list_dither" },
> +    { "list_dither", "list available dither options", OFFSET(list_dither), AV_OPT_TYPE_FLAGS, { .i64 = 0 }, 0, INT_MAX, ENC, "list_dither" },
> +    { "algorithms",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_ALGORITHMS },  0, 0, ENC, "list_dither" },
> +    { "antialiases",  NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_ANTIALIASES }, 0, 0, ENC, "list_dither" },
> +    { "charsets",     NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_CHARSETS },    0, 0, ENC, "list_dither" },
> +    { "colors",       NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_COLORS },      0, 0, ENC, "list_dither" },
>      { NULL },
>  };

LGTM otherwise, thanks.
Andreas Rheinhardt Feb. 5, 2024, 11:54 a.m. UTC | #2
Stefano Sabatini:
> On date Sunday 2024-02-04 18:36:45 +0100, Andreas Rheinhardt wrote:
>> This can be achieved by using AV_OPT_TYPE_FLAGS instead of
>> AV_OPT_TYPE_STRING. It also avoids strcmp() when accessing
>> the option.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavdevice/caca.c | 34 +++++++++++++++++-----------------
>>  1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/libavdevice/caca.c b/libavdevice/caca.c
>> index 6af1649137..c0e09cf6f7 100644
>> --- a/libavdevice/caca.c
>> +++ b/libavdevice/caca.c
>> @@ -24,6 +24,13 @@
>>  #include "libavformat/mux.h"
>>  #include "avdevice.h"
>>  
> 
>> +enum {
>> +    SHOW_ALGORITHMS  = 1 << 0,
>> +    SHOW_ANTIALIASES = 1 << 1,
>> +    SHOW_CHARSETS    = 1 << 2,
>> +    SHOW_COLORS      = 1 << 3,
>> +};
> 
> nit: LIST_ for consistency?
> 

Will make the change and apply.

>> +
>>  typedef struct CACAContext {
>>      AVClass         *class;
>>      AVFormatContext *ctx;
>> @@ -38,7 +45,7 @@ typedef struct CACAContext {
>>      char            *charset, *color;
>>      char            *driver;
>>  
>> -    char            *list_dither;
>> +    int             list_dither;
>>      int             list_drivers;
>>  } CACAContext;
>>  
>> @@ -99,21 +106,14 @@ static int caca_write_header(AVFormatContext *s)
>>          return AVERROR_EXIT;
>>      }
>>      if (c->list_dither) {
>> -        if (!strcmp(c->list_dither, "colors")) {
>> +        if (c->list_dither & SHOW_COLORS)
>>              list_dither_color(c);
>> -        } else if (!strcmp(c->list_dither, "charsets")) {
>> +        if (c->list_dither & SHOW_CHARSETS)
>>              list_dither_charset(c);
>> -        } else if (!strcmp(c->list_dither, "algorithms")) {
>> +        if (c->list_dither & SHOW_ALGORITHMS)
>>              list_dither_algorithm(c);
>> -        } else if (!strcmp(c->list_dither, "antialiases")) {
>> +        if (c->list_dither & SHOW_ANTIALIASES)
>>              list_dither_antialias(c);
>> -        } else {
>> -            av_log(s, AV_LOG_ERROR,
>> -                   "Invalid argument '%s', for 'list_dither' option\n"
>> -                   "Argument must be one of 'algorithms, 'antialiases', 'charsets', 'colors'\n",
>> -                   c->list_dither);
>> -            return AVERROR(EINVAL);
>> -        }
>>          return AVERROR_EXIT;
>>      }
>>  
>> @@ -205,11 +205,11 @@ static const AVOption options[] = {
>>      { "charset",      "set charset used to render output", OFFSET(charset), AV_OPT_TYPE_STRING, {.str = "default" }, 0, 0, ENC },
>>      { "color",        "set color used to render output",   OFFSET(color),   AV_OPT_TYPE_STRING, {.str = "default" }, 0, 0, ENC },
>>      { "list_drivers", "list available drivers",  OFFSET(list_drivers), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, ENC },
>> -    { "list_dither", "list available dither options", OFFSET(list_dither), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1, ENC, "list_dither" },
>> -    { "algorithms",   NULL, 0, AV_OPT_TYPE_CONST, {.str = "algorithms"}, 0, 0, ENC, "list_dither" },
>> -    { "antialiases",  NULL, 0, AV_OPT_TYPE_CONST, {.str = "antialiases"},0, 0, ENC, "list_dither" },
>> -    { "charsets",     NULL, 0, AV_OPT_TYPE_CONST, {.str = "charsets"},   0, 0, ENC, "list_dither" },
>> -    { "colors",       NULL, 0, AV_OPT_TYPE_CONST, {.str = "colors"},     0, 0, ENC, "list_dither" },
>> +    { "list_dither", "list available dither options", OFFSET(list_dither), AV_OPT_TYPE_FLAGS, { .i64 = 0 }, 0, INT_MAX, ENC, "list_dither" },
>> +    { "algorithms",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_ALGORITHMS },  0, 0, ENC, "list_dither" },
>> +    { "antialiases",  NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_ANTIALIASES }, 0, 0, ENC, "list_dither" },
>> +    { "charsets",     NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_CHARSETS },    0, 0, ENC, "list_dither" },
>> +    { "colors",       NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_COLORS },      0, 0, ENC, "list_dither" },
>>      { NULL },
>>  };
> 
> LGTM otherwise, thanks.
diff mbox series

Patch

diff --git a/libavdevice/caca.c b/libavdevice/caca.c
index 6af1649137..c0e09cf6f7 100644
--- a/libavdevice/caca.c
+++ b/libavdevice/caca.c
@@ -24,6 +24,13 @@ 
 #include "libavformat/mux.h"
 #include "avdevice.h"
 
+enum {
+    SHOW_ALGORITHMS  = 1 << 0,
+    SHOW_ANTIALIASES = 1 << 1,
+    SHOW_CHARSETS    = 1 << 2,
+    SHOW_COLORS      = 1 << 3,
+};
+
 typedef struct CACAContext {
     AVClass         *class;
     AVFormatContext *ctx;
@@ -38,7 +45,7 @@  typedef struct CACAContext {
     char            *charset, *color;
     char            *driver;
 
-    char            *list_dither;
+    int             list_dither;
     int             list_drivers;
 } CACAContext;
 
@@ -99,21 +106,14 @@  static int caca_write_header(AVFormatContext *s)
         return AVERROR_EXIT;
     }
     if (c->list_dither) {
-        if (!strcmp(c->list_dither, "colors")) {
+        if (c->list_dither & SHOW_COLORS)
             list_dither_color(c);
-        } else if (!strcmp(c->list_dither, "charsets")) {
+        if (c->list_dither & SHOW_CHARSETS)
             list_dither_charset(c);
-        } else if (!strcmp(c->list_dither, "algorithms")) {
+        if (c->list_dither & SHOW_ALGORITHMS)
             list_dither_algorithm(c);
-        } else if (!strcmp(c->list_dither, "antialiases")) {
+        if (c->list_dither & SHOW_ANTIALIASES)
             list_dither_antialias(c);
-        } else {
-            av_log(s, AV_LOG_ERROR,
-                   "Invalid argument '%s', for 'list_dither' option\n"
-                   "Argument must be one of 'algorithms, 'antialiases', 'charsets', 'colors'\n",
-                   c->list_dither);
-            return AVERROR(EINVAL);
-        }
         return AVERROR_EXIT;
     }
 
@@ -205,11 +205,11 @@  static const AVOption options[] = {
     { "charset",      "set charset used to render output", OFFSET(charset), AV_OPT_TYPE_STRING, {.str = "default" }, 0, 0, ENC },
     { "color",        "set color used to render output",   OFFSET(color),   AV_OPT_TYPE_STRING, {.str = "default" }, 0, 0, ENC },
     { "list_drivers", "list available drivers",  OFFSET(list_drivers), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, ENC },
-    { "list_dither", "list available dither options", OFFSET(list_dither), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1, ENC, "list_dither" },
-    { "algorithms",   NULL, 0, AV_OPT_TYPE_CONST, {.str = "algorithms"}, 0, 0, ENC, "list_dither" },
-    { "antialiases",  NULL, 0, AV_OPT_TYPE_CONST, {.str = "antialiases"},0, 0, ENC, "list_dither" },
-    { "charsets",     NULL, 0, AV_OPT_TYPE_CONST, {.str = "charsets"},   0, 0, ENC, "list_dither" },
-    { "colors",       NULL, 0, AV_OPT_TYPE_CONST, {.str = "colors"},     0, 0, ENC, "list_dither" },
+    { "list_dither", "list available dither options", OFFSET(list_dither), AV_OPT_TYPE_FLAGS, { .i64 = 0 }, 0, INT_MAX, ENC, "list_dither" },
+    { "algorithms",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_ALGORITHMS },  0, 0, ENC, "list_dither" },
+    { "antialiases",  NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_ANTIALIASES }, 0, 0, ENC, "list_dither" },
+    { "charsets",     NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_CHARSETS },    0, 0, ENC, "list_dither" },
+    { "colors",       NULL, 0, AV_OPT_TYPE_CONST, {.i64 = SHOW_COLORS },      0, 0, ENC, "list_dither" },
     { NULL },
 };