[MPlayer-dev-eng] [PATCH] vf_delogo: variable rectangle

[Reimar Döffinger]

> +    char *file;

const char *
is probably better.

>  
> +static void fix_band(struct vf_priv_s *p)

doxy-documentation for new functions would be nice.

> +static void report_info(struct vf_priv_s *p)
> +{
> +    mp_msg(MSGT_VFILTER, MSGL_V, "delogo: %d x %d, %d x %d, band = %d\n",
> +           p->xoff, p->yoff, p->lw, p->lh, p->band);
> +}

Seems to me liek you could just do this in fix_band

> +    while (tr < p->n_timed_rect - 1 && ipts >= p->timed_rect[tr + 1].ts)
> +        tr++;
> +    while (tr >= 0 && ipts < p->timed_rect[tr].ts)
> +        tr--;

Are you sure you want to allow it to go backwards?

> +    p->xoff = p->yoff = p->lw = p->lh = p->band = p->show = 0;
> +    if (tr >= 0) {
[...]
> +    } else {
> +        p->xoff = p->yoff = p->lw = p->lh = p->band = 0;
> +    }

This looks a bit double...

> +    while (fgets(line, sizeof(line), f)) {
> +        lineno++;
> +        if (*line == '#' || *line == '\n')
> +            continue;
> +        if (n_rect == alloc_rect) {
> +            alloc_rect = alloc_rect ? 2 * alloc_rect : 256;
> +            rect = realloc(rect, alloc_rect * sizeof(*rect));

Potential integer overflow.

> +            if (!rect)
> +                abort();

Abort sure isn't nice. Haven't looked what if any the alternatives are.

> +        }
> +        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);

I'd expect this whole part can be simplified by fscanf, in the worst case by also
using getc/ungetc

> +    rect = realloc(rect, n_rect * sizeof(*rect));

That's just pointless just to save a few bytes IMO.