[FFmpeg-devel] [PATCH] vf_delogo: allow to change the rectangle based on the time

Stefano Sabatini stefano.sabatini-lala at poste.it
Tue Aug 16 18:19:16 CEST 2011


On date Monday 2011-08-15 23:31:02 +0200, Nicolas George encoded:
> L'octidi 28 thermidor, an CCXIX, Stefano Sabatini a écrit :
> > ehm...
> 
> Sorry. Here it is.
> 
> Regards,
> 
> -- 
>   Nicolas George

> From 24a113bc12a764b872f4508ebec632cdd7b965ff Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Mon, 15 Aug 2011 15:10:57 +0200
> Subject: [PATCH] vf_delogo: allow to change the rectangle based on the time.
> 
> This is a port of MPlayer's commit 33488.
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  doc/filters.texi        |   20 +++++++
>  libavfilter/vf_delogo.c |  142 +++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 150 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 73e2ac8..ab1b3bb 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -457,6 +457,12 @@ When set to 1, a green rectangle is drawn on the screen to simplify
>  finding the right @var{x}, @var{y}, @var{w}, @var{h} parameters, and
>  @var{band} is set to 4. The default value is 0.
>  
> + at item file
> +Read timestamps and coordinates from @var{file}. @var{file} is a text file
> +with on each line a timestamp (in seconds and in ascending order) and the
> +x:y:w:h:band coordinates (band can be omitted), or just 0.
> +If this option is present, the other options are ignored.

Add a note regarding # lines.

> +
>  @end table
>  
>  Some examples follow.
> @@ -476,6 +482,20 @@ As the previous example, but use named options:
>  delogo=x=0:y=0:w=100:h=77:band=10
>  @end example
>  
> + at item
> +As the previous example, but only remove the logo from time 5s to time 10s
> +and from time 1800s to time 1805s:
> + at example
> +delogo=file=top_left_5s.txt
> + at end example
> + at code{top_left_5s.txt} contains:
> + at example
> +5    0:0:100:77:10
> +10   0
> +1800 0:0:100:77:10
> +1805 0
> + at end example
> +
>  @end itemize
>  
>  @section drawbox
> diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
> index f0852f8..6d09a04 100644
> --- a/libavfilter/vf_delogo.c
> +++ b/libavfilter/vf_delogo.c
> @@ -133,6 +133,13 @@ static void apply_delogo(uint8_t *dst, int dst_linesize,
>  typedef struct {
>      const AVClass *class;
>      int x, y, w, h, band, show;
> +    const char *file;

> +    struct timed_rectangle {
> +        int64_t ts;
> +        int x, y, w, h, b;
> +    } *timed_rect;
> +    int n_timed_rect;

nit: timed_rects, nb_timed_rects => clear corrispondency

> +    int cur_timed_rect;
>  }  DelogoContext;
>  
>  #define OFFSET(x) offsetof(DelogoContext, x)
> @@ -145,6 +152,7 @@ static const AVOption delogo_options[]= {
>      {"band", "set delogo area band size", OFFSET(band), FF_OPT_TYPE_INT, {.dbl= 4}, -1, INT_MAX },
>      {"t",    "set delogo area band size", OFFSET(band), FF_OPT_TYPE_INT, {.dbl= 4}, -1, INT_MAX },
>      {"show", "show delogo area",          OFFSET(show), FF_OPT_TYPE_INT, {.dbl= 0},  0, 1       },
> +    {"file", "read times and coordinates from a file", OFFSET(file), FF_OPT_TYPE_STRING, {.str= NULL}, 0, 0 },
>      {NULL},
>  };
>  
> @@ -172,6 +180,106 @@ static int query_formats(AVFilterContext *ctx)
>      return 0;
>  }
>  
> +static void fix_band(DelogoContext *delogo, int show)
> +{
> +    delogo->show = show || delogo->band < 0;
> +    if (delogo->show)
> +        delogo->band = 4;
> +    delogo->w += delogo->band*2;
> +    delogo->h += delogo->band*2;
> +    delogo->x -= delogo->band;
> +    delogo->y -= delogo->band;
> +}
> +
> +static void update_sub(DelogoContext *p, int64_t pts)
> +{
> +    int tr = p->cur_timed_rect;
> +    while (tr < p->n_timed_rect - 1 && pts >= p->timed_rect[tr + 1].ts)
> +        tr++;
> +    while (tr >= 0 && pts < p->timed_rect[tr].ts)
> +        tr--;
> +    if (tr == p->cur_timed_rect)
> +        return;
> +    p->cur_timed_rect = tr;
> +    if (tr >= 0) {
> +        p->x    = p->timed_rect[tr].x;
> +        p->y    = p->timed_rect[tr].y;
> +        p->w    = p->timed_rect[tr].w;
> +        p->h    = p->timed_rect[tr].h;
> +        p->band = p->timed_rect[tr].b;
> +    } else {
> +        p->x = p->y = p->w = p->h = p->band = 0;
> +    }
> +    fix_band(p, 0);
> +}
> +
> +static int load_timed_rectangles(AVFilterContext *ctx, DelogoContext *delogo)
> +{
> +    FILE *f;
> +    char line[2048];
> +    int lineno = 0, p;
> +    double ts, last_ts = 0;
> +    struct timed_rectangle *rect = NULL, *nr;
> +    int n_rect = 0, alloc_rect = 0;
> +
> +    f = fopen(delogo->file, "r");
> +    if (!f) {
> +        av_log(ctx, AV_LOG_ERROR, "unable to load %s: %s\n",
> +            delogo->file, strerror(errno));
> +        return AVERROR(errno);
> +    }
> +    while (fgets(line, sizeof(line), f)) {
> +        lineno++;
> +        if (*line == '#' || *line == '\n')
> +            continue;

> +        if (n_rect == alloc_rect) {

somehow confusing var names (what is alloc_rect? is a single rect, is
an array of rects, is a number of allocated rects?)

> +            if (alloc_rect > INT_MAX / 2 / (int)sizeof(*rect)) {
> +                av_log(ctx, AV_LOG_ERROR, "too many rectangles\n");

> +                errno = ENOMEM;
> +                goto load_error;

I'd prefer avoiding to twiddle with errno, better
err = AVERROR(ENOMEM);
goto load_error;

> +            }
> +            alloc_rect = alloc_rect ? 2 * alloc_rect : 256;
> +            nr = av_realloc(rect, alloc_rect * sizeof(*rect));
> +            if (!nr) {
> +                av_log(ctx, AV_LOG_ERROR, "out of memory\n");
> +                goto load_error;

don't know if realloc already set the error, but that's pretty opaque
so better to set it explicitely

> +            }
> +            rect = nr;
> +        }
> +        nr = rect + n_rect;
> +        memset(nr, 0, sizeof(*nr));

> +        p = sscanf(line, "%lf %d:%d:%d:%d:%d",
> +                   &ts, &nr->x, &nr->y, &nr->w, &nr->h, &nr->b);

nit++: s/p/n/ (p is usually used for temporary pointers, n for integer
numbers)

> +        if ((p == 2 && !nr->x) || p == 5 || p == 6) {

please specify in the docs that band is optional

> +            if (ts <= last_ts)
> +                av_log(ctx, AV_LOG_ERROR, "%s:%d: wrong time\n",
> +                       delogo->file, lineno);

More explicit: "invalid time specification, ts %f is <= the ts %f specified in the previous line\n"

> +            nr->ts = AV_TIME_BASE * ts + 0.5;

why AV_TIME_BASE? Why don't you directly store the time as a double?

> +            n_rect++;
> +        } else {
> +            av_log(ctx, AV_LOG_ERROR, "%s:%d: syntax error\n",
> +                   delogo->file, lineno);
> +        }
> +    }
> +    fclose(f);

> +    if (!n_rect) {
> +        av_log(ctx, AV_LOG_ERROR, "%s: no rectangles found\n", delogo->file);
> +        av_free(rect);
> +        return -1;

AVERROR(EINVAL)

> +    }

> +    nr = av_realloc(rect, n_rect * sizeof(*rect));
> +    if (nr)
> +        rect = nr;
> +    delogo->timed_rect   = rect;

why not to use directly delogo_timed_rect?

> +    delogo->n_timed_rect = n_rect;

same, what about avoiding the n_rect temporary?

> +    return 0;
> +
> +load_error:
> +    av_free(rect);
> +    fclose(f);
> +    return AVERROR(errno);
> +}
> +
[...]
-- 
FFmpeg = Fundamental & Furious Monstrous Power Epic Gadget


More information about the ffmpeg-devel mailing list