On Wed, 7 Oct 2015 14:19:58 +0200, Stefano Sabatini wrote:
> > > 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.
Great.
> > Or should I use FF_API_OLD_FILTER_OPTS?
>
> No, I think that's related to another issue.
I thought so but wasn't sure.
> > 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.
I'm fine either way so if a single patch is easier for you, I'll do
that. I'll send a v2 of the patch in a minute.
Thanks for the review,
--
Jean Delvare
SUSE L3 Support
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
