[FFmpeg-devel] [PATCH] swscale: Support setting filters through AVOptions

[wm4]

On Mon, 4 Nov 2013 23:38:55 +0100
Stefano Sabatini <stefasab at gmail.com> wrote:

> On date Monday 2013-11-04 22:30:25 +0100, wm4 encoded:
> > On Mon, 4 Nov 2013 14:32:23 +0100
> > Stefano Sabatini <stefasab at gmail.com> wrote:
> > 
> > > On date Tuesday 2013-10-29 20:16:21 +0100, wm4 encoded:
> [...]
> > > > The description isn't terribly clear IMHO. It's not
> > > > really clear to me how to do what sws_getDefaultFilter() and
> > > > sws_getGaussianVec() did (because these were the only real uses for
> > > > this filter stuff apparently). Does the user have to reimplement them,
> > > > and then convert the array of coefficients to a string separated by
> > > > ','?
> > > 
> > > I could easily support gauss() and const() vector operators.
> > > 
> > > At this point we need to extend the syntax. Probably something like:
> > > 
> > > vector  ::= [domain=][function|sequence]
> > > vectors ::= vector [&vector]
> > > filter  ::= vectors
> > 
> > Yes, if you want to go this way, this should be added. The new API
> > should cover all uses of the old API (unless they're deemed as not
> > worth supporting).
> 
> This doesn't yet support vectors operators like convolve or scalar
> product. I don't know if it is really worth since this will require
> the implementation of a proper parser/mini-language.
> 
> > 
> > > > (Also what's the difference between '|' and ','?)
> > > > Or maybe use the original functions, and then convert the SwsFilter
> > > > manually to a string?
> > > > 
> > > > All in all pretty weird IMO. It's geared towards somehow making the
> > > > filter vectors settable through ffmpeg/ffplay (because apparently these
> > > > tools are too dumb to have their own option parsers), instead of making
> > > > it usable to API users.
> > > 
> 
> > > What's the problem with API users? Can't they set them directly in the
> > > SwsContext with sws_getCachedContext()?
> > 
> > Using a set of functions is easier than generating a string. (Both for
> > users and implementers of an API!)
> 
> struct SwsContext *sws_getCachedContext(struct SwsContext *context,
>                                         int srcW, int srcH, enum AVPixelFormat srcFormat,
>                                         int dstW, int dstH, enum AVPixelFormat dstFormat,
>                                         int flags, SwsFilter *srcFilter,
>                                         SwsFilter *dstFilter, const double *param);
> 
> This implies that you can set the filter directly by using the
> vector/filter functions (or alternatively specifying a filter and
> parsing it).

That's why I added setters for SWsFilters in my previous patch...

> > 
> > > > What exactly is the advantage of this, and the
> > > > use case? Everything becomes more awkward, but at least it's supported
> > > > by the glorious AVOption API? I'm quite a bit skeptic about this.
> > > 
> > > Why "more awkward"? The idea is to be able to set it through external
> > > components (e.g. libavfilter) and to let the high-level user (with no
> > > direct interface to libswscale internals) to define them. Note that
> > > this is orthogonal to setting it programmatically.
> > 
> 
> > User interface issues shouldn't be the job of a very low level core
> > library like libswscale.
> 
> So where do you suggest to implement such stuff?

They could be separate utility functions somewhere. I do not think a
low level API should dictate a command line syntax.