[FFmpeg-devel,01/54] avformat: Add internal flags for AV(In|Out)putFormat

Both AVInputFormat and AVOutputFormat currently lack an equivalent to
AVCodec's caps_internal. E.g. if reading a header fails, each demuxer
is currently required to clean up manually, which often means to just
call the demuxer's read_close function. This could (and will) be done
generically via an equivalent of FF_CODEC_CAP_INIT_CLEANUP.

Because of the unholy ABI-relationship between libavdevice and
libavformat adding such a flag is only possible when the ABI is open
(despite the flag not being part of the public API), such as now.
Therefore such a flag is also added to AVOutputFormat, despite there
being no immediate use for it.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/avformat.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

On 6/15/2021 8:01 PM, Andreas Rheinhardt wrote:
> Both AVInputFormat and AVOutputFormat currently lack an equivalent to
> AVCodec's caps_internal. E.g. if reading a header fails, each demuxer
> is currently required to clean up manually, which often means to just
> call the demuxer's read_close function. This could (and will) be done
> generically via an equivalent of FF_CODEC_CAP_INIT_CLEANUP.
> 
> Because of the unholy ABI-relationship between libavdevice and
> libavformat adding such a flag is only possible when the ABI is open
> (despite the flag not being part of the public API), such as now.
> Therefore such a flag is also added to AVOutputFormat, despite there
> being no immediate use for it.

Since new "non public" fields can if needed be added at the very end of 
the struct, is this still an issue?
It's adding new public ones which generate issues because they will 
invariably push the offset of all the non public ones down by a few 
bytes, and thus break lavd devices.

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavformat/avformat.h | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index cd7b0d941c..81d2ac38d0 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -535,6 +535,11 @@ typedef struct AVOutputFormat {
>        */
>       int priv_data_size;
>   
> +    /**
> +     * Internal flags. See FF_FMT_FLAG_* in internal.h.
> +     */
> +    int flags_internal;
> +
>       int (*write_header)(struct AVFormatContext *);
>       /**
>        * Write a packet. If AVFMT_ALLOW_FLUSH is set in flags,
> @@ -674,6 +679,11 @@ typedef struct AVInputFormat {
>        */
>       int priv_data_size;
>   
> +    /**
> +     * Internal flags. See FF_FMT_FLAG_* in internal.h.
> +     */
> +    int flags_internal;
> +
>       /**
>        * Tell if a given file has a chance of being parsed as this format.
>        * The buffer provided is guaranteed to be AVPROBE_PADDING_SIZE bytes
>

James Almer:
> On 6/15/2021 8:01 PM, Andreas Rheinhardt wrote:
>> Both AVInputFormat and AVOutputFormat currently lack an equivalent to
>> AVCodec's caps_internal. E.g. if reading a header fails, each demuxer
>> is currently required to clean up manually, which often means to just
>> call the demuxer's read_close function. This could (and will) be done
>> generically via an equivalent of FF_CODEC_CAP_INIT_CLEANUP.
>>
>> Because of the unholy ABI-relationship between libavdevice and
>> libavformat adding such a flag is only possible when the ABI is open
>> (despite the flag not being part of the public API), such as now.
>> Therefore such a flag is also added to AVOutputFormat, despite there
>> being no immediate use for it.
> 
> Since new "non public" fields can if needed be added at the very end of
> the struct, is this still an issue?

Yes, because one is allowed to use an old libavdevice together with a
new libavformat. The AV(In|Out)putFormats defined in libavdevice don't
have these new fields; so using these fields in libavformat is
problematic even when they are at the end. While this is not an
unsurmountable problem (if a commit adding new fields bumps libavdevice
version, one can check via via AV_IS_(IN|OUT)PUT_DEVICE whether the
in/outformat is a device and then use a version check whether the given
libavdevice version has the desired field), it is easier to just add
flags_internal to AVOutputFormat, too.

> It's adding new public ones which generate issues because they will
> invariably push the offset of all the non public ones down by a few
> bytes, and thus break lavd devices.
> 
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>   libavformat/avformat.h | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index cd7b0d941c..81d2ac38d0 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -535,6 +535,11 @@ typedef struct AVOutputFormat {
>>        */
>>       int priv_data_size;
>>   +    /**
>> +     * Internal flags. See FF_FMT_FLAG_* in internal.h.
>> +     */
>> +    int flags_internal;
>> +
>>       int (*write_header)(struct AVFormatContext *);
>>       /**
>>        * Write a packet. If AVFMT_ALLOW_FLUSH is set in flags,
>> @@ -674,6 +679,11 @@ typedef struct AVInputFormat {
>>        */
>>       int priv_data_size;
>>   +    /**
>> +     * Internal flags. See FF_FMT_FLAG_* in internal.h.
>> +     */
>> +    int flags_internal;
>> +
>>       /**
>>        * Tell if a given file has a chance of being parsed as this
>> format.
>>        * The buffer provided is guaranteed to be AVPROBE_PADDING_SIZE
>> bytes
>>
>

Andreas Rheinhardt (12021-06-16):
> Yes, because one is allowed to use an old libavdevice together with a
> new libavformat.

Why do we allow that? What is the actual benefit?

Regards,

Nicolas George:
> Andreas Rheinhardt (12021-06-16):
>> Yes, because one is allowed to use an old libavdevice together with a
>> new libavformat.
> 
> Why do we allow that? What is the actual benefit?
> 
AFAIK: Nothing. And I don't like it.

- Andreas

Andreas Rheinhardt (12021-06-16):
> AFAIK: Nothing. And I don't like it.

Good news!

So, would somebody object if we made it so that only versions from the
same version, and possibly same configuration, could be used together?

Regards,

On Wed, Jun 16, 2021 at 11:33 AM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Nicolas George:
> > Andreas Rheinhardt (12021-06-16):
> >> Yes, because one is allowed to use an old libavdevice together with a
> >> new libavformat.
> >
> > Why do we allow that? What is the actual benefit?
> >
> AFAIK: Nothing. And I don't like it.

It was my expectation that it would only make sense to use a set of
dynamic libraries generated from the same build (and thus revision),
not to mix and match. Is there a cost to no longer allowing (i.e.
documenting) mixing of library versions? Is there a benefit, e.g. in
being able to break (internal) ABI (not an issue i oversee)?

(My interest here of course is moving forward with avdevice and
implementing their APIs in dshow.)

Cheers,
Dee

Diederick C. Niehorster (12021-06-16):
> Is there a benefit, e.g. in being able to break (internal) ABI (not an
> issue i oversee)?

The current discussion is precisely about a benefit: libraries accessing
each other's internals would no longer be a technical problem, which
makes us free on how we consider it an organisational problem.

Regards,

Diederick C. Niehorster:
> On Wed, Jun 16, 2021 at 11:33 AM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
>>
>> Nicolas George:
>>> Andreas Rheinhardt (12021-06-16):
>>>> Yes, because one is allowed to use an old libavdevice together with a
>>>> new libavformat.
>>>
>>> Why do we allow that? What is the actual benefit?
>>>
>> AFAIK: Nothing. And I don't like it.
> 
> It was my expectation that it would only make sense to use a set of
> dynamic libraries generated from the same build (and thus revision),
> not to mix and match. Is there a cost to no longer allowing (i.e.
> documenting) mixing of library versions? Is there a benefit, e.g. in
> being able to break (internal) ABI (not an issue i oversee)?
> 
Cost: It might force you to update more libraries, thereby increasing
download (or upload if you are the distributor) size.
Benefit: Besides fixing the horrible libavformat-libavdevice
relationship (we are currently not able to add a public field to
AV(In|Out)putFormat unless the ABI is open; we are not able to add new
private fields either if they are to be used in libavformat unless we
add checks for whether we deal with a device from an old version of
libavdevice that doesn't this option; we can not remove private fields
unless the ABI is open) it also makes sizeof of all our internal structs
usable in all libraries; because all the offsets are fixed, the avpriv
functions that are basically getters and setters can be removed
immediately. Structures crossing library boundaries were no problem any
more (we accidentally broke ABI in 88d80cb97 with this; it wouldn't be a
problem at all if we disallowed using libraries from different
revisions). We can also modify all avpriv symbols without caring about
compatibility with older users.

- Andreas

On Wed, Jun 16, 2021 at 12:45 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
> Cost: It might force you to update more libraries, thereby increasing
> download (or upload if you are the distributor) size.
> Benefit: Besides fixing the horrible libavformat-libavdevice
> relationship (we are currently not able to add a public field to
> AV(In|Out)putFormat unless the ABI is open; we are not able to add new
> private fields either if they are to be used in libavformat unless we
> add checks for whether we deal with a device from an old version of
> libavdevice that doesn't this option; we can not remove private fields
> unless the ABI is open) it also makes sizeof of all our internal structs
> usable in all libraries; because all the offsets are fixed, the avpriv
> functions that are basically getters and setters can be removed
> immediately. Structures crossing library boundaries were no problem any
> more (we accidentally broke ABI in 88d80cb97 with this; it wouldn't be a
> problem at all if we disallowed using libraries from different
> revisions). We can also modify all avpriv symbols without caring about
> compatibility with older users.

Thanks both for writing in, seems to me like costs outweigh the
benefits. Its certainly the easiest solution to an important part of
the avdevice debate.

Curious what others think!

Cheers,
Dee

On 6/16/2021 6:37 AM, Nicolas George wrote:
> Andreas Rheinhardt (12021-06-16):
>> AFAIK: Nothing. And I don't like it.
> 
> Good news!
> 
> So, would somebody object if we made it so that only versions from the
> same version, and possibly same configuration, could be used together?

Since merging lavf and lavf is apparently not an option, I will not be 
against restricting lavf and lavd to matching versions, which will 
basically achieve the same effect and save us all these headaches.
At least until lavd is changed to not require or use lavf at all, or do 
it in a saner way.

> 
> Regards,
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>

James Almer (12021-06-16):
> Since merging lavf and lavf is apparently not an option, I will not be
> against restricting lavf and lavd to matching versions

Please explain why you would be against it otherwise, and why you are
against for other libraries.

Regards,

On 6/16/2021 6:41 PM, Nicolas George wrote:
> James Almer (12021-06-16):
>> Since merging lavf and lavf is apparently not an option, I will not be
>> against restricting lavf and lavd to matching versions
> 
> Please explain why you would be against it otherwise,

I'm not sure what you mean. I would not be against it, it's just that if 
we were to merge lavf and lavd, this wouldn't even be something to 
consider. Since the idea of merging them was rejected, I'm in favor of 
version locking them.

> and why you are
> against for other libraries.

Can you be more specific?

James Almer (12021-06-16):
> I'm not sure what you mean. I would not be against it, it's just that if we
> were to merge lavf and lavd, this wouldn't even be something to consider.

Have you not read the discussion? The benefits go way beyond the tiny
lavf-lavd issues.

> > and why you are
> > against for other libraries.
> Can you be more specific?

When I say "I am for X" and you reply "I am not against Y", with Y⊂X and
Y≠X, you are implicitly saying that you are against X∖Y. I proposed to
restrict to matching versions on all libraries, you replied you were not
against restricting for lavf-lavd, stating implicitly that you are
against it for the libraries.

So, I ask explicitly: are you against restricting to matching versions
for all the libraries? If so, then why?

Regards,

On 6/16/2021 7:14 PM, Nicolas George wrote:
> James Almer (12021-06-16):
>> I'm not sure what you mean. I would not be against it, it's just that if we
>> were to merge lavf and lavd, this wouldn't even be something to consider.
> 
> Have you not read the discussion? The benefits go way beyond the tiny
> lavf-lavd issues.
> 
>>> and why you are
>>> against for other libraries.
>> Can you be more specific?
> 
> When I say "I am for X" and you reply "I am not against Y", with Y⊂X and
> Y≠X, you are implicitly saying that you are against X∖Y. I proposed to
> restrict to matching versions on all libraries, you replied you were not
> against restricting for lavf-lavd, stating implicitly that you are
> against it for the libraries.

Considering this discussion started about lavd and lavf, and at no point 
in your email you said "all libraries" when suggesting version locking 
anything (In fact, you mistakenly wrote "only versions from the same 
version"), i figured it would be obvious i was commenting specifically 
about lavd<->lavf.

> 
> So, I ask explicitly: are you against restricting to matching versions
> for all the libraries? If so, then why?

Of course i am against doing so for all libraries. They are already 
locked at the major soname level as required, and that's enough.
Lavd as is is locking certain lavf's public structs and making it 
impossible to add new fields to them, which constricts development 
considerably, so making their locking strict to at least the minor 
soname level is a very good idea. But this isn't an issue at all for 
other libraries.
I can add new fields to any public lavc struct, compile it and have a 
lavf that was linked to an old lavc use it at runtime, and it will work 
just fine.
I see no benefit to your suggestion that's worth removing such freedom 
from the end user.

James Almer (12021-06-16):
> Considering this discussion started about lavd and lavf, and at no point in
> your email you said "all libraries" when suggesting version locking anything
> (In fact, you mistakenly wrote "only versions from the same version"), i
> figured it would be obvious i was commenting specifically about lavd<->lavf.

Ok, I read too much in your wording after you read too little in mine.
No problem.

> Of course i am against doing so for all libraries. They are already locked
> at the major soname level as required, and that's enough.
> Lavd as is is locking certain lavf's public structs and making it impossible
> to add new fields to them, which constricts development considerably, so
> making their locking strict to at least the minor soname level is a very
> good idea. But this isn't an issue at all for other libraries.
> I can add new fields to any public lavc struct, compile it and have a lavf
> that was linked to an old lavc use it at runtime, and it will work just
> fine.
> I see no benefit to your suggestion that's worth removing such freedom from
> the end user.

I am sorry, but you are not looking hard enough. You are describing ONE
of the several compatibility issue we have, the one that happens most on
lavf-lavd.

But please realize that every single avpriv symbol is a reason to lock
all libraries version of its own.

It is very dense for lavd, but lavd is tiny. There are tons of it on the
lavc-lavc side, many different from each other and requiring subtly
different solutions if they need updating.

Furthermore, what you explained is not a reason to be against, it is the
absence of reason to be for. Opposing something is not the same thing as
not supporting it: if you do not care, or if you do not know, you are
encouraged to neither oppose nor support.

So, I ask again:

Do you have a reason to oppose locking all library versions together?

Regards,

On 6/17/2021 6:27 AM, Nicolas George wrote:
> James Almer (12021-06-16):
>> Considering this discussion started about lavd and lavf, and at no point in
>> your email you said "all libraries" when suggesting version locking anything
>> (In fact, you mistakenly wrote "only versions from the same version"), i
>> figured it would be obvious i was commenting specifically about lavd<->lavf.
> 
> Ok, I read too much in your wording after you read too little in mine.
> No problem.
> 
>> Of course i am against doing so for all libraries. They are already locked
>> at the major soname level as required, and that's enough.
>> Lavd as is is locking certain lavf's public structs and making it impossible
>> to add new fields to them, which constricts development considerably, so
>> making their locking strict to at least the minor soname level is a very
>> good idea. But this isn't an issue at all for other libraries.
>> I can add new fields to any public lavc struct, compile it and have a lavf
>> that was linked to an old lavc use it at runtime, and it will work just
>> fine.
>> I see no benefit to your suggestion that's worth removing such freedom from
>> the end user.
> 
> I am sorry, but you are not looking hard enough. You are describing ONE
> of the several compatibility issue we have, the one that happens most on
> lavf-lavd.
> 
> But please realize that every single avpriv symbol is a reason to lock
> all libraries version of its own.

Countless avpriv_ symbols would not be gone with any kind of locking. 
Only a couple ones that work as getter/setter to workaround struct 
offsets. And, surprise, all of those are in lavd, which will be gone if 
we version lock it with lavf.

> 
> It is very dense for lavd, but lavd is tiny. There are tons of it on the
> lavc-lavc side, many different from each other and requiring subtly
> different solutions if they need updating.
> 
> Furthermore, what you explained is not a reason to be against, it is the
> absence of reason to be for. Opposing something is not the same thing as
> not supporting it: if you do not care, or if you do not know, you are
> encouraged to neither oppose nor support.
> 
> So, I ask again:
> 
> Do you have a reason to oppose locking all library versions together?

I already gave you the reason why I'm *against* it. I find it insulting 
that you completely disregard it and then ask again if i have one, 
instead of if i have another.

Version locking all libraries adds a constrain that's not required, 
brings no worthwhile benefits, and removes freedom for the end user.
And I'll mention that your wish to do this certainly feels like a 
concealed attempt to try to push decisions and changes closer towards 
your personal end goal of merging all libraries.

I have nothing else to say. I support version locking lavf and lavd, and 
completely oppose version locking every other library for the reasons i 
already gave you.

James Almer (12021-06-17):
> Countless avpriv_ symbols would not be gone with any kind of locking.

No, they would not be gone. But they would no longer be a compatibility
and maintenance burden either. That is a huge benefit.

> I already gave you the reason why I'm *against* it. I find it insulting that
> you completely disregard it and then ask again if i have one, instead of if
> i have another.
> 
> Version locking all libraries adds a constrain that's not required, brings
> no worthwhile benefits,

This is no reason to be against it, this is just you showing that you
did not think deep enough to realize the benefits.

I am sorry if my way of pointing this to you rubs you the wrong way, but
I hope you will some day realize that this attitude from you rubs me the
wrong way. We both have to make efforts to communicate with the other.

> 			  and removes freedom for the end user.

This, I accept as a reason, but a very weak one. Do we know how many
users would want to make use of that freedom?

> And I'll mention that your wish to do this certainly feels like a concealed
> attempt to try to push decisions and changes closer towards your personal
> end goal of merging all libraries.

Please refrain in the future from accusing people of covert intents, it
is rarely helpful.

> I have nothing else to say. I support version locking lavf and lavd, and
> completely oppose version locking every other library for the reasons i
> already gave you.

A very weak one, and a few technical misunderstandings.

Regards,

On 6/17/2021 9:55 AM, Nicolas George wrote:
> James Almer (12021-06-17):
>> Countless avpriv_ symbols would not be gone with any kind of locking.
> 
> No, they would not be gone. But they would no longer be a compatibility
> and maintenance burden either. That is a huge benefit.
> 
>> I already gave you the reason why I'm *against* it. I find it insulting that
>> you completely disregard it and then ask again if i have one, instead of if
>> i have another.
>>
>> Version locking all libraries adds a constrain that's not required, brings
>> no worthwhile benefits,
> 
> This is no reason to be against it, this is just you showing that you
> did not think deep enough to realize the benefits.
> 
> I am sorry if my way of pointing this to you rubs you the wrong way, but
> I hope you will some day realize that this attitude from you rubs me the
> wrong way. We both have to make efforts to communicate with the other.

Start by dropping the condescending and disregardful attitude every 
single one of your replies exudes.

> 
>> 			  and removes freedom for the end user.
> 
> This, I accept as a reason, but a very weak one. Do we know how many
> users would want to make use of that freedom?
> 
>> And I'll mention that your wish to do this certainly feels like a concealed
>> attempt to try to push decisions and changes closer towards your personal
>> end goal of merging all libraries.
> 
> Please refrain in the future from accusing people of covert intents, it
> is rarely helpful.
> 
>> I have nothing else to say. I support version locking lavf and lavd, and
>> completely oppose version locking every other library for the reasons i
>> already gave you.
> 
> A very weak one, and a few technical misunderstandings.

Case in point.

James Almer (12021-06-17):
> Start by dropping the condescending and disregardful attitude every single
> one of your replies exudes.

Stop giving me orders.

Nicolas George (12021-06-17):
> Stop giving me orders.

And, to make things more things more constructive:

What you perceive as condescension from me is actually annoyance,
because I feel you never actually read my arguments carefully enough to
realize what I mean.

I have apologized for rubbing you the wrong way, I am genuinely sorry.
But things will not evolve for the good unless your make an effort too.

Top-posting on purpose:

Several core developers stated to be in favor of locking lavd<->lavf
together at the minor version, for instance since it solves a lot of
the problems the intimate linking between the two libraries creates.
Should this be turned into a patch somehow (i'm not sure how to do
such linking)? If so, could someone send this patch or guide me
through it, so that the discussion can be finished?
Thanks!

On Thu, Jun 17, 2021 at 12:45 AM James Almer <jamrial@gmail.com> wrote:
>
> On 6/16/2021 7:14 PM, Nicolas George wrote:
> > James Almer (12021-06-16):
> >> I'm not sure what you mean. I would not be against it, it's just that if we
> >> were to merge lavf and lavd, this wouldn't even be something to consider.
> >
> > Have you not read the discussion? The benefits go way beyond the tiny
> > lavf-lavd issues.
> >
> >>> and why you are
> >>> against for other libraries.
> >> Can you be more specific?
> >
> > When I say "I am for X" and you reply "I am not against Y", with Y⊂X and
> > Y≠X, you are implicitly saying that you are against X∖Y. I proposed to
> > restrict to matching versions on all libraries, you replied you were not
> > against restricting for lavf-lavd, stating implicitly that you are
> > against it for the libraries.
>
> Considering this discussion started about lavd and lavf, and at no point
> in your email you said "all libraries" when suggesting version locking
> anything (In fact, you mistakenly wrote "only versions from the same
> version"), i figured it would be obvious i was commenting specifically
> about lavd<->lavf.
>
> >
> > So, I ask explicitly: are you against restricting to matching versions
> > for all the libraries? If so, then why?
>
> Of course i am against doing so for all libraries. They are already
> locked at the major soname level as required, and that's enough.
> Lavd as is is locking certain lavf's public structs and making it
> impossible to add new fields to them, which constricts development
> considerably, so making their locking strict to at least the minor
> soname level is a very good idea. But this isn't an issue at all for
> other libraries.
> I can add new fields to any public lavc struct, compile it and have a
> lavf that was linked to an old lavc use it at runtime, and it will work
> just fine.
> I see no benefit to your suggestion that's worth removing such freedom
> from the end user.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Diederick C. Niehorster (12021-08-25):
> Several core developers stated to be in favor of locking lavd<->lavf
> together at the minor version, for instance since it solves a lot of
> the problems the intimate linking between the two libraries creates.
> Should this be turned into a patch somehow (i'm not sure how to do
> such linking)? If so, could someone send this patch or guide me
> through it, so that the discussion can be finished?

If I had to do it, this is how, but it may require some experimenting:

- In version.h of all libraries, add a macro
  avLIBRARY_version_same_minor() that expands to
  avLIBRARY_version_MINOR_MAJOR().

- In all libraries, implement avLIBRARY_version_same_minor() the same
  way as avLIBRARY_version(). By the magic of macros, it will define the
  avLIBRARY_version_MINOR_MAJOR symbol.

- In libavdevice, add this to avdevice_version_same_minor():

  if ((avformat_version_same_minor()) & ~0xFF != (LIBAVFORMAT_VERSION & ~0xFF))
      abort();

It does not matter if the application does not call
avformat_version_same_minor(), it will mark the symbol as required, and
the dynamic linker will check.

But it should not be done only for libavdevice, it should be done for
all libraries that use avpriv symbols too.

Regards,