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

[wm4]

On Tue, 5 Dec 2017 22:38:11 +0100
Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Tue, Dec 5, 2017 at 8:23 PM, James Almer <jamrial at gmail.com> wrote:
> > On 12/5/2017 8:12 AM, Hendrik Leppkes wrote:  
> >> On Tue, Dec 5, 2017 at 12:31 AM, Mateusz <mateuszb at poczta.onet.pl> 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.  
> >>
> >> Yeah generally undefing all of them might result in errors in other
> >> windows headers, so thats bad.
> >>  
> >>>
> >>> 2) #undef near
> >>> after #include <windows.h> works in ffmpeg but is danger -- see 1)  
> >>
> >> Doing it rather late,  ie. right before its used in the file should be
> >> relatively safe. If not, we can always rename the variable.
> >> But it seems to work so far. And from the nature of this bug, it
> >> should just error out instead of resulting in other breakage - if it
> >> happens.
> >>  
> >>>
> >>> 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  
> >>
> >> We can't really revert those, so fixing them is better.
> >>  
> >>> 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;
> >>>      if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init)
> >>>          return 0;
> >>>
> >>>  
> >>
> >> atomic_compare_exchange* usage is a bit iffy because it uses pointers,
> >> and the compat wrappers don't fully represent all types properly (for
> >> simplicity, implementing them in a generic way would be madness).
> >> So this work-around seems fine to me, if it builds with GCC as well.  
> >
> > GCC may accept it (No idea what code it would generate, though), but
> > chances are Clang will complain since it's stricter about correct type
> > usage in these functions, and making the variable to be passed as the
> > "expected" parameter atomic is not correct.
> >  
> 
> Why does this code use atomics anyway?
> The actual locking is done using a lock manager, and it warns/errors
> when the lock functions are used concurrently without locking.
> 
> So, perhaps we should just remove that?

Well, in the mid-range, we could just drop all lock manager and atomics
code by managing the codec list like the BSF list, and finishing the
codec static init cleanup.