libavcodec: Don't use dllexport, only dllimport when building DLLs

diff mbox

The only purpose of dllexport (which is set while building the library
that exports the symbols) is to have the linker automatically
export such symbols into a DLL without using a def file - it doesn't
affect the generated code.

For MSVC builds, this isn't essential since we override what symbols
to export via an autogenerated def file instead.

Update a comment in configure to refer to the right concept.

With lld, this avoids warnings about duplicate export directives,
when some symbols are requested to be exported both via dllexport
attributes and via the autogenerated def file.

This also reduces the number of lines of code marginally.
---
 configure             | 2 +-
 libavcodec/internal.h | 6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

On 11/16/2017 5:22 AM, Martin Storsjö wrote:
> The only purpose of dllexport (which is set while building the library
> that exports the symbols) is to have the linker automatically
> export such symbols into a DLL without using a def file - it doesn't
> affect the generated code.
> 
> For MSVC builds, this isn't essential since we override what symbols
> to export via an autogenerated def file instead.
> 
> Update a comment in configure to refer to the right concept.
> 
> With lld, this avoids warnings about duplicate export directives,
> when some symbols are requested to be exported both via dllexport
> attributes and via the autogenerated def file.
> 
> This also reduces the number of lines of code marginally.
> ---
>  configure             | 2 +-
>  libavcodec/internal.h | 6 +-----
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/configure b/configure
> index 3bad7fb..62dffd4 100755
> --- a/configure
> +++ b/configure
> @@ -3883,7 +3883,7 @@ case $target_os in
>      mingw32*|mingw64*)
>          target_os=mingw32
>          if enabled shared; then
> -            # Cannot build both shared and static libs when using dllexport.
> +            # Cannot build both shared and static libs when using dllimport.
>              disable static
>          fi
>          check_ldflags -Wl,--nxcompat
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index da1b2fa..868e3df 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -285,12 +285,8 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame);
>   */
>  AVCPBProperties *ff_add_cpb_side_data(AVCodecContext *avctx);
>  
> -#if defined(_WIN32) && CONFIG_SHARED
> -#ifdef BUILDING_avcodec
> -#    define av_export_avcodec __declspec(dllexport)
> -#else
> +#if defined(_WIN32) && CONFIG_SHARED && !defined(BUILDING_avcodec)

Sorry if it's obvious, but when is BUILDING_avcodec not defined, for
that matter? library.mak seems to add -DBUILDING_avcodec to the
preprocessor flags of every libavcodec object.

>  #    define av_export_avcodec __declspec(dllimport)
> -#endif
>  #else
>  #    define av_export_avcodec
>  #endif
>

On Fri, 17 Nov 2017, James Almer wrote:

> On 11/16/2017 5:22 AM, Martin Storsjö wrote:
>> The only purpose of dllexport (which is set while building the library
>> that exports the symbols) is to have the linker automatically
>> export such symbols into a DLL without using a def file - it doesn't
>> affect the generated code.
>> 
>> For MSVC builds, this isn't essential since we override what symbols
>> to export via an autogenerated def file instead.
>> 
>> Update a comment in configure to refer to the right concept.
>> 
>> With lld, this avoids warnings about duplicate export directives,
>> when some symbols are requested to be exported both via dllexport
>> attributes and via the autogenerated def file.
>> 
>> This also reduces the number of lines of code marginally.
>> ---
>>  configure             | 2 +-
>>  libavcodec/internal.h | 6 +-----
>>  2 files changed, 2 insertions(+), 6 deletions(-)
>> 
>> diff --git a/configure b/configure
>> index 3bad7fb..62dffd4 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3883,7 +3883,7 @@ case $target_os in
>>      mingw32*|mingw64*)
>>          target_os=mingw32
>>          if enabled shared; then
>> -            # Cannot build both shared and static libs when using dllexport.
>> +            # Cannot build both shared and static libs when using dllimport.
>>              disable static
>>          fi
>>          check_ldflags -Wl,--nxcompat
>> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
>> index da1b2fa..868e3df 100644
>> --- a/libavcodec/internal.h
>> +++ b/libavcodec/internal.h
>> @@ -285,12 +285,8 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame);
>>   */
>>  AVCPBProperties *ff_add_cpb_side_data(AVCodecContext *avctx);
>> 
>> -#if defined(_WIN32) && CONFIG_SHARED
>> -#ifdef BUILDING_avcodec
>> -#    define av_export_avcodec __declspec(dllexport)
>> -#else
>> +#if defined(_WIN32) && CONFIG_SHARED && !defined(BUILDING_avcodec)
>
> Sorry if it's obvious, but when is BUILDING_avcodec not defined, for
> that matter? library.mak seems to add -DBUILDING_avcodec to the
> preprocessor flags of every libavcodec object.

When object files in libavformat includes this header, these variables 
will get the __declspec(dllimport) attribute - which is the whole reason 
why this is an issue at all.

Technically, this attribute means that instead of referring directly to 
the variables, there's an implicit variable __imp_<name> which is a 
pointer to the real one. So this means that libavformat object files will 
refer to __imp_avpriv_foo (which is provided by the avcodec DLL import 
library), while object files within avcodec themselves refer directly to 
the variable itself without this indirection.

// Martin

On 11/17/2017 5:35 PM, Martin Storsjö wrote:
> On Fri, 17 Nov 2017, James Almer wrote:
> 
>> On 11/16/2017 5:22 AM, Martin Storsjö wrote:
>>> The only purpose of dllexport (which is set while building the library
>>> that exports the symbols) is to have the linker automatically
>>> export such symbols into a DLL without using a def file - it doesn't
>>> affect the generated code.
>>>
>>> For MSVC builds, this isn't essential since we override what symbols
>>> to export via an autogenerated def file instead.
>>>
>>> Update a comment in configure to refer to the right concept.
>>>
>>> With lld, this avoids warnings about duplicate export directives,
>>> when some symbols are requested to be exported both via dllexport
>>> attributes and via the autogenerated def file.
>>>
>>> This also reduces the number of lines of code marginally.
>>> ---
>>>  configure             | 2 +-
>>>  libavcodec/internal.h | 6 +-----
>>>  2 files changed, 2 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 3bad7fb..62dffd4 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -3883,7 +3883,7 @@ case $target_os in
>>>      mingw32*|mingw64*)
>>>          target_os=mingw32
>>>          if enabled shared; then
>>> -            # Cannot build both shared and static libs when using
>>> dllexport.
>>> +            # Cannot build both shared and static libs when using
>>> dllimport.
>>>              disable static
>>>          fi
>>>          check_ldflags -Wl,--nxcompat
>>> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
>>> index da1b2fa..868e3df 100644
>>> --- a/libavcodec/internal.h
>>> +++ b/libavcodec/internal.h
>>> @@ -285,12 +285,8 @@ int ff_decode_frame_props(AVCodecContext *avctx,
>>> AVFrame *frame);
>>>   */
>>>  AVCPBProperties *ff_add_cpb_side_data(AVCodecContext *avctx);
>>>
>>> -#if defined(_WIN32) && CONFIG_SHARED
>>> -#ifdef BUILDING_avcodec
>>> -#    define av_export_avcodec __declspec(dllexport)
>>> -#else
>>> +#if defined(_WIN32) && CONFIG_SHARED && !defined(BUILDING_avcodec)
>>
>> Sorry if it's obvious, but when is BUILDING_avcodec not defined, for
>> that matter? library.mak seems to add -DBUILDING_avcodec to the
>> preprocessor flags of every libavcodec object.
> 
> When object files in libavformat includes this header, these variables
> will get the __declspec(dllimport) attribute - which is the whole reason
> why this is an issue at all.

Right, forgot this attribute was used in headers included from outside
libavcodec and not in c files.

Nevermind then. Should be good if confirmed working.

> 
> Technically, this attribute means that instead of referring directly to
> the variables, there's an implicit variable __imp_<name> which is a
> pointer to the real one. So this means that libavformat object files
> will refer to __imp_avpriv_foo (which is provided by the avcodec DLL
> import library), while object files within avcodec themselves refer
> directly to the variable itself without this indirection.
> 
> // Martin
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel