[FFmpeg-devel] [PATCH] avfilter/delogo: Set default band to 1

Stefano Sabatini stefasab at gmail.com
Wed Oct 7 14:19:58 CEST 2015


On date Wednesday 2015-10-07 13:57:46 +0200, Jean Delvare encoded:
> Hi Stefano,
> 
> On Wed, 7 Oct 2015 11:21:45 +0200, Stefano Sabatini wrote:
> > On date Wednesday 2015-10-07 08:54:59 +0200, Jean Delvare encoded:
> > > The original interpolation algorithm behaved poorly on the borders and
> > > did not even guarantee continuity at the borders. For this reason, a
> > > second interpolation/blending pass was required on the borders to make
> > > them seamless.
> > > 
> > > However, since the interpolation algorithm was improved in June 2013,
> > > the border issues no longer exist. The new algorithm does guarantee
> > > continuity at the borders, making the second pass useless. A larger
> > > band always increases the cumulated interpolation error. In most cases
> > > it also increases the average interpolation error, even though the
> > > samples in the band are only partially interpolated.
> > > 
> > > For this reason I would like to get rid of the "band" parameter. As a
> > > first step, let's change its default value from 4 to 1 and document it
> > > as deprecated.
> > > 
> > > I have benchmarked this change on a combination of input sources and
> > > realistic logo areas. Lowering the band value from 4 to 1 resulted in
> > > 8 to 39 % less interpolation error per frame (or 1 to 34 % less
> > > interpolation error per luma sample.)
> > > 
> > > Signed-off-by: Jean Delvare <jdelvare at suse.de>
> > > ---
> > >  doc/filters.texi        |    4 +++-
> > >  libavfilter/vf_delogo.c |   11 +++++++++--
> > >  2 files changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > --- ffmpeg.orig/doc/filters.texi	2015-10-02 11:41:24.035943770 +0200
> > > +++ ffmpeg/doc/filters.texi	2015-10-02 12:02:08.231484828 +0200
> > > @@ -4590,7 +4590,9 @@ specified.
> > >  
> > >  @item band, t
> > >  Specify the thickness of the fuzzy edge of the rectangle (added to
> > > - at var{w} and @var{h}). The default value is 4.
> > > + at var{w} and @var{h}). The default value is 1. This option is
> > > +deprecated, setting higher values should no longer be necessary and
> > > +is not recommended.
> > >  
> > >  @item show
> > >  When set to 1, a green rectangle is drawn on the screen to simplify
> > > --- ffmpeg.orig/libavfilter/vf_delogo.c	2015-10-02 11:41:24.035943770 +0200
> > > +++ ffmpeg/libavfilter/vf_delogo.c	2015-10-07 08:45:39.037203764 +0200
> > > @@ -165,8 +165,9 @@ static const AVOption delogo_options[]=
> > >      { "y",    "set logo y position",       OFFSET(y),    AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > >      { "w",    "set logo width",            OFFSET(w),    AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > >      { "h",    "set logo height",           OFFSET(h),    AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, FLAGS },
> > > -    { "band", "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { .i64 =  4 },  1, INT_MAX, FLAGS },
> > > -    { "t",    "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { .i64 =  4 },  1, INT_MAX, FLAGS },
> > > +    /* Actual default value for band/t is 1, set in init */
> > > +    { "band", "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { .i64 =  0 },  0, INT_MAX, FLAGS },
> > > +    { "t",    "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { .i64 =  0 },  0, INT_MAX, FLAGS },
> > >      { "show", "show delogo area",          OFFSET(show), AV_OPT_TYPE_BOOL,{ .i64 =  0 },  0, 1,       FLAGS },
> > >      { NULL }
> > >  };
> > > @@ -201,6 +202,12 @@ static av_cold int init(AVFilterContext
> > >      CHECK_UNSET_OPT(w);
> > >      CHECK_UNSET_OPT(h);
> > >  
> > > +    if (s->band == 0) { /* Unset, use default */
> > > +        av_log(ctx, AV_LOG_WARNING, "Note: default band value was changed from 4 to 1.\n");
> > > +        s->band = 1;
> > > +    } else if (s->band != 1) {
> > > +        av_log(ctx, AV_LOG_WARNING, "Option band is deprecated.\n");
> > > +    }
> > >      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);
> > 
> > LGTM. BTW, if you want to drop the band option, you could ifdef it so
> > that it will be dropt at the next lavfi major bump.
> 
> Good idea. Something like this?
> 
> ---
>  libavfilter/vf_delogo.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> --- ffmpeg.orig/libavfilter/vf_delogo.c	2015-10-07 08:45:39.037203764 +0200
> +++ ffmpeg/libavfilter/vf_delogo.c	2015-10-07 13:52:43.716487733 +0200
> @@ -165,9 +165,11 @@ static const AVOption delogo_options[]=
>      { "y",    "set logo y position",       OFFSET(y),    AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, FLAGS },
>      { "w",    "set logo width",            OFFSET(w),    AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, FLAGS },
>      { "h",    "set logo height",           OFFSET(h),    AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, FLAGS },
> +#if LIBAVFILTER_VERSION_MAJOR < 7
>      /* Actual default value for band/t is 1, set in init */
>      { "band", "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { .i64 =  0 },  0, INT_MAX, FLAGS },
>      { "t",    "set delogo area band size", OFFSET(band), AV_OPT_TYPE_INT, { .i64 =  0 },  0, INT_MAX, FLAGS },
> +#endif
>      { "show", "show delogo area",          OFFSET(show), AV_OPT_TYPE_BOOL,{ .i64 =  0 },  0, 1,       FLAGS },
>      { NULL }
>  };
> @@ -202,12 +204,16 @@ static av_cold int init(AVFilterContext
>      CHECK_UNSET_OPT(w);
>      CHECK_UNSET_OPT(h);
>  
> +#if LIBAVFILTER_VERSION_MAJOR < 7
>      if (s->band == 0) { /* Unset, use default */
>          av_log(ctx, AV_LOG_WARNING, "Note: default band value was changed from 4 to 1.\n");
>          s->band = 1;
>      } else if (s->band != 1) {
>          av_log(ctx, AV_LOG_WARNING, "Option band is deprecated.\n");
>      }
> +#else
> +    s->band = 1;
> +#endif
>      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);

Looks fine to me.
  
> Or should I use FF_API_OLD_FILTER_OPTS?

No, I think that's related to another issue.

> Assuming this is what you had in mind, would it go as a separate patch,
> or should this be folded in the patch I just sent?

Use a single patch since it involves less work, unless you prefer to
do it with a separate patch.
-- 
FFmpeg = Frightening and Faithless Merciless Powerful Epic Gnome


More information about the ffmpeg-devel mailing list