[FFmpeg-devel] [PATCH] fix MSVC compilation errors

[Mateusz]

W dniu 05.12.2017 o 17:44, James Almer pisze:
> On 12/5/2017 1:40 PM, Mateusz wrote:
>> W dniu 05.12.2017 o 15:20, James Almer pisze:
>>> On 12/4/2017 8:31 PM, Mateusz wrote:
>>>> After some tests:
>>>> 1) #undef far
>>>> after #include <windows.h> is wrong -- in oleauto.h is declaration
>>>> WINOLEAUTAPI VarUI1FromI8(LONG64 i64In, _Out_ BYTE FAR* pbOut);
>>>> and 'FAR' is defined as 'far' which is define as empty.
>>>>
>>>> 2) #undef near
>>>> after #include <windows.h> works in ffmpeg but is danger -- see 1)
>>>>
>>>> 3) after
>>>> git revert 3701d499f8734ec5f3e7359de7311b15d92070b0
>>>> git revert 590136e78da3d091ea99ab5432543d47a559a461
>>>> and patch
>>>> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
>>>> -    MXFPackage packages[2] = {};
>>>> +    MXFPackage packages[2] = {{NULL}};
>>>> VS 2017 can compile ffmpeg and fate stops at audiomatch-nero-16000-mono-he-m4a
>>>> (without reverting 590136e hangs at api-flac-test.exe)
>>>>
>>>> 4) if for any reasons commits 3701d49 and 590136e shouldn't be reverted,
>>>> we can apply this patch and
>>>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>>>> index baf09119fe..b34a3803b8 100644
>>>> --- a/libavcodec/utils.c
>>>> +++ b/libavcodec/utils.c
>>>> @@ -1943,7 +1943,7 @@ int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op))
>>>>
>>>>  int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
>>>>  {
>>>> -    _Bool exp = 0;
>>>> +    atomic_bool exp = 0;
>>>>      if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init)
>>>>          return 0;
>>>>
>>>> @@ -1969,7 +1969,7 @@ int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
>>>>
>>>>  int ff_unlock_avcodec(const AVCodec *codec)
>>>>  {
>>>> -    _Bool exp = 1;
>>>> +    atomic_bool exp = 1;
>>>
>>> No, these are not meant to be atomic.
>>>
>>> _Bool is c99, so maybe we could replace its usage with int in these lock
>>> functions. The result will be the exact same.
>>
>> In atomic types the main thing is width (sizeof).
>> In atomic operations destination memory MUST BE sufficient
>> to copy all bytes from source, in
>> atomic_compare_exchange_strong(&ff_avcodec_locked, &exp, 1)
>> sizeof(exp) MUST BE >= sizeof(ff_avcodec_locked) == sizeof(atomic_bool)
>>
>> In ffmpeg implementation of atomic code for MSVC sizeof(atomic_bool) == 8
>> and sizeof(_Bool) < 8 -- it is the reason of hangs.
>>
>> And all volatile objects MUST BE explicitly volatile -- instead of
>> static atomic_bool ff_avcodec_locked;
>> should be
>> static volatile atomic_bool ff_avcodec_locked;
> 
> No, the win32 wrapper should add volatile to the atomic_* typedefs if
> anything. No other implementation needs it, so it must not be used here.

Yes, you are right -- it should work not only in Windows. It is a bit tricky...
In Linux stdatomic.h looks quite different than ffmpeg win32/gcc wrappers.

Now I have no opinion how it should be done right.

Mateusz