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

Aaron Levinson alevinsn_dev at levland.net
Tue Dec 12 01:28:30 EET 2017


On 12/9/2017 11:25 PM, Rostislav Pehlivanov wrote:
> On 10 December 2017 at 02:41, Aaron Levinson <alevinsn_dev at levland.net>
> wrote:
> 
>> On 12/9/2017 6:24 PM, Aaron Levinson wrote:
>>
>>> On 12/9/2017 6:15 PM, Aaron Levinson wrote:
>>>
>>>> On 12/9/2017 1:18 AM, Hendrik Leppkes wrote:
>>>>
>>>>> On Fri, Dec 8, 2017 at 8:49 AM, Mateusz <mateuszb at poczta.onet.pl>
>>>>> wrote:
>>>>>
>>>>>> W dniu 07.12.2017 o 22:58, Hendrik Leppkes pisze:
>>>>>>
>>>>>>> Am 07.12.2017 20:40 schrieb "Mateusz" <mateuszb at poczta.onet.pl>:
>>>>>>>
>>>>>>> W dniu 07.12.2017 o 10:42, Hendrik Leppkes pisze:
>>>>>>>
>>>>>>>> On Thu, Dec 7, 2017 at 2:02 AM, Mateusz <mateuszb at poczta.onet.pl>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> After commit 3701d49 'error_resilience: remove avpriv_atomic usage'
>>>>>>>>> we have included windows.h in much more files and we should
>>>>>>>>> avoid conflicts with defines/function declarations.
>>>>>>>>>
>>>>>>>>> We should declare compatible variables for atomic compat wrappers
>>>>>>>>> that expect fixed size variables in atomic_compare_exchange* macro.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Mateusz Brzostek <mateuszb at poczta.onet.pl>
>>>>>>>>> ---
>>>>>>>>>    libavcodec/jpegls.h   |  2 ++
>>>>>>>>>    libavcodec/mjpegdec.h |  2 ++
>>>>>>>>>    libavcodec/mss2.c     |  6 +++---
>>>>>>>>>    libavcodec/utils.c    | 12 ++++++++++++
>>>>>>>>>    libavformat/mxfenc.c  |  2 +-
>>>>>>>>>    5 files changed, 20 insertions(+), 4 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/libavcodec/jpegls.h b/libavcodec/jpegls.h
>>>>>>>>> index c8997c7861..6b89b2afa3 100644
>>>>>>>>> --- a/libavcodec/jpegls.h
>>>>>>>>> +++ b/libavcodec/jpegls.h
>>>>>>>>> @@ -32,6 +32,8 @@
>>>>>>>>>    #include "avcodec.h"
>>>>>>>>>    #include "internal.h"
>>>>>>>>>
>>>>>>>>> +#undef near /* This file uses struct member 'near' which in
>>>>>>>>> windows.h
>>>>>>>>>
>>>>>>>> is defined as empty. */
>>>>>>>
>>>>>>>> +
>>>>>>>>>    typedef struct JpeglsContext {
>>>>>>>>>        AVCodecContext *avctx;
>>>>>>>>>    } JpeglsContext;
>>>>>>>>> diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
>>>>>>>>> index c84a40aa6e..c36fba5f22 100644
>>>>>>>>> --- a/libavcodec/mjpegdec.h
>>>>>>>>> +++ b/libavcodec/mjpegdec.h
>>>>>>>>> @@ -39,6 +39,8 @@
>>>>>>>>>    #include "hpeldsp.h"
>>>>>>>>>    #include "idctdsp.h"
>>>>>>>>>
>>>>>>>>> +#undef near /* This file uses struct member 'near' which in
>>>>>>>>> windows.h
>>>>>>>>>
>>>>>>>> is defined as empty. */
>>>>>>>
>>>>>>>> +
>>>>>>>>>    #define MAX_COMPONENTS 4
>>>>>>>>>
>>>>>>>>>    typedef struct MJpegDecodeContext {
>>>>>>>>> diff --git a/libavcodec/mss2.c b/libavcodec/mss2.c
>>>>>>>>> index 9e7cc466de..3180af1d60 100644
>>>>>>>>> --- a/libavcodec/mss2.c
>>>>>>>>> +++ b/libavcodec/mss2.c
>>>>>>>>> @@ -464,9 +464,9 @@ static int decode_wmv9(AVCodecContext *avctx,
>>>>>>>>> const
>>>>>>>>>
>>>>>>>> uint8_t *buf, int buf_size,
>>>>>>>
>>>>>>>>        return 0;
>>>>>>>>>    }
>>>>>>>>>
>>>>>>>>> -typedef struct Rectangle {
>>>>>>>>> +struct Rectangle {
>>>>>>>>>        int coded, x, y, w, h;
>>>>>>>>> -} Rectangle;
>>>>>>>>> +};
>>>>>>>>>
>>>>>>>>>    #define MAX_WMV9_RECTANGLES 20
>>>>>>>>>    #define ARITH2_PADDING 2
>>>>>>>>> @@ -485,7 +485,7 @@ static int mss2_decode_frame(AVCodecContext
>>>>>>>>> *avctx,
>>>>>>>>>
>>>>>>>> void *data, int *got_frame,
>>>>>>>
>>>>>>>>
>>>>>>>>>        int keyframe, has_wmv9, has_mv, is_rle, is_555, ret;
>>>>>>>>>
>>>>>>>>> -    Rectangle wmv9rects[MAX_WMV9_RECTANGLES], *r;
>>>>>>>>> +    struct Rectangle wmv9rects[MAX_WMV9_RECTANGLES], *r;
>>>>>>>>>        int used_rects = 0, i, implicit_rect = 0,
>>>>>>>>> av_uninit(wmv9_mask);
>>>>>>>>>
>>>>>>>>>        if ((ret = init_get_bits8(&gb, buf, buf_size)) < 0)
>>>>>>>>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>>>>>>>>> index baf09119fe..70a0764714 100644
>>>>>>>>> --- a/libavcodec/utils.c
>>>>>>>>> +++ b/libavcodec/utils.c
>>>>>>>>> @@ -1943,7 +1943,13 @@ int av_lockmgr_register(int (*cb)(void
>>>>>>>>> **mutex,
>>>>>>>>>
>>>>>>>> enum AVLockOp op))
>>>>>>>
>>>>>>>>
>>>>>>>>>    int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
>>>>>>>>>    {
>>>>>>>>> +#if !defined(COMPAT_ATOMICS_DUMMY_STDATOMIC_H) &&
>>>>>>>>>
>>>>>>>> !defined(COMPAT_ATOMICS_PTHREAD_STDATOMIC_H) && \
>>>>>>>
>>>>>>>> +    !defined(COMPAT_ATOMICS_SUNCC_STDATOMIC_H) &&
>>>>>>>>>
>>>>>>>> !defined(COMPAT_ATOMICS_WIN32_STDATOMIC_H)
>>>>>>>
>>>>>>>>        _Bool exp = 0;
>>>>>>>>> +#else
>>>>>>>>> +    atomic_bool exp = 0;
>>>>>>>>> +#endif
>>>>>>>>> +
>>>>>>>>>        if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE ||
>>>>>>>>>
>>>>>>>> !codec->init)
>>>>>>>
>>>>>>>>            return 0;
>>>>>>>>>
>>>>>>>>> @@ -1969,7 +1975,13 @@ int ff_lock_avcodec(AVCodecContext *log_ctx,
>>>>>>>>>
>>>>>>>> const AVCodec *codec)
>>>>>>>
>>>>>>>>
>>>>>>>>>    int ff_unlock_avcodec(const AVCodec *codec)
>>>>>>>>>    {
>>>>>>>>> +#if !defined(COMPAT_ATOMICS_DUMMY_STDATOMIC_H) &&
>>>>>>>>>
>>>>>>>> !defined(COMPAT_ATOMICS_PTHREAD_STDATOMIC_H) && \
>>>>>>>
>>>>>>>> +    !defined(COMPAT_ATOMICS_SUNCC_STDATOMIC_H) &&
>>>>>>>>>
>>>>>>>> !defined(COMPAT_ATOMICS_WIN32_STDATOMIC_H)
>>>>>>>
>>>>>>>>        _Bool exp = 1;
>>>>>>>>> +#else
>>>>>>>>> +    atomic_bool exp = 1;
>>>>>>>>> +#endif
>>>>>>>>> +
>>>>>>>>>        if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE ||
>>>>>>>>>
>>>>>>>> !codec->init)
>>>>>>>
>>>>>>>>            return 0;
>>>>>>>>>
>>>>>>>>>
>>>>>>>> These ifdefs here are very ugly, and as mentioned in another mail,
>>>>>>>> the
>>>>>>>> atomics in those two functions arent even required - all access to
>>>>>>>> those variables is supposed to be protected by the lockmgr anyway.
>>>>>>>> So it would be easier to just remove any atomic nature of those
>>>>>>>> variables (or at the very lease replace the compare_exchange with a
>>>>>>>> store to solve this problem at hand).
>>>>>>>>
>>>>>>>
>>>>>>> I'm not sure but are you proposed to revert commit
>>>>>>> https://github.com/FFmpeg/FFmpeg/commit/590136e78da3d091ea99
>>>>>>> ab5432543d
>>>>>>> 47a559a461
>>>>>>>
>>>>>>>
>>>>>>> Basically, yes. Atomics are not needed for this variable, as access
>>>>>>> to it
>>>>>>> should be serialized anyways.
>>>>>>>
>>>>>>
>>>>>> OK for me. I've sent smaller patch (only near/Rectangle stuff).
>>>>>>
>>>>>>
>>>>> LGTM.
>>>>>
>>>>
>>>> This patch doesn't apply anymore to the latest code base due to changes
>>>> to mxfenc.c.
>>>>
>>>
>>> I should add that the changes to mxfenc.c aren't needed any longer. With
>>> the subpatch to mxfenc.c removed, the remain patch applies cleanly, and I
>>> can confirm that it fixes MSVC build issues as well.
>>>
>>> Aaron Levinson
>>>
>>
>> One more detail--while it builds with the patch altered as described
>> above, it doesn't work at run-time with MSVC builds without reverting patch
>> https://github.com/FFmpeg/FFmpeg/commit/590136e78da3d091ea99
>> ab5432543d47a559a461 as described in earlier comments.  So, when the
>> patch is finally committed, it would be helpful to revert the atomics patch
>> as well.
>>
>>
>> Aaron Levinson
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> 
> I submitted a patch to deal with this yet no one has reviewed it -
> http://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/221898.html

Hendrik reviewed it last Friday, and in his last response, from earlier 
today, he indicated that he would revert it unless he heard otherwise. 
In addition, I just provided my own comments in a response to the thread.

Aaron Levinson


More information about the ffmpeg-devel mailing list