diff mbox

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

Message ID 20170805094206.21791-1-george@nsup.org
State Superseded
Headers show

Commit Message

Nicolas George Aug. 5, 2017, 9:42 a.m. UTC
Signed-off-by: Nicolas George <george@nsup.org>
---
 doc/filters.texi | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Michael Niedermayer Aug. 5, 2017, 1:36 p.m. UTC | #1
On Sat, Aug 05, 2017 at 11:42:06AM +0200, Nicolas George wrote:
> Signed-off-by: Nicolas George <george@nsup.org>
> ---
>  doc/filters.texi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 7e5a9a625a..886cd5a7e8 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -161,6 +161,11 @@ follow the same constraints order of the previous point. The following
>  
>  @end itemize
>  
> +Future evolutions of filters may require inserting new options or changing
> +their order, especially for the non-essential options, and that would break
> +options given without their name. For that reason, uses that require
> +stability should favor the @var{key=value} notation.

This is ambigous and if interpreted litterally
then even a scale=320:240 would no longer be guranteed to work
but would require scale=width=320:height=240 to be used.

the shorthand is widely used and convenient
IMHO declaring it _all_ unstable would be a mistake

also iam thinking that we should either accept the option order we
choose when initially adding a filter and try
to maintain compatibility whenever that is possible.
Or specify exactly for each filter in the code which options are allowed
in shorthand and then also not accept options beyond that if accessed
through shorthand

That way anything that works will contine to work and one cannot
mistakly write a script that uses unstable shorthand options


[...]
Nicolas George Aug. 5, 2017, 1:42 p.m. UTC | #2
L'octidi 18 thermidor, an CCXXV, Michael Niedermayer a écrit :
> This is ambigous and if interpreted litterally
> then even a scale=320:240 would no longer be guranteed to work
> but would require scale=width=320:height=240 to be used.

That is intentional. Note that w=320:h=240 is enough: four characters
more.

> the shorthand is widely used and convenient

It is convenient, but not absolutely necessary. Once in a script, the
extra function names make it more readable and more robust anyway.

> That way anything that works will contine to work and one cannot
> mistakly write a script that uses unstable shorthand options

Remember that "anything that works will continue to work" is opposed to
"we can add new features without being burdened by past mistakes".

Regards,
Michael Niedermayer Aug. 5, 2017, 3:34 p.m. UTC | #3
On Sat, Aug 05, 2017 at 03:42:08PM +0200, Nicolas George wrote:
> L'octidi 18 thermidor, an CCXXV, Michael Niedermayer a écrit :
> > This is ambigous and if interpreted litterally
> > then even a scale=320:240 would no longer be guranteed to work
> > but would require scale=width=320:height=240 to be used.
> 
> That is intentional. Note that w=320:h=240 is enough: four characters
> more.

I think you do not realize how annoying this would be in practice

Users do not know all these nuances that one syntax is stable and work
and one works but is not future proof. It also violates the priniple of
least surprise. Aka this fails one of the most fundamental rules of
user interface design


> 
> > the shorthand is widely used and convenient
> 
> It is convenient, but not absolutely necessary. Once in a script, the
> extra function names make it more readable and more robust anyway.
> 
> > That way anything that works will contine to work and one cannot
> > mistakly write a script that uses unstable shorthand options
> 

> Remember that "anything that works will continue to work" is opposed to
> "we can add new features without being burdened by past mistakes".

This comment makes me a bit sad.

It implies that old code is bad without any solid fact, nothing one can
proof, disproof, or fix. And thats not even related here, you
didnt start by wanting to fix a mistake in the order, the changed order
was a side effect of other work.

also my suggestion was to define what is stable now and maintain that
properly and disallow what is not stable. Have a clear interface and
stick to it.

Stable interfaces are important.
Nicolas George Aug. 5, 2017, 4:42 p.m. UTC | #4
L'octidi 18 thermidor, an CCXXV, Michael Niedermayer a écrit :
> I think you do not realize how annoying this would be in practice
> 
> Users do not know all these nuances that one syntax is stable and work
> and one works but is not future proof. It also violates the priniple of
> least surprise. Aka this fails one of the most fundamental rules of
> user interface design

I think all competent users should realize that a shortcut is less
stable than the full notation.

> This comment makes me a bit sad.
> 
> It implies that old code is bad without any solid fact, nothing one can
> proof, disproof, or fix.

I am not saying that all the old code is bad. But do you want to imply
that all of it is good? As long as some of it is bas, we need to improve
it, and unconditional compatibility is an hindrance to that.

>			   And thats not even related here, you
> didnt start by wanting to fix a mistake in the order, the changed order
> was a side effect of other work.

Actually, the "other work" was fixing a mistake: implementing the
options in the individual filters.

> also my suggestion was to define what is stable now and maintain that
> properly and disallow what is not stable. Have a clear interface and
> stick to it.
> 
> Stable interfaces are important.

Named options are stable. There is no need for anything more.

Regards,
James Almer Aug. 5, 2017, 5:14 p.m. UTC | #5
On 8/5/2017 1:42 PM, Nicolas George wrote:
> L'octidi 18 thermidor, an CCXXV, Michael Niedermayer a écrit :
>> I think you do not realize how annoying this would be in practice
>>
>> Users do not know all these nuances that one syntax is stable and work
>> and one works but is not future proof. It also violates the priniple of
>> least surprise. Aka this fails one of the most fundamental rules of
>> user interface design
> 
> I think all competent users should realize that a shortcut is less
> stable than the full notation.

Users may not know that it's a shortcut to begin with.

> 
>> This comment makes me a bit sad.
>>
>> It implies that old code is bad without any solid fact, nothing one can
>> proof, disproof, or fix.
> 
> I am not saying that all the old code is bad. But do you want to imply
> that all of it is good? As long as some of it is bas, we need to improve
> it, and unconditional compatibility is an hindrance to that.
> 
>> 			   And thats not even related here, you
>> didnt start by wanting to fix a mistake in the order, the changed order
>> was a side effect of other work.
> 
> Actually, the "other work" was fixing a mistake: implementing the
> options in the individual filters.
> 
>> also my suggestion was to define what is stable now and maintain that
>> properly and disallow what is not stable. Have a clear interface and
>> stick to it.
>>
>> Stable interfaces are important.
> 
> Named options are stable. There is no need for anything more.
> 
> Regards,
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Nicolas George Aug. 5, 2017, 5:15 p.m. UTC | #6
L'octidi 18 thermidor, an CCXXV, James Almer a écrit :
> Users may not know that it's a shortcut to begin with.

It is exactly the reason I am proposing to document it here.

Regards,
James Almer Aug. 5, 2017, 6 p.m. UTC | #7
On 8/5/2017 2:15 PM, Nicolas George wrote:
> L'octidi 18 thermidor, an CCXXV, James Almer a écrit :
>> Users may not know that it's a shortcut to begin with.
> 
> It is exactly the reason I am proposing to document it here.

Indeed, but that's something that should have been done once AVOptions
gained the shorthand feature with the CLI, not several years down the
line and hundreds of scripts and tutorials potentially considering it a
fixed syntax.

At this point, breaking current shorthand behavior is pretty disruptive,
at least without some warnings and removal grace period.
My suggestion would be to keep dummy options in a similar fashion we
keep deprecated functions, so the shorthand notation does not start
trying to fill values of unexpected new or moved options. Making them
raise a warning about how it will stop working in the near future (No
need for two years like with API, one or two releases should be enough)
and maybe mention the new option that will be filled by the shorthand
notation.

Is there for that matter a way to achieve this for the CLI only and not
keeping the dummy options for library users?
Nicolas George Aug. 5, 2017, 9:17 p.m. UTC | #8
L'octidi 18 thermidor, an CCXXV, James Almer a écrit :
> Indeed, but that's something that should have been done once AVOptions
> gained the shorthand feature with the CLI, not several years down the
> line and hundreds of scripts and tutorials potentially considering it a
> fixed syntax.
> 
> At this point, breaking current shorthand behavior is pretty disruptive,
> at least without some warnings and removal grace period.
> My suggestion would be to keep dummy options in a similar fashion we
> keep deprecated functions, so the shorthand notation does not start
> trying to fill values of unexpected new or moved options. Making them
> raise a warning about how it will stop working in the near future (No
> need for two years like with API, one or two releases should be enough)
> and maybe mention the new option that will be filled by the shorthand
> notation.

That would be ideal, if we had unlimited manpower. Unfortunately, we do
not.

> Is there for that matter a way to achieve this for the CLI only and not
> keeping the dummy options for library users?

Not easily, I think.

Regards,
James Almer Aug. 5, 2017, 10:51 p.m. UTC | #9
On 8/5/2017 6:17 PM, Nicolas George wrote:
> L'octidi 18 thermidor, an CCXXV, James Almer a écrit :
>> Indeed, but that's something that should have been done once AVOptions
>> gained the shorthand feature with the CLI, not several years down the
>> line and hundreds of scripts and tutorials potentially considering it a
>> fixed syntax.
>>
>> At this point, breaking current shorthand behavior is pretty disruptive,
>> at least without some warnings and removal grace period.
>> My suggestion would be to keep dummy options in a similar fashion we
>> keep deprecated functions, so the shorthand notation does not start
>> trying to fill values of unexpected new or moved options. Making them
>> raise a warning about how it will stop working in the near future (No
>> need for two years like with API, one or two releases should be enough)
>> and maybe mention the new option that will be filled by the shorthand
>> notation.
> 
> That would be ideal, if we had unlimited manpower. Unfortunately, we do
> not.

What do you mean? What i suggested would be done each time an option is
removed or added anywhere but at the end, both of which afaik are
uncommon cases.
It's not something that requires a rewrite of the current codebase.

> 
>> Is there for that matter a way to achieve this for the CLI only and not
>> keeping the dummy options for library users?
> 
> Not easily, I think.
> 
> Regards,
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Nicolas George Aug. 6, 2017, 8:37 a.m. UTC | #10
L'octidi 18 thermidor, an CCXXV, James Almer a écrit :
> What do you mean? What i suggested would be done each time an option is
> removed or added anywhere but at the end, both of which afaik are
> uncommon cases.
> It's not something that requires a rewrite of the current codebase.

I mean that since I consider the break bearable (somebody upgrades a
piece of software, they MUST test the scripts that depend on it, and
fixing the issue once and for all is easy), I am not willing to spend my
time implementing (and testing, for this kind of thing that takes time)
the deprecation-warning-and-backward-compatibility dance.

Therefore, I think anybody opposing that change has three choices:
propose a solution (possibly a patch) that does not require extra work,
present or future, from the lavfi maintainers; own up that they are
blocking progress in the lavfi cleanup; or yield.

Note: to help the transition, I am perfectly willing to write a
ChangeLog entry, that does not take much time:

- The order of options in filters is no longer considered stable. If
  long-term stability is necessary (i.e. for scripts), use named options
  (e.g. overlay=50:100 -> overlay=x=50:y=100).

We have caused much more severe breakage in the past (especially
considering that only minor options change, and they were usually set by
name anyway), and ones that were much harder to fix.

Regards,
Michael Niedermayer Aug. 7, 2017, 1:59 a.m. UTC | #11
On Sun, Aug 06, 2017 at 10:37:35AM +0200, Nicolas George wrote:
> L'octidi 18 thermidor, an CCXXV, James Almer a écrit :
> > What do you mean? What i suggested would be done each time an option is
> > removed or added anywhere but at the end, both of which afaik are
> > uncommon cases.
> > It's not something that requires a rewrite of the current codebase.
> 
> I mean that since I consider the break bearable (somebody upgrades a
> piece of software, they MUST test the scripts that depend on it, and
> fixing the issue once and for all is easy), I am not willing to spend my
> time implementing (and testing, for this kind of thing that takes time)
> the deprecation-warning-and-backward-compatibility dance.

Lets take a step back and look at this

There are some rarely used options in multi input filters like
overlay which break.
Noone even noticed except me

And you propose to declare the most used syntax from every filter
unstable.

This just doesnt add up, its like shooting the patient in the head as
a treatment for a cold

Please correct me if iam wrong, but isnt all that is needed to just
not remove the options from the AVOption array from the tiny number
of filters affected?
Or declare the tiny number of moved/changed options as removed/not
supported in shorthand notation.

[...]
Marton Balint Aug. 7, 2017, 7:25 a.m. UTC | #12
On Mon, 7 Aug 2017, Michael Niedermayer wrote:

> On Sun, Aug 06, 2017 at 10:37:35AM +0200, Nicolas George wrote:
>> L'octidi 18 thermidor, an CCXXV, James Almer a écrit :
>>> What do you mean? What i suggested would be done each time an option is
>>> removed or added anywhere but at the end, both of which afaik are
>>> uncommon cases.
>>> It's not something that requires a rewrite of the current codebase.
>>
>> I mean that since I consider the break bearable (somebody upgrades a
>> piece of software, they MUST test the scripts that depend on it, and
>> fixing the issue once and for all is easy), I am not willing to spend my
>> time implementing (and testing, for this kind of thing that takes time)
>> the deprecation-warning-and-backward-compatibility dance.
>
> Lets take a step back and look at this
>
> There are some rarely used options in multi input filters like
> overlay which break.
> Noone even noticed except me
>
> And you propose to declare the most used syntax from every filter
> unstable.
>
> This just doesnt add up, its like shooting the patient in the head as
> a treatment for a cold

Do you like this text better?

Future evolutions of filters may require inserting new options or changing 
their order, especially for the non-essential options, and while we make 
reasonable effort to keep the order so the options given without their 
name would not break, sometimes that is not feasable. Therefore users 
should generally favor the @var{key=value} notation, or refer to the 
Changelog file which should contain such incompatible changes.

>
> Please correct me if iam wrong, but isnt all that is needed to just
> not remove the options from the AVOption array from the tiny number
> of filters affected?
> Or declare the tiny number of moved/changed options as removed/not
> supported in shorthand notation.

Yeah, what is needed for compatibility? Only a single AVOption line, or 
additional code as well?

Regards,
Marton
Nicolas George Aug. 7, 2017, 9:03 a.m. UTC | #13
> >Lets take a step back and look at this
> >
> >There are some rarely used options in multi input filters like
> >overlay which break.
> >Noone even noticed except me
> >
> >And you propose to declare the most used syntax from every filter
> >unstable.
> >
> >This just doesnt add up, its like shooting the patient in the head as
> >a treatment for a cold

No, that is like telling the patient they need to take their antibiotics
until the last day of treatment and not stop once they feel better,
because they risk getting a pneumonia. Which is technically true, only
very unlikely.


Le decadi 20 thermidor, an CCXXV, Marton Balint a écrit :
> Do you like this text better?
> 
> Future evolutions of filters may require inserting new options or changing
> their order, especially for the non-essential options, and while we make
> reasonable effort to keep the order so the options given without their name
> would not break, sometimes that is not feasable. Therefore users should
> generally favor the @var{key=value} notation, or refer to the Changelog file
> which should contain such incompatible changes.

This is obviously the intent, but I do not think that the documentation
is the place to make that kind of intent statement.

> Yeah, what is needed for compatibility? Only a single AVOption line, or
> additional code as well?

Additional code as well. Not a lot, but still some. And testing. An
exponential number of cases.

I do not intend to do it.

We have already wasted more time than all the users combined will spend
typing "x=", "y=". Seriously, people !?!

Now, to all that stated a negative opinion about this:

I have in the queue a patch series that changes the options in a minor
way, but at the same time fixes bugs and long-standing limitations of
lavfi and makes the code more robust and cleaner.

Do you :

1. propose to implement and test compatibility options yourself?

2. own up you are blocking these enhancements?

3. accept the series and the breakage?

I will consider the absence of reply in a reasonable time as "4. do not
care".

Regards,
John Warburton Aug. 7, 2017, 5:41 p.m. UTC | #14
On Mon, Aug 7, 2017 at 10:03 AM, Nicolas George <george@nsup.org> wrote:
>> >Lets take a step back and look at this
>> >
>> >There are some rarely used options in multi input filters like
>> >overlay which break.
>> >Noone even noticed except me
...

> Now, to all that stated a negative opinion about this:
>
> I have in the queue a patch series that changes the options in a minor
> way, but at the same time fixes bugs and long-standing limitations of
> lavfi and makes the code more robust and cleaner.
>
> Do you :
>
> 1. propose to implement and test compatibility options yourself?
>
> 2. own up you are blocking these enhancements?
>
> 3. accept the series and the breakage?

I am not a developer (though am learning to read the source code) but
if an old FFmpeg script of mine breaks, as they have on a couple of
occasions, isn't it logical for a user of the command line to go
straight to the documentation, and the answer (and the fix) always
seems to be there? Or just type "ffmpeg -h filter=scale" or similar,
where the order of un-named options isn't even mentioned? In my
experience today, much more so than in the olden days when we used to
type "C> wp.exe mydoc.wpd", CLI users aren't afraid of documentation
and development.

Regards,
JW
Gyan Aug. 7, 2017, 6:44 p.m. UTC | #15
On Mon, Aug 7, 2017 at 11:11 PM, John Warburton <john@johnwarburton.net>
wrote:


> if an old FFmpeg script of mine breaks, as they have on a couple of
> occasions, isn't it logical for a user of the command line to go
> straight to the documentation, and the answer (and the fix) always
> seems to be there?
>

If a modified filter with value-only arguments produces unexpected results,
possible if the new option accepts same type/range as the older option in
that position, then the user may be confounded as to why the filter is
behaving differently. Same if the new option isn't accommodating and the
command doesn't parse. This patch is being inserted in the docs up top, in
the generic filtering section. The user is unlikely to look there for
guidance, but in the section specific to the filter. So it may not be clear
why the filter invocation fails, given the keys aren't present.
Marton Balint Aug. 7, 2017, 7:13 p.m. UTC | #16
On Tue, 8 Aug 2017, Gyan wrote:

> On Mon, Aug 7, 2017 at 11:11 PM, John Warburton <john@johnwarburton.net>
> wrote:
>
>
>> if an old FFmpeg script of mine breaks, as they have on a couple of
>> occasions, isn't it logical for a user of the command line to go
>> straight to the documentation, and the answer (and the fix) always
>> seems to be there?
>>
>
> If a modified filter with value-only arguments produces unexpected results,
> possible if the new option accepts same type/range as the older option in
> that position, then the user may be confounded as to why the filter is
> behaving differently. Same if the new option isn't accommodating and the
> command doesn't parse. This patch is being inserted in the docs up top, in
> the generic filtering section. The user is unlikely to look there for
> guidance, but in the section specific to the filter. So it may not be clear
> why the filter invocation fails, given the keys aren't present.

That is why I believe a breaking change such as the overlay filter 
option order change should be mentioned in the Changelog.

Regards,
Marton
Michael Niedermayer Aug. 7, 2017, 9:33 p.m. UTC | #17
On Mon, Aug 07, 2017 at 09:25:27AM +0200, Marton Balint wrote:
> 
> 
> On Mon, 7 Aug 2017, Michael Niedermayer wrote:
> 
> >On Sun, Aug 06, 2017 at 10:37:35AM +0200, Nicolas George wrote:
> >>L'octidi 18 thermidor, an CCXXV, James Almer a écrit :
> >>>What do you mean? What i suggested would be done each time an option is
> >>>removed or added anywhere but at the end, both of which afaik are
> >>>uncommon cases.
> >>>It's not something that requires a rewrite of the current codebase.
> >>
> >>I mean that since I consider the break bearable (somebody upgrades a
> >>piece of software, they MUST test the scripts that depend on it, and
> >>fixing the issue once and for all is easy), I am not willing to spend my
> >>time implementing (and testing, for this kind of thing that takes time)
> >>the deprecation-warning-and-backward-compatibility dance.
> >
> >Lets take a step back and look at this
> >
> >There are some rarely used options in multi input filters like
> >overlay which break.
> >Noone even noticed except me
> >
> >And you propose to declare the most used syntax from every filter
> >unstable.
> >
> >This just doesnt add up, its like shooting the patient in the head as
> >a treatment for a cold
> 
> Do you like this text better?

No


> 
> Future evolutions of filters may require inserting new options or
> changing their order,

This is not true.
like future evolutions of the C standard dont require reordering
function arguments or standard POSIX tools dont require
renaming options.
There simply is no such requirement
We should be honest in the text.
"People working on future evolutions of libavfilter _WANT_ to insert
options or change their order"

If we write that its required some will realize that this is unlikely
and look deeper and it would make FFmpeg as a whole look
unprofessional.


> especially for the non-essential options, and
> while we make reasonable effort to keep the order so the options
> given without their name would not break,

So far there was effort toward changing the order not toward keeping it.
If effort is directed towards keeping it then we probably do not need
this text.

Yes we may hit a case once in a while where theres a majority prefering
to break some rarely used case for code cleanliness but this doesnt
require declaring the whole of the interface as unstable.
It is basically the whole as everyone and everything uses the shorthand
notation including our and others docs, examples and tutorials.


> sometimes that is not
> feasable. Therefore users should generally favor the @var{key=value}
> notation,  [...]

I do not agree as a maintainer of several filters of libavfilter.

I do try to keep my code compatible
and if someone would inform me that theres a break in the interface of
a filter of mine i would fix that if it is possible.
[if there are such issues in filters i maintain, please directly
 mail me or ping me on IRC or CC me on tickets]

I do care about users of my code, be that end users of tools or
developers using the libs.
And to me a stable interface and stable public API is a essential part
in (practically) usable software.

Will i fix every interface break in every filter? i do not know.
I do know though that iam interrested in helping with that as long
as the shorthand is part of the stable interface and help from me is
welcome. So far ive the feeling that my help is VERY unwelcome.
And this patch removes the shorthand from the stable interface.
Removing any and all reasons why i would be interrested in working on
it.


> 
> >
> >Please correct me if iam wrong, but isnt all that is needed to just
> >not remove the options from the AVOption array from the tiny number
> >of filters affected?
> >Or declare the tiny number of moved/changed options as removed/not
> >supported in shorthand notation.
> 
> Yeah, what is needed for compatibility? Only a single AVOption line,
> or additional code as well?

There are 2 ways at least, one is:

One line for each removed option, in fact the removed line, and
possbly one line to copy it to the new place if this cannot be
directly referred to from the AVOption line.
If the factored struct is referenceable by offset from the context
that is either is part of the other then only a single line is needed

Heres the removed and added options from the patches:
As you can see its the same options, only OFFSET would have to
refer to the new place

-    { "eof_action", "Action to take when encountering EOF from secondary input ",
-        OFFSET(eof_action), AV_OPT_TYPE_INT, { .i64 = EOF_ACTION_REPEAT },
-        EOF_ACTION_REPEAT, EOF_ACTION_PASS, .flags = FLAGS, "eof_action" },
-        { "repeat", "Repeat the previous frame.",   0, AV_OPT_TYPE_CONST, { .i64 = EOF_ACTION_REPEAT }, .flags = FLAGS, "eof_action" },
-        { "endall", "End both streams.",            0, AV_OPT_TYPE_CONST, { .i64 = EOF_ACTION_ENDALL }, .flags = FLAGS, "eof_action" },
-        { "pass",   "Pass through the main input.", 0, AV_OPT_TYPE_CONST, { .i64 = EOF_ACTION_PASS },   .flags = FLAGS, "eof_action" },
...
-    { "shortest", "force termination when the shortest input terminates", OFFSET(opt_shortest), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
...
-    { "repeatlast", "repeat overlay of the last overlay frame", OFFSET(opt_repeatlast), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },


+static const AVOption framesync_options[] = {
+    { "eof_action", "Action to take when encountering EOF from secondary input ",
+        OFFSET(opt_eof_action), AV_OPT_TYPE_INT, { .i64 = EOF_ACTION_REPEAT },
+        EOF_ACTION_REPEAT, EOF_ACTION_PASS, .flags = FLAGS, "eof_action" },
+        { "repeat", "Repeat the previous frame.",   0, AV_OPT_TYPE_CONST, { .i64 = EOF_ACTION_REPEAT }, .flags = FLAGS, "eof_action" },
+        { "endall", "End both streams.",            0, AV_OPT_TYPE_CONST, { .i64 = EOF_ACTION_ENDALL }, .flags = FLAGS, "eof_action" },
+        { "pass",   "Pass through the main input.", 0, AV_OPT_TYPE_CONST, { .i64 = EOF_ACTION_PASS },   .flags = FLAGS, "eof_action" },
+    { "shortest", "force termination when the shortest input terminates", OFFSET(opt_shortest), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS },
+    { "repeatlast", "repeat overlay of the last overlay frame", OFFSET(opt_repeatlast), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, FLAGS },
+    { NULL }
+};


The other is a list of shorthand options (one line per affected filter)
That way options are moveable, as the order in the shorthand list
defines order and which are shorthand options.
This would only be needed for filters where this differs from the default.
It would require some code to handle that shorthand list but would
make cleanup easier and clearly delimiting what is part of the shorthand
and what is not alot clearer.


[...]
Michael Niedermayer Aug. 7, 2017, 10:01 p.m. UTC | #18
On Mon, Aug 07, 2017 at 11:03:51AM +0200, Nicolas George wrote:
> > >Lets take a step back and look at this
> > >
> > >There are some rarely used options in multi input filters like
> > >overlay which break.
> > >Noone even noticed except me
> > >
> > >And you propose to declare the most used syntax from every filter
> > >unstable.
> > >
> > >This just doesnt add up, its like shooting the patient in the head as
> > >a treatment for a cold
> 
> No, that is like telling the patient they need to take their antibiotics
> until the last day of treatment and not stop once they feel better,
> because they risk getting a pneumonia. Which is technically true, only
> very unlikely.
> 
> 
> Le decadi 20 thermidor, an CCXXV, Marton Balint a écrit :
> > Do you like this text better?
> > 
> > Future evolutions of filters may require inserting new options or changing
> > their order, especially for the non-essential options, and while we make
> > reasonable effort to keep the order so the options given without their name
> > would not break, sometimes that is not feasable. Therefore users should
> > generally favor the @var{key=value} notation, or refer to the Changelog file
> > which should contain such incompatible changes.
> 
> This is obviously the intent, but I do not think that the documentation
> is the place to make that kind of intent statement.
> 
> > Yeah, what is needed for compatibility? Only a single AVOption line, or
> > additional code as well?
> 
> Additional code as well. Not a lot, but still some. And testing. An
> exponential number of cases.
> 
> I do not intend to do it.
> 
> We have already wasted more time than all the users combined will spend
> typing "x=", "y=". Seriously, people !?!
> 
> Now, to all that stated a negative opinion about this:
> 
> I have in the queue a patch series that changes the options in a minor
> way, but at the same time fixes bugs and long-standing limitations of
> lavfi and makes the code more robust and cleaner.
> 
> Do you :
> 

> 1. propose to implement and test compatibility options yourself?

If you push the lavfi patchset but the doc option instability patch
is not pushed then ill likely try to fix the compatibility issue
due to the moved options in overlay and similar filters.

If theres some preferrance on how to fix it?
(iam happy to fix it in the way you perfer as long as its not more
 work than the obvious way to fix it)


[...]
Nicolas George Aug. 8, 2017, 9:20 a.m. UTC | #19
Le decadi 20 thermidor, an CCXXV, Marton Balint a écrit :
> That is why I believe a breaking change such as the overlay filter option
> order change should be mentioned in the Changelog.

Yes, you are completely right.

Also, let me clarify something: pushing this documentation change does
not mean I intend to break the order of options at a whim. Not at all.
In practice, nothing will change, almost. But it means that if there is
a strong need to change the order, we can consider it comfortably, and
without wasting valuable developer time with compatibility workarounds.

Regarding the exact rule: we could document that there are some options
we do not intend to touch (essential options), but what would be the
point, really? Do we require users to learn the list by rote? Are they
supposed to  look in the documentation each time? The rule "use named
options, it will keep working" is simple, easy to remember, and the
extra requirement (write "x=", "y=", essential options are usually
short) is minimal and small compared to the task of learning which
option would be stable and which would not.

With that said, what about this change?

Regards,
Michael Niedermayer Aug. 8, 2017, 1:09 p.m. UTC | #20
Hi

On Tue, Aug 08, 2017 at 11:20:31AM +0200, Nicolas George wrote:
> Le decadi 20 thermidor, an CCXXV, Marton Balint a écrit :
> > That is why I believe a breaking change such as the overlay filter option
> > order change should be mentioned in the Changelog.
> 
> Yes, you are completely right.
> 
> Also, let me clarify something: pushing this documentation change does
> not mean I intend to break the order of options at a whim. Not at all.
> In practice, nothing will change, almost. But it means that if there is
> a strong need to change the order, we can consider it comfortably, and
> without wasting valuable developer time with compatibility workarounds.
> 

> Regarding the exact rule: we could document that there are some options
> we do not intend to touch (essential options), but what would be the
> point, really?

Iam a bit puzzled that you really seem not to see what damage this
would do.

All scripts, all applications using libavfilter must be changed to
use the named arguments if the shorthand is not stable.
all examples will need to be changed too or the examples would show
syntax that is non stable.
Examples do not target scripts or end users specifically so we may
end up with duplication and extra complexity.

If you dislike C language lawyerng over undefined behavior, this is
exactly the same but worse.
If we say it may be unstable even if its very very unlikely everyone
who takes stability serious will change their code, tutorials, docs
and examples.
This will also add alot of confusion to teh end user as suddenly
the consistent use of shorthand is replaced by some documents and
tutorials using the shorthand in accordance of it being ok-ish and
some take the strict approuch and remove all shorthand use.
Extra explanations to clarify "why" added, cause the user to waste more
time to learn about "why"

The whole would add a reason to avoid libavfilter and use an
alternative
If i "as a user" have the choice between 2 libs, one with a long term
stable interface and gurantee by its developers that it will stay stable
and a lib that in practice broke its interface and added a declaration
that parts which where stable before are not anymore and unspecified
future changes are possible". I would favor he lib with stable interface.
And loss of users also always means a loss of potential developers,
as users of a lib is a source of developers of that lib.
Loss of some users is bad on its own of course ...

[...]
Nicolas George Aug. 8, 2017, 1:54 p.m. UTC | #21
Le primidi 21 thermidor, an CCXXV, Michael Niedermayer a écrit :
> Iam a bit puzzled that you really seem not to see what damage this
> would do.

I am a lot puzzled that you really imagine it would do such damage.

> All scripts, all applications using libavfilter must be changed to
> use the named arguments if the shorthand is not stable.
> all examples will need to be changed too or the examples would show
> syntax that is non stable.
> Examples do not target scripts or end users specifically so we may
> end up with duplication and extra complexity.
> 
> If you dislike C language lawyerng over undefined behavior, this is
> exactly the same but worse.
> If we say it may be unstable even if its very very unlikely everyone
> who takes stability serious will change their code, tutorials, docs
> and examples.
> This will also add alot of confusion to teh end user as suddenly
> the consistent use of shorthand is replaced by some documents and
> tutorials using the shorthand in accordance of it being ok-ish and
> some take the strict approuch and remove all shorthand use.
> Extra explanations to clarify "why" added, cause the user to waste more
> time to learn about "why"

This is all in a day's work for anybody upgrading any kind of program.
People switching to later versions of Firefox or LibreOffice have a
harder time than that.

> The whole would add a reason to avoid libavfilter and use an
> alternative

Nonsense, the syntax with named options is stable.

Your ability to imagine what would annoy users and cause damage is
obviously not much better than mine, which is worthless. It is not
surprising, neither of us is a normal user. Then let us stop speculating
and let us listen to what actual users have said on the matter about the
issue on this very mailing-list, please.

Regards,
diff mbox

Patch

diff --git a/doc/filters.texi b/doc/filters.texi
index 7e5a9a625a..886cd5a7e8 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -161,6 +161,11 @@  follow the same constraints order of the previous point. The following
 
 @end itemize
 
+Future evolutions of filters may require inserting new options or changing
+their order, especially for the non-essential options, and that would break
+options given without their name. For that reason, uses that require
+stability should favor the @var{key=value} notation.
+
 If the option value itself is a list of items (e.g. the @code{format} filter
 takes a list of pixel formats), the items in the list are usually separated by
 @samp{|}.