[FFmpeg-devel] [PATCH] restoring binary compatibility with ffmpeg 0.5

Reinhard Tartler siretart
Thu Jun 10 09:58:36 CEST 2010


On Wed, Jun 09, 2010 at 19:41:33 (CEST), Michael Niedermayer wrote:

> On Wed, Jun 09, 2010 at 03:42:13PM +0200, Reinhard Tartler wrote:
>> On Wed, Jun 09, 2010 at 15:10:18 (CEST), Michael Niedermayer wrote:
>> 
>> > On Wed, Jun 09, 2010 at 06:23:36AM +0200, Reinhard Tartler wrote:
>> >> On Wed, Jun 09, 2010 at 03:08:29 (CEST), Michael Niedermayer wrote:
>> >> 
>> >> > On Mon, Jun 07, 2010 at 01:11:41PM +0200, Reinhard Tartler wrote:
>> >> >> On Mo, Jun 07, 2010 at 12:52:14 (CEST), Michael Niedermayer wrote:
>> >> >> 
>> >> >> > On Mon, Jun 07, 2010 at 09:42:42AM +0200, Reinhard Tartler wrote:
>> >> >> >> On Mo, Jun 07, 2010 at 08:02:54 (CEST), Reimar D?ffinger wrote:
>> >> >> >> 
>> >> >> >> > On Mon, Jun 07, 2010 at 07:52:11AM +0200, Reinhard Tartler wrote:
>> >> >> >> >> void av_init_packet(AVPacket *pkt) av_weak_alias(av_init_packet);
>> >> >> >> >> void av_init_packet(AVPacket *pkt)
>> >> >> >> >> {
>> >> >> >> >>     av_log(NULL, AV_LOG_WARNING, "diverting av_*_packet function calls to libavcodec. Recompile to improve performance\n");
>> >> >> >> >>     av_init_packet(pkt);
>> >> >> >> >
>> >> >> >> > ff_internal_init_packet() and add one such to lavc.
>> >> >> >> > Either way, we should make sure we have a solution the next time.
>> >> >> >> > Since the @LIBAVFORMAT version is not accepted in lavc, does that
>> >> >> >> > mean no matter what we do, we will always break ABI if we move code?!
>> >> >> >> 
>> >> >> >> if I understand you correctly, you not only consider ABI breakages
>> >> >> >> between releases, but also between any svn revision? 
>> >> >> >
>> >> >> > i do
>> >> >> 
>> >> >> OK. I agree that we should unneccessarily do such breakages, not even in
>> >> >> trunk nor elsewhere.
>> >> >> 
>> >> >> >
>> >> >> >> Then I fear yes.
>> >> >> >> However, the break is already there since quite some time, and fixing it
>> >> >> >> to have it compatible to ffmpeg 0.5 has (or at least should have)
>> >> >> >> priority, IMO.
>> >> >> >
>> >> >> > for future moves, is there a problem with moving the symbols and
>> >> >> > updating the version script in the new home so it matches the version
>> >> >> > of the old (@LIBAVFORMAT in lavc for the specific symbols)
>> >> >> > ?
>> >> >> 
>> >> >> We could, but this wouldn't help. As you already noticed, ld-linux.so
>> >> >> obviously really expects the versioned symbol to be in libavformat:
>> >> >
>> >> > another try
>> >> > assume
>> >> > foobar at libavformat1 in libavformat
>> >> 
>> >> that would be 'foobar@@libavformat1'. Symbols by the linker script
>> >> always get tagged as 'default' symbol.
>> >
>> > fine, so s/@/@@/g
>> >
>> >
>> >> 
>> >> > to move:
>> >> > add foobar at libavformat1 in libavcodec
>> >> > mark foobar at libavformat1 in libavformat as weak symbol
>> >> > and put the whole foobar in libavformat under #if
>> >> > does this work?
>> >> > i think it should, ld.so favors globals over weak
>> >> 
>> >> we are talking about public symbols only here. Since libavformat always
>> >> includes libavcodec's public headers, you will always get redeclarations
>> >> errors for the offending function.
>> >
>> > void functA();
>> >
>> > void __attribute__((weak)) functA(){
>> >     printf(" libA0 ");
>> > }
>> 
>> Two questions:
>>  a) doesn't this attribute need to be at symbol declaration and not
>>     definition?
>
> The way i understand it, it needs to be at the definition
> at the declaration it possibly means a weak refernce which is something
> that is 0 (instead of failing to link) if no definition is found

I see.

I've now tried your suggestion to (re)introduce the functions like you
propose:

#if LIBAVFORMAT_VERSION_MAJOR < 53
[...]
void __attribute__((weak)) av_init_packet(AVPacket *pkt)
{
     abort();
}
#endif

building a shared libavformat.so works without warnings/errors regarding
symbol collisions and results this

$ readelf -s /tmp/ffmpeg-trunk/lib/libavformat.so | grep av_init_packet@
   466: 000abc00     8 FUNC    WEAK   DEFAULT   13 av_init_packet@@LIBAVFORMAT_52

I guess this is exactly what you've expected here.

With this setup, I've now tried both the ffplay binary from this
compilation and the system /usr/bin/ffplay (based on version 0.5.2). In
*both* cases, it seems that the binary at runtime picks up the version
in libavformat:

>> readelf -s /tmp/ffmpeg-trunk/bin/ffplay | grep av_init_packet
    49: 00000000     0 FUNC    GLOBAL DEFAULT  UND av_init_packet at LIBAVFORMAT_52 (10)


>> /tmp/ffmpeg-trunk/bin/ffplay /tmp/foo.avi
FFplay version SVN-r23556, Copyright (c) 2003-2010 the FFmpeg developers
  built on Jun 10 2010 08:09:17 with gcc 4.4.3
  configuration: --enable-avfilter --enable-libvpx --enable-shared --disable-static --enable-gpl --prefix=/tmp/ffmpeg-trunk --enable-postproc --enable-libvorbis
  libavutil     50.18. 0 / 50.18. 0
  libavcodec    52.75. 1 / 52.75. 1
  libavformat   52.68. 0 / 52.68. 0
  libavdevice   52. 2. 0 / 52. 2. 0
  libavfilter    1.20. 0 /  1.20. 0
  libswscale     0.11. 0 /  0.11. 0
  libpostproc   51. 2. 0 / 51. 2. 0
zsh: abort      /tmp/ffmpeg-trunk/bin/ffplay /srv/media/UDS-Sevilla.avi

>
>>  b) How do you call the 'non-weak' symbol in lavc? Or do you propose to
>>     duplicate code? 
>
> I do not know in which cases the symbol can be called
> If it cant ever then an empty function with assert(0) should do.

Well, my original patch used dlopen() to force using the symbols from
libavcodec.so.52. This should work in any case, but I guess this is not
what we want here.

> If it can be called i guess we have to try what works in that case
> code duplication is an option though if all else fails

With my observation above, this would effectively mean to move the
symbols back, which we don't want either.

>> > compiled with -W -Wall -pedantic shows no errors and no warnings
>> > (it would be a bug imo if it did)
>> >
>> >
>> >>  Putting them in a seperate compilation unit that does not include
>> >> libavcodec's public headers won't work either, because the
>> >> implementation of these compat symbols would just call the now moved
>> >> symbols.
>> >
>> > that would depend on the implementation i guess, also strictly speaking
>> > nothing should ever call the weak symbols, they are overridden by global
>> > symbols. (iam not 100% sure abut -Bsymbolic and vissibility effects on this)
>> 
>> I'm pretty sure that it does have effects.
>
> then at least the documenattion of -Bsymbolic is wrong as it speaks about
> global symbols and weak symbols and global symbols are distinct in elf.
>
>
>> 
>> >> 
>> >> IMO the proper solution for this scenario is to
>> >> 
>> >> add  foobar@@libavcodec1 in libavcodec
>> >> add  ff_compat_foobar@@libavformat1 in libavformat that just calls foobar at libavcodec1
>> >> add  foobar at libavformat1 as alias in libavformat
>> >> 
>> >> NB: the syntax '@@' vs. '@'.
>> >> 
>> >> I still claim that step 3 is supported by all relevant platforms, even
>> >> ARM RVCT. Any counterexamples?
>> >
>> > your solution adds overhead aka it slows the code down
>> 
>> That's why my proposed dummy functions emit a runtime warning that
>> suggests to recompile the application for performance reasons.
>
> one warning for each call IIRC

No, only in av_packet_init, this makes the amount of spamming
acceptable.  For this case.

> btw, the average user of a distro package might be confused by a suggestion
> to recompile

Indeed, but this seems to me as an acceptable compromise.

-- 
Gruesse/greetings,
Reinhard Tartler, KeyID 945348A4




More information about the ffmpeg-devel mailing list