[FFmpeg-devel] [PATCH 3/4] lavfi/delogo: option show shouldn't affect band

[Stefano Sabatini]

On date Friday 2013-07-05 10:34:10 +0200, Jean Delvare encoded:
> Enabling option show (to see the borders of the logo area) forcibly
> sets the band option to 4. This is documented, but still surprising
> and confusing IMHO. Both options should be independent.

The subject and this paragraph are prescribing a behavior rather than
describing the change, and thus is not very clear what the patch is
about.

> Also document exactly where the rectangle is drawn.
> 
> Signed-off-by: Jean Delvare <khali at linux-fr.org>
> ---
>  doc/filters.texi        |    8 ++++++--
>  libavfilter/vf_delogo.c |    4 +---
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> --- ffmpeg.orig/libavfilter/vf_delogo.c	2013-07-05 09:10:16.425572220 +0200
> +++ ffmpeg/libavfilter/vf_delogo.c	2013-07-05 09:10:22.032572303 +0200
> @@ -195,10 +195,8 @@ static av_cold int init(AVFilterContext
>      CHECK_UNSET_OPT(w);
>      CHECK_UNSET_OPT(h);
>  
> -    if (s->band < 0 || s->show) {
> -        s->show = 1;
> +    if (s->band < 0)
>          s->band = 4;
> -    }

Looks good. This is technically a backward compatibiliy break but I
don't think people would care, bumping micro may be useful
nonetheless.

You could probably set default band value to 4, change the range to
0..MAX and remove this check altogether (this code was written when we
still lack proper option handling).

>      av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d show:%d\n",
>             s->x, s->y, s->w, s->h, s->band, s->show);
> --- ffmpeg.orig/doc/filters.texi	2013-07-05 09:01:05.040695940 +0200
> +++ ffmpeg/doc/filters.texi	2013-07-05 09:50:13.998651184 +0200
> @@ -2780,8 +2780,12 @@ Specify the thickness of the fuzzy edge
>  
>  @item show

>  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
> - at var{band} is set to 4. The default value is 0.
> +finding the right @var{x}, @var{y}, @var{w}, and @var{h} parameters.
> +The default value is 0.

> The rectangle is drawn on the outermost
> +pixels which will be (partly) replaced with interpolated values.
> +The values of the next pixels immediately outside this rectangle in
> +each direction will be used to compute the interpolated pixel values
> +inside the rectangle.

I'd prefer to put this change in a separate patch, since it made more
hard for me to understand what was the patch was about.

Note: I wouldn't be against removing the show option altogether, since
what it does can be better achieved by using a separate filter
(drawbox).
-- 
FFmpeg = Fanciful and Fostering Meaningless Portable Elfic Gymnast