diff mbox series

[FFmpeg-devel] avcodec/put_bits: Restore x64 ABI compatibility with releases <= 4.3

Message ID 20210322063507.818154-1-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel] avcodec/put_bits: Restore x64 ABI compatibility with releases <= 4.3
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt March 22, 2021, 6:35 a.m. UTC
88d80cb97528d52dac3178cf5393d6095eca6200 changed the type of
PutBitContext.BitBuf to uint64_t; it used to be an uint32_t.
While said structure is not public, it is nevertheless used by
certain avpriv functions and therefore crosses library boundaries:
avpriv_align_put_bits and avpriv_copy_bits were used in other libraries
in release 4.3 (and at the time of 88d80cb9) and so this commit broke
ABI.

This commit mitigates the trouble caused by this by using an uint32_t
again, but only for the 4.4 release branch and not the master branch,
as doing so for master, would break the ABI of master again, although
it is very unlikely that anyone would be helped by this (there don't
seem to be any users that combine libavcodec built from master and
libavformat from an old release: otherwise we would have received bug
reports about said ABI break).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Important: This is only intended for the 4.4 release branch.

 libavcodec/put_bits.h | 7 -------
 1 file changed, 7 deletions(-)

Comments

James Almer March 22, 2021, 2:32 p.m. UTC | #1
On 3/22/2021 3:35 AM, Andreas Rheinhardt wrote:
> 88d80cb97528d52dac3178cf5393d6095eca6200 changed the type of
> PutBitContext.BitBuf to uint64_t; it used to be an uint32_t.
> While said structure is not public, it is nevertheless used by
> certain avpriv functions and therefore crosses library boundaries:
> avpriv_align_put_bits and avpriv_copy_bits were used in other libraries
> in release 4.3 (and at the time of 88d80cb9) and so this commit broke
> ABI.
> 
> This commit mitigates the trouble caused by this by using an uint32_t
> again, but only for the 4.4 release branch and not the master branch,
> as doing so for master, would break the ABI of master again, although
> it is very unlikely that anyone would be helped by this (there don't
> seem to be any users that combine libavcodec built from master and
> libavformat from an old release: otherwise we would have received bug
> reports about said ABI break).

So basically, this would only be a problem if your application loads 
lavf <= 4.3 (that was linked to lavc <= 4.3) alongside lavc 4.4 at 
runtime, right?

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> Important: This is only intended for the 4.4 release branch.
> 
>   libavcodec/put_bits.h | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
> index cd66a82a44..f07944a8fb 100644
> --- a/libavcodec/put_bits.h
> +++ b/libavcodec/put_bits.h
> @@ -35,16 +35,9 @@
>   
>   #include "version.h"
>   
> -#if ARCH_X86_64
> -// TODO: Benchmark and optionally enable on other 64-bit architectures.
> -typedef uint64_t BitBuf;
> -#define AV_WBBUF AV_WB64
> -#define AV_WLBUF AV_WL64
> -#else
>   typedef uint32_t BitBuf;
>   #define AV_WBBUF AV_WB32
>   #define AV_WLBUF AV_WL32
> -#endif
>   
>   static const int BUF_BITS = 8 * sizeof(BitBuf);

LGTM
Andreas Rheinhardt March 22, 2021, 7:02 p.m. UTC | #2
James Almer:
> On 3/22/2021 3:35 AM, Andreas Rheinhardt wrote:
>> 88d80cb97528d52dac3178cf5393d6095eca6200 changed the type of
>> PutBitContext.BitBuf to uint64_t; it used to be an uint32_t.
>> While said structure is not public, it is nevertheless used by
>> certain avpriv functions and therefore crosses library boundaries:
>> avpriv_align_put_bits and avpriv_copy_bits were used in other libraries
>> in release 4.3 (and at the time of 88d80cb9) and so this commit broke
>> ABI.
>>
>> This commit mitigates the trouble caused by this by using an uint32_t
>> again, but only for the 4.4 release branch and not the master branch,
>> as doing so for master, would break the ABI of master again, although
>> it is very unlikely that anyone would be helped by this (there don't
>> seem to be any users that combine libavcodec built from master and
>> libavformat from an old release: otherwise we would have received bug
>> reports about said ABI break).
> 
> So basically, this would only be a problem if your application loads
> lavf <= 4.3 (that was linked to lavc <= 4.3) alongside lavc 4.4 at
> runtime, right?
> 

lavfi also used avpriv_align_put_bits up until
8129c32e488645db325263a6bee01311b83e7ed9, so lavfi <= 4.3 is dangerous,
too, if combined with lavc 4.4.

>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>> Important: This is only intended for the 4.4 release branch.
>>
>>   libavcodec/put_bits.h | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
>> index cd66a82a44..f07944a8fb 100644
>> --- a/libavcodec/put_bits.h
>> +++ b/libavcodec/put_bits.h
>> @@ -35,16 +35,9 @@
>>     #include "version.h"
>>   -#if ARCH_X86_64
>> -// TODO: Benchmark and optionally enable on other 64-bit architectures.
>> -typedef uint64_t BitBuf;
>> -#define AV_WBBUF AV_WB64
>> -#define AV_WLBUF AV_WL64
>> -#else
>>   typedef uint32_t BitBuf;
>>   #define AV_WBBUF AV_WB32
>>   #define AV_WLBUF AV_WL32
>> -#endif
>>     static const int BUF_BITS = 8 * sizeof(BitBuf);
> 
> LGTM
diff mbox series

Patch

diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
index cd66a82a44..f07944a8fb 100644
--- a/libavcodec/put_bits.h
+++ b/libavcodec/put_bits.h
@@ -35,16 +35,9 @@ 
 
 #include "version.h"
 
-#if ARCH_X86_64
-// TODO: Benchmark and optionally enable on other 64-bit architectures.
-typedef uint64_t BitBuf;
-#define AV_WBBUF AV_WB64
-#define AV_WLBUF AV_WL64
-#else
 typedef uint32_t BitBuf;
 #define AV_WBBUF AV_WB32
 #define AV_WLBUF AV_WL32
-#endif
 
 static const int BUF_BITS = 8 * sizeof(BitBuf);