Hi,
On Wed, Oct 3, 2012 at 4:02 AM, Diego Biurrun <di...@biurrun.de> wrote:
> On Tue, Oct 02, 2012 at 05:48:26PM -0700, Ronald S. Bultje wrote:
>> 2) a lot of ff_ symbols, and dsputil_init, are accessed from outside
>> the library in which they exist. These are bugs and should be fixed (i.e.
>> renamed).
>
> You seem to be working on the wrong tree.
This is Chrome's tree.
>
>> --- a/configure
>> +++ b/configure
>> @@ -2616,7 +2616,7 @@ probe_cc(){
>> _flags_filter=msvc_flags
>> - _ld_lib='lib%.a'
>> + _ld_lib='%.lib'
>> _ld_path='-libpath:'
>
> Do static libs still work as expected?
Will test.
>> @@ -3078,11 +3078,13 @@ case $target_os in
>> shlibdir_default="$bindir_default"
>> SLIBPREF=""
>> SLIBSUF=".dll"
>> + LIBPREF=""
>> + LIBSUF=".lib"
>
> Tabs; fix your editor.
Yup, fixed.
> How does this affect normal gcc MinGW builds?
Haven't tested yet. It probably breaks it right now, I'll unbreak that later.
>> --- 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.
>
>> --- a/libavcodec/raw.h
>> +++ b/libavcodec/raw.h
>> @@ -28,13 +28,14 @@
>>
>> -extern const PixelFormatTag ff_raw_pix_fmt_tags[];
>> +extern AVCODEC_SYMBOL const PixelFormatTag ff_raw_pix_fmt_tags[];
>
> same
So, yes, but, these are used between libraries. E.g.
ff_raw_pix_fmt_tags is used in libavformat:
dhcp-172-31-81-183:ffmpeg rbultje$ grep ff_raw_pix_fmt_tags libavformat/*.c
libavformat/utils.c:
if(ff_find_pix_fmt(ff_raw_pix_fmt_tags, tag) == st->codec->pix_fmt)
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.
>> +#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).
>> +#ifndef AVUTIL_SYMBOLS_H
>> +#define AVUTIL_SYMBOLS_H
>> +
>> +#include "libavutil/avconfig.h"
>
> No need for the "libavutil/" prefix, it's the same directory.
It's generated, i.e. it lives in the build dir, not in the source dir,
so if you don't have the libavutil prefix, it won't build, since the
header isn't in the same dir (sourcedir vs. builddir) as the file that
included it. See other files including specifically avconfig.h, they
all use the libavutil/ prefix.
>> --- a/library.mak
>> +++ b/library.mak
>> @@ -94,7 +95,7 @@ endef
>>
>> -$(EXAMPLES) $(TESTPROGS) $(TOOLS): $(THIS_LIB) $(DEP_LIBS)
>> +#$(EXAMPLES) $(TESTPROGS) $(TOOLS): $(THIS_LIB) $(DEP_LIBS)
>> $(TESTPROGS): $(SUBDIR)$(LIBNAME)
>
> stray change
No, that's fully intentional, although it should be removed. 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. 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.
Ronald
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
