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

[James Almer]

On 12/15/2018 6:18 AM, 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
> the strftime string used in avpriv_dict_set_timestamp is somehow not
> portable there, and the error has previously just been hidden by the
> faulty check.

gmtime_r() is what's failing. I even tried forcing the fallback
implementation in libavutil/time_internal.h that uses gmtime() and got
the same result.

mxfdec does this weird thing of converting the mxf timestamp into a tm
struct, then to a int64_t which is used to call
avpriv_dict_set_timestamp(), which in turn converts it into a tm struct
using gmtime.
There's even a comment about msvc so i guess at some point it worked,
but perhaps this should be simplified instead.

> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>