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/590136e78da3d091ea99ab5432543d
>>>>> 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/590136e78da3d091ea99ab5432543d47a559a461
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