[FFmpeg-devel] [PATCH 01/16] doc/filters: document the unstability of the shorthand options notation.

[Clément Bœsch]

On Fri, Aug 11, 2017 at 12:33:25PM +0200, Nicolas George wrote:
> Le quartidi 24 thermidor, an CCXXV, Clement Boesch a écrit :
> > I'm afraid of the situation where a developer will feel like the order of
> > the options is not ideal, or an option could be renamed for consistency
> > with other filters, and will take the easy way out "oh well, we documented
> > it's unstable, users can only blame themselves for relying on it, not even
> > mentioning the new order/name is much better for everyone".
> 
> In my mind, even with this documentation commit, changing the order of
> an option would still require posting a patch on the mailing-list to
> discuss it. It could be stated explicitly, but I do not know where (the
> user documentation is not the place).
> 
> As for renaming options, this is absolutely not allowed.
> 
> > If you're referring to this current patch serie, can you list here all the
> > filters that are affected by an API break and to what degree?
> > 
> > We can discuss here if it's an acceptable change or not, and should
> > require a compatibility code (maybe it will be for one or two filters
> > only).
> 
> All the filters that use framesync2.h and have any of the "shortest",
> "eof_action" and "repeatlast" options lose the possibility to set these
> options by shorthand notation. I.e. you can no longer put 1 in the fifth
> or sixth position in the arguments list and hope it will fall on
> "shortest".
> 

That looks like it has a marginal effect in this particular case. I'd
agree with just documenting it in the Changelog and still not making it
"the rule" (that is, NAK on the doc patch). Also please bump minor or
micro.

If other people insist on compatibility, I'd suggest the following:

In ff_framesync2_init(), run av_opt_find() with "eof_action", "shortest"
and "repeatlast" on parent (AVFilterContext); if you can't find the option
you fallback on the framesync opts values. Then you wrap the fields and
options in the filters with ifdefery, and announce the future breakage in
doc/APIchanges.

I don't think that's worth the hassle (even though I don't think that's
much more work), but people may disagree.

> > We could consider an API to deprecate an AVOption for future similar
> > issue. I think we already had such discussion in the past (maybe that was
> > around the time we changed "user-agent" into "user_agent"), involving the
> > presence of a flag.
> 
> Yes, we could, and that takes time that could be dedicated to fixing
> actual bugs or implementing new features. Ask any user: "if you agree to
> always write x=, y= in your overlay filters, I can fix your bug faster,
> what do you prefer?", I know what they will answer.

You know the answer of the user with the bug, not the hundreds others who
don't have the bug, rely on the shorthand, read a SO/reddit/forum post
with a now non-working example, or execute a random plugin of an app which
suddenly stop working.

But your patch doesn't seem to affect the x and y of overlay, so everything is
fine, right? :)

[...]

BTW, now that I think about it, how about this:

We add a flag to AVOption such as AV_OPT_FLAG_FILTER_STABLE_SHORTHAND and
flag all the options that we believe won't need to move in the future and
for which we can always rely on (typically x and y in overlay, or w and h
in scale).

Then we simply warn the user for every shorthand use of other options
("using short-hand form with '<option>' may cause issue in the future as
it could be swapped around").

That way, we:

- give clear visibility of the instability of (some of) the shorthands
- affect only marginal uses (that is, late options because we are
  generally going to flag the few first ones)
- gain flexibility to indeed swap around the options at will in the future
  (we initially wait for a release or two such that the users gets aware
  of their potential misuses with the introduced warning)
- keep shorthands useful for the most essential cases
- provide trust on the current use of shorthands.

Regards,

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170811/43cf54bb/attachment.sig>