-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ABI breakage in OpenH264 2.3.0 #3564
Comments
CC @jrmuizel since this will block both GNOME and Fedora from updating to the new OpenH264. Sorry it took so long for us to notice. :( |
@GuangweiWang do you have an opinion on what we should do here? |
@YCSun-Meta as well? |
@mcatanzaro |
Can this be handled ASAP please? The current version, 2.3.0, is not safe to use due to this problem. |
@mcatanzaro |
Thanks! |
Oops, although this is fixed in 2.3.1 it is still not fixed on the master branch. Reopening. |
Fixed via #3576 |
OpenH264 2.3.0 has the same ABI version (soname) as 2.2.0, so there should not have been any ABI changes in this release. However, fac04ce changed the size of SEncParamExt. This is a problem because SEncParamExt is public ABI and it's allocated on the stack in a bunch of different applications. (Credit to Javier Jardon and Seppo Yli-Olli for discovering this.)
Anyway, using OpenH264 2.3.0 with applications built against 2.2.0 will surely result in crashes. Either the ABI changes should be reverted, or the ABI version should be bumped. Suffice to say that a corrected 2.3.1 release would be appreciated. Due to the license quagmire surrounding H.264, people using OpenH264 cannot patch it to fix this issue and have to wait for Cisco to release a fixed version, and so everyone will need to stick with 2.2.0 until a corrected release is available.
Note this has happened before and this problem looks almost exactly the same as the first time this has happened. It's rather weird that this has happened twice, because "it's not OK to change the size of public structs" is one of the most basic rules of C ABI. OpenH264 should consider ways to prevent this from happening a third time, e.g. running an ABI checker like abidiff as part of continuous integration, or making the structs opaque in a future ABI version and providing getter/setter functions to access them.
Previous example of how to bump ABI version. A corrected release should either (a) just bump this version, forcing applications to recompile, or (b) revert fac04ce (probably less-desirable since it looks like that is fixing a bug).
The text was updated successfully, but these errors were encountered: