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

[James Almer]

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.

If the code can't be simplified to remove the gmtime_r() usage, then
both modules should perhaps just skip writing/reading the modification
date metadata tag. And for that matter, shouldn't such tag be written or
not depending on the bitexact flag?