[FFmpeg-devel] [PATCH] lavfi/color: use AVOptions
Stefano Sabatini
stefasab at gmail.com
Tue Jun 19 18:42:58 CEST 2012
On date Tuesday 2012-06-19 16:07:45 +0000, Paul B Mahol encoded:
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
> doc/filters.texi | 12 +++---
> libavfilter/vsrc_color.c | 83 +++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 73 insertions(+), 22 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 50cd72e..fa953ab 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -3419,24 +3419,24 @@ cellauto=p='@@@@ @@ @@@@':s=100x400:full=0:rule=18
>
> Provide an uniformly colored input.
>
> -It accepts the following parameters:
> - at var{color}:@var{frame_size}:@var{frame_rate}
> +This source accepts list of options in the form of
> + at var{key}=@var{value} pairs separated by ":".
Please mention the old deprecated syntax:
|This source accepts a list of options in the form of
|@var{key}=@var{value} pairs separated by ":".
|
|Alternatively, it accepts a string in the form
|@var{color}:@var{size}:@var{rate}, but this syntax is
|deprecated.
>
> Follows the description of the accepted parameters.
>
> @table @option
>
> - at item color
> + at item color, c
> Specify the color of the source. It can be the name of a color (case
> insensitive match) or a 0xRRGGBB[AA] sequence, possibly followed by an
> alpha specifier. The default value is "black".
>
> - at item frame_size
> + at item size, s
> Specify the size of the sourced video, it may be a string of the form
> @var{width}x at var{height}, or the name of a size abbreviation. The
> default value is "320x240".
>
> - at item frame_rate
> + at item rate, r
> Specify the frame rate of the sourced video, as the number of frames
> generated per second. It has to be a string in the format
> @var{frame_rate_num}/@var{frame_rate_den}, an integer number, a float
> @@ -3451,7 +3451,7 @@ frames per second, which will be overlayed over the source connected
> to the pad with identifier "in".
> @example
> -"color=red@@0.2:qcif:10 [color]; [in][color] overlay [out]"
> +"color=c=red@@0.2:qcif:10 [color]; [in][color] overlay [out]"
still borken?
> @end example
>
> @section movie
> diff --git a/libavfilter/vsrc_color.c b/libavfilter/vsrc_color.c
> index 112c27c..51c959b 100644
> --- a/libavfilter/vsrc_color.c
> +++ b/libavfilter/vsrc_color.c
> @@ -31,11 +31,15 @@
> #include "libavutil/colorspace.h"
> #include "libavutil/imgutils.h"
> #include "libavutil/mathematics.h"
> +#include "libavutil/opt.h"
> #include "libavutil/parseutils.h"
> #include "drawutils.h"
>
> typedef struct {
> + const AVClass *class;
> int w, h;
> + char *rate_str;
> + char *color_str;
> uint8_t color_rgba[4];
> AVRational time_base;
> uint64_t pts;
> @@ -43,35 +47,82 @@ typedef struct {
> FFDrawColor color;
> } ColorContext;
>
> +#define OFFSET(x) offsetof(ColorContext, x)
> +
> +static const AVOption color_options[]= {
> + { "size", "set frame size", OFFSET(w), AV_OPT_TYPE_IMAGE_SIZE, {.str = "320x240"}, 0, 0 },
> + { "s", "set frame size", OFFSET(w), AV_OPT_TYPE_IMAGE_SIZE, {.str = "320x240"}, 0, 0 },
> + { "rate", "set frame rate", OFFSET(rate_str), AV_OPT_TYPE_STRING, {.str = "25"}, 0, 0 },
> + { "r", "set frame rate", OFFSET(rate_str), AV_OPT_TYPE_STRING, {.str = "25"}, 0, 0 },
> + { "color", "set color", OFFSET(color_str), AV_OPT_TYPE_STRING, {.str = "black"}, CHAR_MIN, CHAR_MAX },
> + { "c", "set color", OFFSET(color_str), AV_OPT_TYPE_STRING, {.str = "black"}, CHAR_MIN, CHAR_MAX },
> + { NULL },
> +};
> +
> +static const AVClass color_class = {
> + .class_name = "color",
> + .item_name = av_default_item_name,
> + .option = color_options,
> + .version = LIBAVUTIL_VERSION_INT,
> + .category = AV_CLASS_CATEGORY_FILTER,
> +};
> +
> static av_cold int color_init(AVFilterContext *ctx, const char *args, void *opaque)
> {
> ColorContext *color = ctx->priv;
> - char color_string[128] = "black";
> - char frame_size [128] = "320x240";
> - char frame_rate [128] = "25";
> AVRational frame_rate_q;
> + char color_string[128] = "black";
> + char frame_rate[128] = "25";
> + char frame_size[128] = "320x240";
> + char *colon = 0, *equal = 0;
> int ret;
>
> - if (args)
> - sscanf(args, "%127[^:]:%127[^:]:%127s", color_string, frame_size, frame_rate);
> + color->class = &color_class;
>
> - 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 (args) {
> + colon = strrchr(args, ':');
> + equal = strrchr(args, '=');
> }
>
> - 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 (!args || (equal && (!colon || equal < colon))) {
> + av_opt_set_defaults(color);
> + if ((ret = av_set_options_string(color, args, "=", ":")) < 0) {
> + av_log(ctx, AV_LOG_ERROR, "Error parsing options string: '%s'\n", args);
> + goto fail;
> + }
> + if (av_parse_video_rate(&frame_rate_q, color->rate_str) < 0 ||
> + frame_rate_q.den <= 0 || frame_rate_q.num <= 0) {
> + av_log(ctx, AV_LOG_ERROR, "Invalid frame rate: %s\n", color->rate_str);
> + ret = AVERROR(EINVAL);
> + goto fail;
> + }
> + if (av_parse_color(color->color_rgba, color->color_str, -1, ctx) < 0) {
> + ret = AVERROR(EINVAL);
> + goto fail;
> + }
> + } else {
> + av_log(ctx, AV_LOG_WARNING, "Flat options syntax is deprecated, use key=value pairs.\n");
> + sscanf(args, "%127[^:]:%127[^:]:%127s", color_string, frame_size, frame_rate);
> + 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?).
> +
> 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.
--
FFmpeg = Foolish Fundamental Mysterious Patchable Ecumenical Geisha
More information about the ffmpeg-devel
mailing list