[FFmpeg-devel] [PATCH] lavfi/color: use AVOptions

[Paul B Mahol]

On 6/19/12, Stefano Sabatini <stefasab at gmail.com> wrote:
> On date Tuesday 2012-06-19 16:07:45 +0000, Paul B Mahol encoded:
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>

[...]

>> +        if (av_parse_video_size(&color->w, &color->h, frame_size) < 0) {
>> +            av_log(ctx, AV_LOG_ERROR, "Invalid frame size: %s\n",
>> frame_size);
>> +            return AVERROR(EINVAL);
>> +        }
>> +        if (av_parse_video_rate(&frame_rate_q, frame_rate) < 0 ||
>> +                frame_rate_q.den <= 0 || frame_rate_q.num <= 0) {
>> +            av_log(ctx, AV_LOG_ERROR, "Invalid frame rate: %s\n",
>> frame_rate);
>> +            return AVERROR(EINVAL);
>> +        }
>> +        if (av_parse_color(color->color_rgba, color_string, -1, ctx) <
>> 0)
>> +            return AVERROR(EINVAL);
>>      }
>
> I see code duplication but removing it seems not worth the effort,
> hopefully we'll get rid of it when we'll drop the old syntax (next
> bump?).

Just lets not forget to remove it.

[...]
>
>> +
>>      color->time_base.num = frame_rate_q.den;
>>      color->time_base.den = frame_rate_q.num;
>>
>> -    if ((ret = av_parse_color(color->color_rgba, color_string, -1, ctx))
>> < 0)
>> -        return ret;
>> -
>>      return 0;
>
>> +fail:
>
> This is reached even in case of non-fail, I believe "end" or "exit" is
> more appropriate/less confusing.
>
> [...]
>
> Looks fine otherwise if tested, thanks.

Tested, could not find anything obviously wrong. Applied.