[FFmpeg-devel] [FFmpeg-cvslog] avformat/mxfdec: fix error check in macro

[Marton Balint]

On Sat, 22 Dec 2018, Michael Niedermayer wrote:

> On Sat, Dec 22, 2018 at 01:02:17PM -0300, James Almer wrote:
>> On 12/22/2018 12:50 PM, Michael Niedermayer wrote:
>>> On Sat, Dec 15, 2018 at 10:18:18AM +0100, Hendrik Leppkes wrote:
>>>> On Sat, Dec 15, 2018 at 9:50 AM Paul B Mahol <onemda at gmail.com> wrote:
>>>>>
>>>>> On 12/15/18, James Almer <jamrial at gmail.com> wrote:
>>>>>>> ffmpeg | branch: master | Paul B Mahol <onemda at gmail.com
>>>>>>> <http://ffmpeg.org/mailman/listinfo/ffmpeg-cvslog>> | Thu Dec 13 23:51:02
>>>>>>> 2018 +0100| [e5a0013c4a6248fe7e2a651db1fda6b9bb2cd743] | committer: Paul B
>>>>>>> Mahol
>>>>>>>
>>>>>>> avformat/mxfdec: fix error check in macro
>>>>>>>
>>>>>>> Fixes #6750.
>>>>>>>
>>>>>>>> /http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=e5a0013c4a6248fe7e2a651db1fda6b9bb2cd743
>>>>>>> /---
>>>>>>>
>>>>>>>  libavformat/mxfdec.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>>>>>>> index 887645d28b..f5e3a736e5 100644
>>>>>>> --- a/libavformat/mxfdec.c
>>>>>>> +++ b/libavformat/mxfdec.c
>>>>>>> @@ -2590,7 +2590,7 @@ static int64_t mxf_timestamp_to_int64(uint64_t
>>>>>>> timestamp)
>>>>>>>
>>>>>>>  #define SET_TS_METADATA(pb, name, var, str) do { \
>>>>>>>      var = avio_rb64(pb); \
>>>>>>> -    if ((ret = avpriv_dict_set_timestamp(&s->metadata, name,
>>>>>>> mxf_timestamp_to_int64(var)) < 0)) \
>>>>>>> +    if ((ret = avpriv_dict_set_timestamp(&s->metadata, name,
>>>>>>> mxf_timestamp_to_int64(var))) < 0) \
>>>>>>>          return ret; \
>>>>>>>  } while (0)
>>>>>>
>>>>>> This broke several mxf tests.
>>>>>>
>>>>>> copy-trac4914
>>>>>> lavf-mxf
>>>>>> lavf-mxf_d10
>>>>>> lavf-mxf_dv25
>>>>>> lavf-mxf_dvcpro50
>>>>>> lavf-mxf_opatom
>>>>>> lavf-mxf_opatom_audio
>>>>>
>>>>> Can not reproduce. Is this MSVS only thing?
>>>>
>>>> Its not MSVS, but apparently Windows. I wonder if either gmtime_r or
>>>
>>> this reproduces on mingw64+wine too btw
>>>
>>> @@ -1,49 +1,2 @@
>>>  b37c4d5693cdb5b9ed9b33501ffb682a *tests/data/fate/copy-trac4914.mxf
>>>  561721 tests/data/fate/copy-trac4914.mxf
>>> -#tb 0: 1001/30000
>>> -#media_type 0: video
>>> -#codec_id 0: rawvideo
>>> -#dimensions 0: 480x270
>>> -#sar 0: 1/1
>>> -#tb 1: 1/48000
>>> -#media_type 1: audio
>>> -#codec_id 1: pcm_s16le
>>> -#sample_rate 1: 48000
>>> -#channel_layout 1: 3
>>> -#channel_layout_name 1: stereo
>>> -0,          0,          0,        1,   259200, 0xf36957da
>>> -1,          0,          0,     1602,     6408, 0x1dd7b37c
>>> -0,          1,          1,        1,   259200, 0x29a1f586
>>> -1,       1602,       1602,     1601,     6404, 0xb6854846
>>> -1,       3203,       3203,     1602,     6408, 0x4d3ea85e
>>> -0,          2,          2,        1,   259200, 0x5578d9c3
>>> -0,          3,          3,        1,   259200, 0x83938b61
>>> -1,       4805,       4805,     1601,     6404, 0x5eb15a6d
>>> -1,       6406,       6406,     1602,     6408, 0x059d21a0
>>> -0,          4,          4,        1,   259200, 0xa6ce7618
>>> -0,          5,          5,        1,   259200, 0x4892a0f5
>>> -1,       8008,       8008,     1602,     6408, 0xd8352572
>>> -0,          6,          6,        1,   259200, 0x921c6051
>>> -1,       9610,       9610,     1601,     6404, 0xf69be875
>>> -1,      11211,      11211,     1602,     6408, 0x41e75601
>>> -0,          7,          7,        1,   259200, 0x618c0026
>>> -0,          8,          8,        1,   259200, 0xdbc3ca4d
>>> -1,      12813,      12813,     1601,     6404, 0x75e3196d
>>> -1,      14414,      14414,     1602,     6408, 0xb46bad29
>>> -0,          9,          9,        1,   259200, 0xf088c731
>>> -0,         10,         10,        1,   259200, 0xce77ddee
>>> -1,      16016,      16016,     1602,     6408, 0x41e6ceac
>>> -0,         11,         11,        1,   259200, 0x798565eb
>>> -1,      17618,      17618,     1601,     6404, 0x2258734e
>>> -1,      19219,      19219,     1602,     6408, 0xc46d9103
>>> -0,         12,         12,        1,   259200, 0x57185dc8
>>> -0,         13,         13,        1,   259200, 0x607a9086
>>> -1,      20821,      20821,     1601,     6404, 0xd7c07892
>>> -1,      22422,      22422,     1602,     6408, 0x2aaad91d
>>> -0,         14,         14,        1,   259200, 0x59bd5c34
>>> -0,         15,         15,        1,   259200, 0xadb1da77
>>> -1,      24024,      24024,     1602,     6408, 0x69bfb643
>>> -0,         16,         16,        1,   259200, 0x1f7d7b14
>>> -1,      25626,      25626,     1601,     6404, 0x0e644904
>>> -1,      27227,      27227,     1602,     6408, 0x06e92ea2
>>> -0,         17,         17,        1,   259200, 0xcdd45467
>>> Test copy-trac4914 failed. Look at tests/data/fate/copy-trac4914.err for details.
>>
>> As i said gmtime_r() is what's failing, and it's used by both the muxer
>> and demuxer. Only Cygwin seems to work, probably because it uses its own
>> C libraries.
>
> yes, i did just run into this when testing my postproc patch accross
> platforms, (test fails, looking at what/why, finding thread, reporting ...)

I'll take a look at this.

Regards,
Marton