[Xvid-devel] Patch for XVID MPEG-4 DirectShow/MediaFoundation filter

[Michael Militzer]

Hi Jerker,

sorry, I didn't have time to look into the second part of your patch
earlier.

What I don't like is that it introduces platform and compiler (even
compiler version!) specific code into xvid.h. The main API header is not
really the place for this. Normally, such stuff is handled in portab.h.
Obviously however, it is not an option to include portab.h in xvid.h.

Therefore, what about this most minimal variant:

#ifndef XVID_IMPEXP
#define XVID_IMPEXP             extern
#endif

#ifndef XVID_CALL
#define XVID_CALL
#endif

This is neutral because it doesn't change anything over older versions of
xvid.h. At the same time, the defines can be conveniently populated
globally with whatever is needed for a certain platform or compiler from
the (also platform-/compiler-specific) Makefile. In addition, if XVID_CALL
must be explicitly set to __cdecl we'll have it defined in all source
files. So this would also solve the problem that not all source files
include xvid.h should we want to use XVID_CALL also for the internal
function pointers...

Regards,
Michael


Quoting Jerker Bäck <jerker.back at gmail.com>:

> Hi Edouard,
>
>> My bad
> OK, no problem
>
>> ... xvid
>> not going to work fine when forcing another calling convention during
>> build. Sadly your patch doesn't address that :-)
> I don't see that. To mark the exports as __cdecl would be consistent with
> past and future builds of xvidcore. Even if someone took the trouble to
> cream out some extra performance with a different calling convention, it
> would present the same consistent interface to the outside world (__cdecl).
>
> This is an improvement above the current implicit, unspoken fact that the
> exports actually are __cdecl. We humans may be able to assume that, but the
> compiler will be confused unless told exactly.
>
> --------------------------------------------------------------
>
> Here is another variant
>
> #if defined(XVID_BUILD_DLL) && (defined(_WIN32) || defined(__CYGWIN__) || \
>     defined(_MSC_VER))
> #define XVID_IMPEXP             __declspec(dllexport)
> #elif defined(XVID_BUILD_DLL) && (defined(__GNUC__) && (__GNUC__ >= 4))
> #define XVID_IMPEXP             __attribute__ ((visibility ("default")))
> #else
> #define XVID_IMPEXP             extern
> #endif
> #if defined(_WIN32) || defined(_MSC_VER)
> #define XVID_CALL               __cdecl
> #else
> #define XVID_CALL
> #endif
>
>
> The GCC visibility is taken from:
> http://gcc.gnu.org/wiki/Visibility
> Note: I have no experience of how GCC visibility actually works (pitfalls or
> special tweaks).
>
> The apparently weird "|| defined(_MSC_VER)" is due to the fact that the MS
> compiler may be used without the _WIN32 internal define (undefined for the
> POSIX subsystem).
>
> Regards
> Jerker
>
>
> _______________________________________________
> Xvid-devel mailing list
> Xvid-devel at xvid.org
> http://list.xvid.org/mailman/listinfo/xvid-devel
>
>