[FFmpeg-devel] [PATCH] lavf/flvdec: normalize exporting date metadata

[Alexander Strasser]

On 2021-05-16 21:18 +0200, Anton Khirnov wrote:
> Quoting Alexander Strasser (2021-05-15 20:20:30)
[...]
> >
> > Returning to the code I quoted before now and stating my
> > understanding of if now.
> >
> >     def write__AMF_date(time)
> >       write__UI8 11
> >       write [(time.to_f * 1000.0)].pack('G')
> >       write__SI16( (Time.now.gmtoff / 60).to_i )
> >     end
> >
> > This writes the time in micro seconds without offset as double.
> > The GMT offset in minutes is written afterwards as signed 16 bit
> > integer.
> >
> >
> >     def read__AMF_date
> >       utc_time = Time.at((read__AMF_double / 1000).to_i)
> >       utc_time + (read__SI16 * 60) - Time.now.gmtoff
> >     end
> >
> > This first reads the double and converts it into a Time object.
> > In the following line it reads and adds the stored offset and
> > subtracts the current offset to get rid of its influence.
>
> The writing code looks correct, but the reading code seems suspicious.

Actually I think both are wrong. I was fooled before because the
resulting output of flvtool2 is wrong if the timezone data is zero.

> To test it, I used flvtool2 to write metadata with TZ=America/New_York
> (-04:00) at 19:08 UTC. Then reading it with TZ=Europe/Paris (+02:00)
> gives me:
> - ffmpeg 4.3.2 -- correct, prints date in local timezone
>   metadatadate    : Sun, 16 May 2021 21:08:41 +0200
> - current ffmpeg git master -- correct, prints date in UTC
>   metadatadate    : 2021-05-16T19:08:41Z
> - flvtool2 -- WRONG
>   metadatadate: Sun May 16 15:08:41 GMT+0200 2021
>
> From which I conclude that flvtool2's own reading code is incorrect.

I'm not convinced that interpreting the timezone data will be a good
thing. Therefore I think the new comment in the code is probably not
good advice. I don't mind it much though, as at least for the files
written by flvtool2 it isn't wrong. I don't have any more examples
of this, despite tools writing a UTC timestamp and setting timezone
to zero always.

That the timezone offset written next to a UTC timestamp should
indicate the timezone of the system that wrote it seems unusual.
What useful information would that convey? Probably some vague
location information.

On the bright side: Current and previous FFmpeg code always did the
right thing for metadata dates written by flvtool2 :)

I withdraw my patch as from what we know now it would give wrong
output for metadata files that were generated by flvtool2.

FWIW I have opened a bug report for flvtool2:

  https://github.com/unnu/flvtool2/issues/10

Though it seems not to be maintained since around 2014.

Thanks for looking into this, Anton.


Best regards,
  Alexander