| 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 | expand |
| 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 |
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
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 --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);
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(-)