[FFmpeg-devel] [PATCH] libavfilter: add vf_hue.c

Stefano Sabatini stefasab at gmail.com
Sat Aug 11 10:52:32 CEST 2012


On date Saturday 2012-08-11 08:27:44 +0200, Clément Bœsch encoded:
> On Sat, Aug 11, 2012 at 01:20:45AM +0200, Stefano Sabatini wrote:
> [...]
> > > +} HueContext;
> > > +
> > > +static av_cold int init(AVFilterContext *ctx, const char *args)
> > > +{
> > > +    HueContext *hue = ctx->priv;
> > > +    float h, s;
> > > +
> > 
> > > +    if (args)
> > > +        sscanf(args, "%f:%f", &h, &s);
> > 
> > I tend to prefer the use of named options, despite the fact of
> > breaking compatibility with mp=hue, because of the increased
> > robustness/flexibility/overall consistency.
> > 
> > What's the opinion of other devs?
> > 
> 

> Given the name of the filter, I don't think much more options will be
> added. A more generic filter handling other equalizer parameters such as
> brightness or contrast would have required it, but in that particular case
> I wonder if that matters much...

There is still the possibility to use radians in place of degrees,
which is not possible with the h:s syntax (and I find silly that I
have to specify 0:s if I want to change only s). Add to this the
possibility to specify expressions (e.g. "-PI/4"), perform range
checks and introspection out of the box when using opt.
 
> > Alternatively we could try to support both syntaxes (check how it is
> > done in buffersrc).
> > 
> 
> Why not but in that case it could be done later I guess.

I'm fine with it if people insist with syntax backward compatibility,
with the possibility to extend it later. If it turns out too complex
to maintain both syntaxes I suggest to deprecate and drop this one.
-- 
FFmpeg = Fast Fundamental Meaningless Philosophical Elastic Geek


More information about the ffmpeg-devel mailing list