[FFmpeg-devel] [PATCH] avcodec/libx264: don't define X264_API_IMPORTS when compiling static

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Timo
> Rothenpieler
> Sent: Friday, May 20, 2022 1:38 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define
> X264_API_IMPORTS when compiling static
> 
> On 20/05/2022 12:39, Soft Works wrote:
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Timo
> >> Rothenpieler
> >> Sent: Friday, May 20, 2022 12:18 PM
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx264: don't define
> >> X264_API_IMPORTS when compiling static
> >>
> >> On 20/05/2022 00:52, softworkz wrote:
> >>> From: softworkz <softworkz at hotmail.com>
> >>>
> >>> The definition of X264_API_IMPORTS is required for shared linking
> >>> (when MSVC is used) but it must not be defined in case of static
> >>> builds as is stated in x264.h:
> >>
> >> This doesn't seem right. It's about shared or static linking of
> >> libx264
> >> itself, not ffmpeg.
> >
> > How about some custom macro like DISABLE_X264_API_IMPORTS that one
> > can set when desired?
> >
> > In that case there wouldn't be any logical irritation.
> >
> 
> I'm still quite confused what the actual issue here is.
> Countless libraries ffmpeg depends on need those kind of macros to set
> the correct function import preamble.
> Why does x264 need special treatment? It correctly sets the desired
> flag
> via its pkg-config file.

The current code is 

#if defined(_MSC_VER)
#define X264_API_IMPORTS 1
#endif

Which means that this macro is always set then building with MSVC.
But the macro may only be set when linking to x264.dll, not 
when linking statically to libx264.
(pkg-config can't do anything about that)


This problem was introduced by this change in libx264:

https://code.videolan.org/videolan/x264/-/commit/a615f027ed172e2dd5380e736d487aa858a0c4ff#98b74dd0a8bf575bfdf90bbccf5142a555f06d4f_56_69


Previously, they had this line

#define X264_API __declspec(dllimport)

Which handled the situation automatically by checking whether
it's linked as dll or static lib.

But after that change, you are required to set this 
yourself (as a consumer).

But ffmpeg has no proper condition to set this only when
linking to x264.dll. That's why the current code is
essentially wrong:

#if defined(_MSC_VER)
#define X264_API_IMPORTS 1
#endif

And besides that, I don't think that those things belong into
a code file.


> Is this some "pkg-config does not exist with msvc" thing?


The "official" way of building ffmpeg with MSVC is to run
configure and make from MSYS2/MinGW which then only calls
cl.exe and link.exe from a Visual Studio installation.
In this case pkg-config is used.


I'm working with regular Visual Studio projects, though.
Even dependencies like libx264 are compiled in their own
VS projects.
There's no MSYS2, no make, no pkg-conf involved.


I _think_ that just nobody has ever tried to link libx264
statically when compiling in the "official way", which is
probably rarely used anyway and even more rare that somebody
would bother to link with x264 and once again even more rare
that the one would on top of that decide to link to libx264
statically. That's my guess why nobody has complained about
this during the past 3 years.


Kind regards,
softworkz