[FFmpeg-devel] [PATCH 1/8] Deprecate avcodec_get_pix_fmt_name() in favor of the new av_get_pix_fmt_name().

[Tomas Härdin]

On Mon, 2010-11-08 at 18:41 -0200, Ramiro Polla wrote:
> On Mon, Nov 8, 2010 at 12:43 PM, Tomas H?rdin <tomas.hardin at codemill.se> wrote:
> > Basically what needs to be done for that is have the import libraries
> > also export global variables and not just the functions, thus exposing
> > the entire API. That or add getter functions for all global variables
> > (basically make our own stubs).
> 
> Global variables are always exported. The problem is that MSVC is not
> capable of using globals from a DLL unless they're marked as
> __declspec(dllimport). I consider that a bug, not worth working around
> on our code with dllimports and such. Also I don't like the API
> clutter of adding getters for each global variable (specially since
> that's only a problem when linking to shared libraries with MSVC), but
> that's not up to me to decide.
> 
> I'll try to give a more in-depth explanation of the issue regarding
> dllimport to clear up any confusion:
> When linking statically (without dllimport), MSVC knows the address
> the variable will end up in, so it emits:
> mov    <address of variable>,%eax
> DLLs export pointers to the variables. When linking against DLLs, MSVC
> doesn't know the address the variable will end up in. It will fail to
> link if you haven't used dllimport. If you do use dllimport, it keeps
> a pointer in a known location, and when it loads the DLL it updates
> that pointer. The linker then emits:
> mov    <address of pointer>,%eax
> mov    (%eax),%ecx
> But if you use dllimport while compiling statically, it will also emit
> the 2 movs instead of one (which is a bad thing).
> 
> GCC linking statically (without dllimport) also knows the location of
> the data and emits only one mov. GCC linking to a DLL (without
> dllimport) uses a trick that updates all occurrences of the variable
> in the app, so it doesn't need to keep a pointer and use the variable
> indirectly like MSVC does. It will then also use only one mov.
> If compiled with dllimport, GCC will try to look for the indirect
> pointer variable (_imp__<variable name>) to issue 2 movs. If linking
> to a DLL, that will work, but if linking to a static library it will
> not find the indirect variable and will fail to link, and even if it
> did work, it would also have an unnecessary mov.
> 
> Summing up:
> if we don't use dllimport, we get that the shared MSVC link will fail.
> If we use dllimport under #ifdef _WIN32, we get that the static GCC
> link will fail, and the static MSVC build will have an useless
> indirection.
> If we use dllimport under #ifdef _MSC_VER, we get that the static MSVC
> build will have an useless indirection.

Thanks, that clears up a lot of confusion.

> One option would be to export to avconfig.h if the build was
> shared-only, and use "#if defined(_MSC_VER) && BUILD_SHARED" to set
> __declspec(dllimport). This has a few problems:
> 1. It is possible to build both shared and static libraries at the
> same time, so the define would be wrong with either the shared or the
> static library when used under MSVC.
> 2. Installed headers would be differ between builds possibly for the
> same library (only static, or static+shared), which is not good.
> 3. It would add hacks to our configure and headers, and we don't like hacks.
> Point 1 could be worked around if we explicitly asked users to add
> some configure switch to enable this, which is an even worse hack.

This sounds pretty much like what we figured out previously. There's no
proper way to get global variables imported in a cross platform and
static+shared way. A good thing to keep in mind for the future.

> Another option would be to add an user-defined macro and leave it up
> to the MSVC user with shared builds to define it prior to including
> the headers, like:
> #ifndef DLLIMPORT
> #define DLLIMPORT
> #endif
> extern DLLIMPORT int blah;
> As a downside, this also adds a hack for us, which even if documented
> I don't know how many people would find and use.

The user could add a hack in their code such as what I've done, as long
as they always link dynamically. That way they don't have to patch any
headers:

#ifdef WIN32
#define av_pix_fmt_descriptors av_pix_fmt_descriptors_foo
#endif

#include <libavutil/pixdesc.h>

#ifdef WIN32
#undef av_pix_fmt_descriptors
__declspec(dllimport) const AVPixFmtDescriptor av_pix_fmt_descriptors[];
#endif

> And finally, if someone has to go through all the trouble of looking
> up a macro, a configure switch, or adapting to any hack in our
> codebase, he can instead go through the trouble of updating the header
> files for his own app (which is the current documented and suggested
> way).
> 
> Maybe there is a simple solution and I just don't see it. If someone
> finds that solution, please send us a (clean) patch! Otherwise I don't
> think it's worth going on and on about this issue...

I can't really think of any pretty solution either. But: desiring access
to av_pix_fmt_descriptors is probably fairly common, so adding said
getter for it is probably a good idea. It's also safer, as mentioned
elsewhere in this thread. Thus my support for adding such a function.

The rest of the global variables probably aren't particularly
interesting, but for the record the last time I look they were:
ff_log2_tab, av_reverse, av_md5_size and av_sha1_size. Unless I'm
mistaken, the last three are part of the public API. If not, they should
be labeled as such.

Anyway, I'm not sure if continuing this discussion will be very
fruitful. I think we've exhausted most of what can be said about this.

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101109/177df9ea/attachment.pgp>