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 <jdelv...@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
> > -@var{w} and @var{h}). The default value is 4.
> > +@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);
Or should I use FF_API_OLD_FILTER_OPTS?
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?
Thanks,
--
Jean Delvare
SUSE L3 Support
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
