[FFmpeg-devel] [PATCH 2/5] lavfi/color: switch to AV_OPT_TYPE_COLOR

[Paul B Mahol]

On 5/16/13, Stefano Sabatini <stefasab at gmail.com> wrote:
> On date Monday 2013-05-13 15:11:37 +0000, Paul B Mahol encoded:
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> ---
>>  libavfilter/vsrc_testsrc.c | 26 ++------------------------
>>  1 file changed, 2 insertions(+), 24 deletions(-)
>>
>> diff --git a/libavfilter/vsrc_testsrc.c b/libavfilter/vsrc_testsrc.c
>> index dc8984a..8c4c51a 100644
>> --- a/libavfilter/vsrc_testsrc.c
>> +++ b/libavfilter/vsrc_testsrc.c
>> @@ -63,7 +63,6 @@ typedef struct {
>>      void (* fill_picture_fn)(AVFilterContext *ctx, AVFrame *frame);
>>
>>      /* only used by color */
>> -    char *color_str;
>>      FFDrawContext draw;
>>      FFDrawColor color;
>>      uint8_t color_rgba[4];
>> @@ -87,8 +86,8 @@ typedef struct {
>>
>>  static const AVOption color_options[] = {
>>      /* only used by color */
>> -    { "color", "set color", OFFSET(color_str), AV_OPT_TYPE_STRING, {.str
>> = "black"}, CHAR_MIN, CHAR_MAX, FLAGS },
>> -    { "c",     "set color", OFFSET(color_str), AV_OPT_TYPE_STRING, {.str
>> = "black"}, CHAR_MIN, CHAR_MAX, FLAGS },
>> +    { "color", "set color", OFFSET(color_rgba), AV_OPT_TYPE_COLOR, {.str
>> = "black"}, CHAR_MIN, CHAR_MAX, FLAGS },
>> +    { "c",     "set color", OFFSET(color_rgba), AV_OPT_TYPE_COLOR, {.str
>> = "black"}, CHAR_MIN, CHAR_MAX, FLAGS },
>>
>>      COMMON_OPTIONS
>>      { NULL },
>> @@ -106,7 +105,6 @@ static const AVOption options[] = {
>>  static av_cold int init(AVFilterContext *ctx)
>>  {
>>      TestSourceContext *test = ctx->priv;
>> -    int ret = 0;
>>
>>      if (test->nb_decimals && strcmp(ctx->filter->name, "testsrc")) {
>>          av_log(ctx, AV_LOG_WARNING,
>> @@ -114,18 +112,6 @@ static av_cold int init(AVFilterContext *ctx)
>>                 ctx->filter->name);
>>      }
>>
>
>> -    if (test->color_str) {
>> -        if (!strcmp(ctx->filter->name, "color")) {
>> -            ret = av_parse_color(test->color_rgba, test->color_str, -1,
>> ctx);
>> -            if (ret < 0)
>> -                return ret;
>> -        } else {
>> -            av_log(ctx, AV_LOG_WARNING,
>> -                   "Option 'color' is ignored with source '%s'\n",
>> -                   ctx->filter->name);
>> -        }
>> -    }
>
> Uhm not very happy about now there is no warning in case color is set
> on a source != filter, and duplicating the options or keeping this
> code might be overkill...

Please elaborate.

Also I do not see how that warning is much helpful.
Options could be split if you insist on this.

>
>> -
>>      test->time_base = av_inv_q(test->frame_rate);
>>      test->nb_frame = 0;
>>      test->pts = 0;
>> @@ -241,8 +227,6 @@ static int color_config_props(AVFilterLink *inlink)
>>      if ((ret = config_props(inlink)) < 0)
>>          return ret;
>>
>> -    av_log(ctx, AV_LOG_VERBOSE, "color:0x%02x%02x%02x%02x\n",
>> -           test->color_rgba[0], test->color_rgba[1], test->color_rgba[2],
>> test->color_rgba[3]);
>>      return 0;
>>  }
>>
>> @@ -253,17 +237,11 @@ static int color_process_command(AVFilterContext
>> *ctx, const char *cmd, const ch
>>      int ret;
>>
>>      if (!strcmp(cmd, "color") || !strcmp(cmd, "c")) {
>> -        char *color_str;
>>          uint8_t color_rgba[4];
>>
>>          ret = av_parse_color(color_rgba, args, -1, ctx);
>>          if (ret < 0)
>>              return ret;
>
>> -        color_str = av_strdup(args);
>> -        if (!color_str)
>> -            return AVERROR(ENOMEM);
>> -        av_free(test->color_str);
>> -        test->color_str = color_str;
>
> What if you set the color in the context with av_opt_set()?

What?

>
>>          memcpy(test->color_rgba, color_rgba, sizeof(color_rgba));
>>          ff_draw_color(&test->draw, &test->color, test->color_rgba);
>
> [...]
> --
> FFmpeg = Friendly and Friendly Merciless Political Erudite Guru
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>