[FFmpeg-devel] [Patch] Scale filter should use multiples of 2

[Stefano Sabatini]

On date Wednesday 2010-07-07 23:14:42 +0200, Michael Niedermayer encoded:
> On Wed, Jul 07, 2010 at 11:02:01PM +0200, Stefano Sabatini wrote:
> > On date Wednesday 2010-07-07 15:53:16 -0400, Daniel G. Taylor encoded:
> > > On 07/01/2010 10:27 AM, Stefano Sabatini wrote:
> > > >>Basically, I don't know how I can do this easily, and writing a
> > > >>bunch of code doing it a stupid way seems like a waste of time, so
> > > >>any advice would be appreciated if we do want to go this way.
> > > >
> > > >libavutil/eval.h, check how it is done in the libavfilter-soc overlay
> > > >and setpts filters. This requires to add eval evalutation to the scale
> > > >filter, check the recent patch by Baptiste on which I commented
> > > >yesterday.
> > > 
> > > Thanks for the help here Stefano.
> > > 
> > > I've run into a bit of a snag I'd say... See attached patch to
> > > reproduce what I'm running into, but basically:
> > > 
> > >  * av_parse_expr and such use a comma as a delimiter for function
> > >    arguments
> > >  * avfilter's graph parsing uses a comma as a delimiter for filters
> > > 
> > > Because of that it means anytime you put a comma into the filter
> > > arguments it thinks you are passing in the next filter. If we are
> > > going to allow functions and such as filter arguments it might be a
> > > good idea to change the syntax for specifying filters, maybe using a
> > > pipe character like "|" or something, e.g. -vf
> > > "scale|unsharp|...|crop".
> > > 
> > > Attached patch works for stuff like -vf "scale=320:240" or -vf
> > > "scale=W/2:H/2" and such but if you try -vf "scale=W:round(H, 16)"
> > > you'll immediately see the issue.
> > > 
> > > Thoughts?
> > 
> > Possible solutions:
> > 
> > 1) change the character currently used, which is ",", and which is
> > very likely to appear in expressions inside the filter args.
> > 
> > '|' is an option, note that we didn't choose '|' in the first place
> > because it's a special sh character, for example this doesn't work in
> > sh:
> > -vf scale|unsharp|crop
> > 
> > you need:
> > -vf "scale|unsharp|crop"
> > 
> > For me this doesn't look like a big issue, but we should wart the
> > users in the docs.
> > 
> > The requirement is that the choosen character cannot compare inside
> > the filter args.
> > 
> > 2) make the separator definable by the user, for example we could have
> > something like:
> > -vfd "%" -vf "scale % unsharp % ... % crop".
> > 
> > 3) Implement an escaping mechanism. Since at the first level of
> > escaping there is just one special character, we may use '\' to escape
> > the ',', for example:
> > 
> > scale="W:round(H\, 16), unsharp, crop"
> > 
> > The problem with this approach is that combined with the shell
> > escaping mechanism this can easily became an escaping hell.
> > 
> > So I tend to prefer the option 2, and use by default the value "|" for
> > the separator.
> 
> how do you pass a string to a filter? you need escaping anyway
> a eval expression is just a string ...

Ehm, after further inspection of the code I discovered that the graph
parser *already* supports escaping (through the av_get_token()
function).

So for the particular case above:
"scale=W:round(H\, 16)"

or even:
'scale=W:round(H, 16)'

Regards.
-- 
FFmpeg = Fast Fundamental Most Puritan Epic Ghost