On 10 December 2017 at 02:41, Aaron Levinson <alevinsn_...@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 <mateu...@poczta.onet.pl>
>>>> wrote:
>>>>
>>>>> W dniu 07.12.2017 o 22:58, Hendrik Leppkes pisze:
>>>>>
>>>>>> Am 07.12.2017 20:40 schrieb "Mateusz" <mateu...@poczta.onet.pl>:
>>>>>>
>>>>>> W dniu 07.12.2017 o 10:42, Hendrik Leppkes pisze:
>>>>>>
>>>>>>> On Thu, Dec 7, 2017 at 2:02 AM, Mateusz <mateu...@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 <mateu...@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@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
