[FFmpeg-devel] [PATCH] lavfi/scale: accept named option, make parsing more robust

[Michael Niedermayer]

On Sun, Oct 28, 2012 at 08:05:28PM +0100, Stefano Sabatini wrote:
> On date Sunday 2012-10-28 19:51:01 +0100, Michael Niedermayer encoded:
> > On Sun, Oct 28, 2012 at 05:33:37PM +0100, Stefano Sabatini wrote:
> [...]
> > > Subject: [PATCH] lavfi/scale: accept named option, make parsing more robust
> > > 
> > > Also update documentation accordingly.
> > > 
> > > TODO: bump micro
> > [...]
> > > @@ -76,7 +77,8 @@ typedef struct {
> > >       *  -1 = keep original aspect
> > >       */
> > >      int w, h;
> > > +    const char *flags_str;      ///sws flags string
> > 
> > 
> > > -    unsigned int flags;         ///sws flags
> > > +    int         flags;          ///sws flags
> > 
> > this doesnt look correct
> 
> Bummer, fixed.
> 
> [...]
> > >  static av_cold int init(AVFilterContext *ctx, const char *args)
> > >  {
> > >      ScaleContext *scale = ctx->priv;
> > > -    const char *p;
> > > +    static const char *shorthand[] = { "w", "h", NULL };
> > > +    int ret;
> > >  
> > > -    av_strlcpy(scale->w_expr, "iw", sizeof(scale->w_expr));
> > > -    av_strlcpy(scale->h_expr, "ih", sizeof(scale->h_expr));
> > > +    scale->class = &scale_class;
> > > +    av_opt_set_defaults(scale);
> > > +
> > > +    if ((ret = av_opt_set_from_string(scale, args, shorthand, "=", ":")) < 0)
> > > +        return ret;
> > > +
> > > +    av_log(ctx, AV_LOG_VERBOSE, "w:%s h:%s flags:%s interl:%d\n",
> > > +           scale->w_expr, scale->h_expr, scale->flags_str, scale->interlaced);
> > >  
> > >      scale->flags = SWS_BILINEAR;
> > > -    if (args) {
> > > -        sscanf(args, "%255[^:]:%255[^:]", scale->w_expr, scale->h_expr);
> > > -        p = strstr(args,"flags=");
> > > -        if (p) {
> > > -            const AVClass *class = sws_get_class();
> > > -            const AVOption    *o = av_opt_find(&class, "sws_flags", NULL, 0,
> > > -                                               AV_OPT_SEARCH_FAKE_OBJ);
> > > -            int ret = av_opt_eval_flags(&class, o, p + 6, &scale->flags);
> > > -
> > > -            if (ret < 0)
> > > -                return ret;
> > > -        }
> > > -        if(strstr(args,"interl=1")){
> > > -            scale->interlaced=1;
> > > -        }else if(strstr(args,"interl=-1"))
> > > -            scale->interlaced=-1;
> 
> > > +    if (scale->flags_str) {
> > > +        const AVClass *class = sws_get_class();
> > > +        const AVOption    *o = av_opt_find(&class, "sws_flags", NULL, 0,
> > > +                                           AV_OPT_SEARCH_FAKE_OBJ);
> > > +        int ret = av_opt_eval_flags(&class, o, scale->flags_str, &scale->flags);
> > > +        if (ret < 0)
> > > +            return ret;
> > >      }
> > 
> > I think it would be better if all options would be sent straight to
> > sws instead of being parsed into a local AVClass based context.
> 
> Uhm, do you mean also w and h? How to deal with expressions?

Fixing AVOptions first ;)


> 
> > later requires every option to be duplicated and be kept in sync.
> 
> > also this fails with multiple (sws_)flags options that is like
> > flags=+abc:flags=-abc
>  
> > if w/h need special handling then they could be intercepted and
> > handlded special instead of being sent to sws.
> 
> This is not supported also in current version. This could be
> implemented by returning the options which could not be set (and would
> be useful in other places as well - e.g. the movie source/sink), but
> I'd prefer to leave this to a separate change to avoid to make this
> patch even more painful than it currently already is.

i guess the patch is probably ok

a random unrelated idea i stumbled accross during review is support
for things like -vf scale=qcif


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121028/0ed65aed/attachment.asc>