Hi,
On Wed, Oct 3, 2012 at 8:27 AM, Diego Biurrun <di...@biurrun.de> wrote:
> On Wed, Oct 03, 2012 at 07:37:29AM -0700, Ronald S. Bultje wrote:
>> >> --- a/libavcodec/golomb.h
>> >> +++ b/libavcodec/golomb.h
>> >> @@ -33,13 +33,14 @@
>> >>
>> >> -extern const uint8_t ff_golomb_vlc_len[512];
>> >> -extern const uint8_t ff_ue_golomb_vlc_code[512];
>> >> -extern const int8_t ff_se_golomb_vlc_code[512];
>> >> -extern const uint8_t ff_ue_golomb_len[256];
>> >> +extern AVCODEC_SYMBOL const uint8_t ff_golomb_vlc_len[512];
>> >> +extern AVCODEC_SYMBOL const uint8_t ff_ue_golomb_vlc_code[512];
>> >> +extern AVCODEC_SYMBOL const int8_t ff_se_golomb_vlc_code[512];
>> >> +extern AVCODEC_SYMBOL const uint8_t ff_ue_golomb_len[256];
>> >
>> > I somehow doubt marking ff_-prefixed symbols is necessary.
[..]
>> and the golomb symbols are used by the golomb fate test. Maybe we can
>> link in a bunch of other .c files from libavcodec instead of linking
>> to avcodec.dll instead (I agree that it's ugly). Note that gcc/mingw
>> shared lib build (see symbol list in libavcodec.v) also exports those
>> symbols.
>
> No:
>
> biurrun@passion:/opt/biurrun/libav $ git grep ff_.e_golomb_
> libavcodec/golomb.c:const uint8_t ff_ue_golomb_vlc_code[512]={
> libavcodec/golomb.c:const int8_t ff_se_golomb_vlc_code[512]={
> libavcodec/golomb.c:const uint8_t ff_ue_golomb_len[256]={
> libavcodec/golomb.h:extern const uint8_t ff_ue_golomb_vlc_code[512];
> libavcodec/golomb.h:extern const int8_t ff_se_golomb_vlc_code[512];
> libavcodec/golomb.h:extern const uint8_t ff_ue_golomb_len[256];
> libavcodec/golomb.h: return ff_ue_golomb_vlc_code[buf];
> libavcodec/golomb.h: return ff_ue_golomb_vlc_code[buf];
> libavcodec/golomb.h: return ff_se_golomb_vlc_code[buf];
> libavcodec/golomb.h: put_bits(pb, ff_ue_golomb_len[i], i+1);
> biurrun@passion:/opt/biurrun/libav $ git grep ff_golomb_vlc
> libavcodec/golomb.c:const uint8_t ff_golomb_vlc_len[512]={
> libavcodec/golomb.h:extern const uint8_t ff_golomb_vlc_len[512];
> libavcodec/golomb.h: LAST_SKIP_BITS(re, gb, ff_golomb_vlc_len[buf]);
> libavcodec/golomb.h: LAST_SKIP_BITS(re, gb, ff_golomb_vlc_len[buf]);
> libavcodec/golomb.h: LAST_SKIP_BITS(re, gb, ff_golomb_vlc_len[buf]);
dhcp-172-31-81-183:libav rbultje$ grep golomb libavcodec/golomb-test.c
#include "golomb.h"
set_ue_golomb(&pb, i);
j = get_ue_golomb(&gb);
[..]
set_ue_golomb(&pb, EXTEND(i));
j = get_ue_golomb_long(&gb);
[..]
set_se_golomb(&pb, i - COUNT / 2);
in golomb.h:
static inline int get_ue_golomb(GetBitContext *gb){
unsigned int buf;
int log;
OPEN_READER(re, gb);
UPDATE_CACHE(re, gb);
buf=GET_CACHE(re, gb);
if(buf >= (1<<27)){
buf >>= 32 - 9;
LAST_SKIP_BITS(re, gb, ff_golomb_vlc_len[buf]);
CLOSE_READER(re, gb);
return ff_ue_golomb_vlc_code[buf];
}else{
[..]
Maybe Libav fixed the raw pix fmt stuff, but the point is that we
unfortunately do use internal test symbols in various places.
>> >> +#ifndef AVCODEC_SYMBOLS_H
>> >> +#define AVCODEC_SYMBOLS_H
>> >> +
>> >> +#include "libavutil/avconfig.h"
>> >> +
>> >> +#if AV_HAVE_SHARED_LIBS && defined(_MSC_VER) &&
>> >> !defined(COMPILING_avcodec)
>> >
>> > What's AV_HAVE_SHARED_LIBS? I'm not aware of it anywhere.
>> > What do you need avconfig.h for?
>>
>> These AVCODEC_SYMBOL things will be used in public headers, thus the
>> definition needs t exist in a public header. Since I need to use
>> __declspec(dllimport) only for .dll (shared) builds, not for .lib
>> (static) builds, I need to know in a public header whether I'm a
>> shared lib or not - hence AV_HAVE_SHARED_LIBS (I added that to
>> configure, based on CONFIG_SHARED).
>
> The configure change you speak about is missing then; nothing adds
> that define to avconfig.h ...
You're right, so basically I added shared_libs to HAVE_LIST_PUB and
set its value to $shared just before printing it into avconfig.h, so
it always has the same value as CONFIG_SHARED. Sorry for the
confusion.
>> So, what this does, is that it adds DEP_LIBS (plus unrelated stuff)
>> as a dependency to TESTPROGS, i.e. *.dll becomes a dep for e.g.
>> dct-test.exe. If you look at the link rule for TESTPROGS, it uses
>> $$< to link in all listed dependencies.
>
> No, that rule uses $$^, which has different semantics.
>
>> Now note that you shouldn't link
>> against .dll files on Windows, you generate .lib files that contain
>> the linker information to link against .dll files (but .lib without
>> .dll is a static build, sorry if this is confusing). The .lib files
>> are already part of FFEXTRALIBS. So what you end up getting is a
>> linker that tries to link against .dll files directly (link
>> -libpath:libavcodec -out:dct-test.exe dct-test.o [other object files]
>> libavcodec/libavcodec.dll [other dlls] libavcodec.lib [other libs]
>> [external lib dependencies]), which doesn't make any sense, and
>> obviously fails. This change removes the dependency on the .dll as a
>> listed dep, and thus instead it just links like "link
>> -libpath:libavcodec -out:dct-test.exe dct-test.o [other object files]
>> libavcodec.lib [other libs] [external lib dependencies]", which works
>> as expected.
>
> Does it pass "make check"? I'm suspicious that you may have broken some
> test programs - some of them need to directly link against the libs
> because they access internal symbols...
This chang _fixed_ the test programs rather than breaking them (e.g.
dct-test.exe, like I said).
Ronald
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
