master broke AVCodecContext compatibility with LibAV project

[jyavenard]

The commit that broke compatibility with libav is

commit 7404f3bdb90e6a5dcb59bc0a091e2c5c038e557d
Author: Michael Niedermayer <michael@niedermayer.cc>
Date:   Tue Sep 15 18:01:32 2015 +0200

    lavc: Switch bitrate to 64bit unless compatibility with avconv was requested.

I maintain the ffmpeg support in Firefox.

I was looking at adding support for libavcodec version 57 (current master) when I noticed that AVCodecContext::bit_rate was changed to be an int64_t.

The member rc_max_rate and rc_min_rate were also changed in that commit.

This has very bad consequences for us as it means we won't be able to support libavcodec v57 from either the ffmpeg or libav project.
Up to now we have managed to support all versions of FFmpeg v1.0 and later and LibAV 0.8 and later. It would be unfortunate that this now stop

Please revert that commit, or provide a new member allowing portability.

Thank you.

[Hendrik]

We have decided to no longer maintain binary compatibility with the Libav project, and libavcodec 57 uses a 64-bit bit_rate field now. Note that there have not been any releases using this new ABI yet.

The major versions was bumped in kind (to 57), which signifies a change in ABI.

[jyavenard]

I strongly urge you to reconsider this decision...

[Elon Musk]

It is not just this single int.

[Hendrik]

Replying to jyavenard:

I strongly urge you to reconsider this decision...

This discussion has been going on for a long time, and we are not about to re-open it, sorry.

[Hendrik]

FWIW there are many projects that still manage to support both Libav and FFmpeg, and never even relied on them both having the same ABI. The answer is to simply re-compile to match the OS you are running on. If you build a package for Debian, compile Firefox against all the libraries available on Debian.

libavcodec et al have not been "drop-in" ABI compatible between FFmpeg and Libav for a long time, unless they were specifically built for that - which no distribution did.

[gjdfgh]

Are you really talking about ABI compatibility or just API compatibility?

[jyavenard]

Replying to heleppkes:

libavcodec et al have not been "drop-in" ABI compatible between FFmpeg and Libav for a long time, unless they were specifically built for that - which no distribution did.

Do you realise how many distributions our single binary package support and how many million users that represent?

We do not ship ffmpeg, we can only use whatever library is currently present, libavcodec and libavutil being dynamically linked when required.

We have no way to tell at present which version it is we are loading.
Seeing that we are only using libavcodec (we do the demuxing ourselves), having a compatible AVCodecContext structure is pretty much all we need.

[jyavenard]

Replying to richardpl:

It is not just this single int.

those three ints are the only difference that matters to us.

[Hendrik]

Replying to jyavenard:

Replying to richardpl:

It is not just this single int.

those three ints are the only difference that matters to us.

But "partial" compatibility is not something that works really well. It would set everyone up for silent failure down the line (there are many such "silent" problems already, some codec ids are different, some pixel format ids) - so for us its either all or nothing, and we've decided that "all" is too much of a strain on development and therefor decided against it.

Note that you can query the version functions and adjust behavior at runtime. FFmpeg will always have a micro version over 100, while Libav starts at 0. Doing this in binary is probably rather painful, but no more painful than supporting 56 and 57 in parallel either way.

[Michael Niedermayer]

for reference, the final decission to drop the compatibility mode was made in the IRC meeting on Saturday 2015-09-12, UTC 15:00. Full logs of that should be here: ​http://permalink.gmane.org/gmane.comp.video.ffmpeg.devel/198919
I have no real oppinion on it but IIRC there was noone who really argued to keep it.

But that aside, i dont fully understand how this affects firefox. No distro which used FFmpeg had this compatibility mode enabled. If the binary packages from distros where "compatible enough" then that was by chance of only using a small subset of their features, there certainly was no compatibility when the mode wasnt enabled (and it was not enabled in any official packages from any distro AFAIK).

Now, is it still possible to link and use both across distros, iam not sure it very well might be.
Most fields in AVCodecContext can be accessed through the AVOption API, see libavutil/opt.h
Is this sufficient for your use case ?

[Carl Eugen Hoyos]

Replying to jyavenard:

Please revert that commit

We cannot revert this commit, it would brake binary compatibility.

Do you realise how many distributions our single binary package support

Which distribution has the issue you are describing?

[Michael Niedermayer]

More specifically, using av_opt_set_int() and av_opt_get_int() should allow reading and writing any nummeric fields of most context structures (like AVCodecContext or AVFormatContext) independant of their type or location in the struct

[jyavenard]

Replying to michael:

I have no real oppinion on it but IIRC there was noone who really argued to keep it.

I guess because I wasn't there :)

But that aside, i dont fully understand how this affects firefox. No distro which used FFmpeg had this compatibility mode enabled. If the binary packages from distros where "compatible enough" then that was by chance of only using a small subset of their features, there certainly was no compatibility when the mode wasnt enabled (and it was not enabled in any official packages from any distro AFAIK).

It seems that I need to give a little bit of background information on how firefox uses ffmpeg. I won't talk about distro compiling their own and however they are packaging it : this is not relevant to what I'm talking about.

I'm only referring to the binary anyone can download from mozilla.org web site (e.g. something like ​https://www.mozilla.org/firefox/new/?scene=2#download-fx)

We do not ship firefox with ffmpeg, nor do we recommend people install ffmpeg, we do not link against any ffmpeg libraries either.
When you start firefox and you attempt to play some media, it will check at what is available: gstreamer, flash, ffmpeg, whatever.

To detect if ffmpeg is available, we attempt to dlopen in order libavcodec-ffmpeg.so.56, libavcodec.so.56, libavcodec.so.55, libavcodec.so.54, libavcodec.so.53

That entire process of finding which version of ffmpeg is available is very hacky to be honest, but we haven't found a better one yet.

Once the library has been opened, we attempt to resolve the symbol avcodec_version and we query it to find out which version we have.
While the code we have was implemented in a manner that is as cross-compatible between all versions as possible, we obviously need different code for different version.

Note that I no time do we know if we're dealing with libav or FFmpeg ; and I'm yet to see a reliable method to do so that will work for all versions supported (anything between 53 and 56)

Now all of this has always worked, because all the binary structures we use are binary compatible between either libav or FFmpeg: that is AVCodecContext, AVFrame and AVPacket

So to decode a sample, the AVPacket and AVCodecContext we build will work no matter of which version we use (we have 3 different version of the same code: one for 53, on for 54 and one for 55 (as 56 works with the 55 code).

Now, should libav decide to not follow FFmpeg lead and use different size for some fields, that fundamental requirement will no longer work. We will not be able to create on the heap a AVCodecContext so that it works with any version of libavcodec (so long that they have the same major version number).

So now, I have two possible approaches to solve this issue: get the libAV folks to change those fields so AVCodecContext members, for the ones that we modify manually, are located in the same memory offset.
Or get you guys to revert that change.

Another possibility I guess would be for you to provide us with a reliable way to differentiate libavcodec from libav or the one from FFmpeg.
But then we suddenly have to support two forks: support across major version is already hard enough (we still have lots of people running ubuntu 12.04 for example, that came with LibAV's 0.8). At this stage there are still too many distros that ship with LibAV by default. The wind appears to be shifting (like Ubuntu/Debian now shipping FFmpeg's version instead)

As you can see, we do not care how a particular libavcodec was compiled, if it was compiled with an option or not. How we use it works across all versions, no matter the distribution.

Now, is it still possible to link and use both across distros, iam not sure it very well might be.
Most fields in AVCodecContext can be accessed through the AVOption API, see libavutil/opt.h
Is this sufficient for your use case ?

Going forward it may be. Is this something both forks support ?

Hope you can see where I'm coming from now.

And how all the "suggestions" I've received so far are totally irrelevant to us.

[jyavenard]

Replying to cehoyos:

Replying to jyavenard:

Please revert that commit

We cannot revert this commit, it would brake binary compatibility.

how so, as someone pointed out, 57 isn't set yet. I'm only looking into the future

There's already sufficient changes in 57 that pretty much dropped every API we were using (get_buffer, memory allocation etc..)

Do you realise how many distributions our single binary package support

Which distribution has the issue you are describing?

currently none have an issue, we work with them all (but none of them ship with anything > 2.8).
I'm only looking ahead, for when master becomes 2.10 (or 2.9?) or whatever and it starts to be packaged and distributed.

[Michael Niedermayer]

Replying to jyavenard:

Replying to michael:

[...]

Another possibility I guess would be for you to provide us with a reliable way to differentiate libavcodec from libav or the one from FFmpeg.

the micro version is 100 or larger in ffmpeg, avcodec_version() can be used to detect that, that will work with any half recent version. It wont work with really old versions

[...]

Now, is it still possible to link and use both across distros, iam not sure it very well might be.
Most fields in AVCodecContext can be accessed through the AVOption API, see libavutil/opt.h
Is this sufficient for your use case ?

Going forward it may be. Is this something both forks support ?

yes and i really think using the AVOption functions is a much more robust choice for the use case of providing a cross platform binary, than direct access.
Also while at it, i would suggest (in case you dont already do) that you might want to avoid directly using enum values and defined values as well, many of them are available through lookup from strings through avoption functions or as a random example for one enum av_get_pix_fmt_name() for pix formats.
C does not gurantee enums to have a specific type, nor do all values match between forks

[Cigaes]

Replying to jyavenard:

We will not be able to create on the heap a AVCodecContext

You have got a bug, right here.

Applications are not allowed to allocate AVCodecContext themselves. They have to use the library functions: avcodec_alloc_context3() for example. The size of AVCodecContext can (and does) change due to new fields added to the end without notice; if you allocate it using some kind of malloc(sizeof(AVCodecContext)) code, it will be too small for later versions of the library.

If you allocate using avcodec_alloc_context3() and access the field using av_opt_set_int() and av_opt_get_int(), your code do not have to know its exact size.

get the libAV folks to change those fields so AVCodecContext members

If, after updating your code to fix the API use, you still need that, I suggest you do just that. But I do not think this is necessary.

[jyavenard]

Replying to Cigaes:

Applications are not allowed to allocate AVCodecContext themselves. They have to use the library functions: avcodec_alloc_context3() for example. The size of AVCodecContext can (and does) change due to new fields added to the end without notice; if you allocate it using some kind of malloc(sizeof(AVCodecContext)) code, it will be too small for later versions of the library.

we do not, we use avcodec_alloc_context3.

Should have said AVPacket

[jyavenard]

Replying to michael:

yes and i really think using the AVOption functions is a much more robust choice for the use case of providing a cross platform binary, than direct access.

From which version is this supported?

[Michael Niedermayer]

av_opt_get_int / av_opt_set_int is in FFmpeg 0.9, if you want to support older, there is av_set_int() and av_get_int()

[Cigaes]

There is a av_packet_alloc() function too. The examples do not show it, because allocating AVPacket on the stack is still allowed (but probably not for long), but the function exists and works.

[jyavenard]

Replying to michael:

av_opt_get_int / av_opt_set_int is in FFmpeg 0.9, if you want to support older, there is av_set_int() and av_get_int()

Thank you very much for this.

The only remaining members accessed (that aren't int) are:
get_format, and extradata, get_buffer and release_buffer

But for those last two, I'd have to rethink anyway as they have been removed in 57

[Carl Eugen Hoyos]

Replying to jyavenard:

Replying to cehoyos:

Replying to jyavenard:

Please revert that commit

We cannot revert this commit, it would brake binary compatibility.

how so, as someone pointed out, 57 isn't set yet. I'm only looking into the future

You misunderstand the FFmpeg versioning.
We are at 57 already and it must not be changed (since more than a month).

Yesterday, I had missed that the original commit changing the type of bitrate fixed one (or actually two iirc) real-world bug reports, so I will close this ticket as wontfix since the commit in question definitely cannot be reverted.

There's already sufficient changes in 57 that pretty much dropped every API we were using (get_buffer, memory allocation etc..)

Do I understand correctly that this is unrelated?

Do you realise how many distributions our single binary package support

Which distribution has the issue you are describing?

currently none have an issue, we work with them all (but none of them ship with anything > 2.8).
I'm only looking ahead, for when master becomes 2.10 (or 2.9?) or whatever and it starts to be packaged and distributed.

That is what I mean: Which distribution will be an issue for you in the future (once newer releases are available)?

[jyavenard]

Could you recommend a way to set extra_data field on AVCodecContext?

How can this be done with the use of AVDictionary?

With the change of field size, we can't simply modify AVCodecContext::extra_data structure member.

[Carl Eugen Hoyos]

You could first check the version of the library and then use the version to decide where to find extradata.
But please allow me to repeat my question: On which distribution do you expect that this logic will be needed?

[jyavenard]

Replying to cehoyos:

You could first check the version of the library and then use the version to decide where to find extradata.
But please allow me to repeat my question: On which distribution do you expect that this logic will be needed?

while some distribution have reverted their decision to use LibAV and are now back to FFmpeg (in particular debian and ubuntu); some are still using LibAV.

I don't want to have to guess on who does what; we have a universal binary for all available distribution, it needs to work *everywhere*.

I'll probably end up doing what you're suggesting, but it's a major pain.

[jyavenard]

I should add that at this stage no distribution is shipping libavcodec 57 from the LibAV project, but one day they will do a release, and some people will use it.

[Carl Eugen Hoyos]

Replying to jyavenard:

Replying to cehoyos:

You could first check the version of the library and then use the version to decide where to find extradata.
But please allow me to repeat my question: On which distribution do you expect that this logic will be needed?

while some distribution have reverted their decision to use LibAV and are now back to FFmpeg (in particular debian and ubuntu); some are still using LibAV.

Could you point me to one of them?

[Carl Eugen Hoyos]

Replying to jyavenard:

I'll probably end up doing what you're suggesting, but it's a major pain.

Maybe it would be less pain for you to port the relevant patches to avconv? After all, bit_rate > INT32_MAX is not so unusual today. (As said, it was reported twice iirc.)

[jyavenard]

hmmm... it seems you're right... there's none.

Good, can probably drop libAv's libavcodec 57 support completely then