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

[Anton Khirnov]

Quoting Alexander Strasser (2021-05-15 20:20:30)
> Hi Anton!
> 
> On 2021-05-14 10:09 +0200, Anton Khirnov wrote:
> > Quoting Alexander Strasser (2021-05-12 01:04:28)
> > >
> > > If the timezone data of an AMF 0 date type is non-zero, include that
> > > information as UTC offset in hours and minutes.
> > > ---
> > >  libavformat/flvdec.c | 18 +++++++++++++++---
> > >  1 file changed, 15 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> > > index ddaceaafe4..c941e62e23 100644
> > > --- a/libavformat/flvdec.c
> > > +++ b/libavformat/flvdec.c
> > > @@ -688,9 +688,21 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream,
> > >              time =  date.milliseconds / 1000; // to seconds
> > >              gmtime_r(&time, &t);
> > >
> > > -            // timezone is ignored, since there is no easy way to offset the UTC
> > > -            // timestamp into the specified timezone
> > > -            strftime(datestr, sizeof(datestr), "%Y-%m-%dT%H:%M:%SZ", &t);
> > > +            strftime(datestr, sizeof(datestr), "%Y-%m-%dT%H:%M:%S", &t);
> > > +
> > > +            if (date.timezone) {
> > > +                int off_tz = date.timezone; // offset in minutes
> > > +                char ch_sign = '+';
> > > +                if (off_tz < 0) {
> > > +                    off_tz = -off_tz;
> > > +                    ch_sign = '-';
> > > +                }
> > > +                if (off_tz > 99*60 + 59)
> > > +                    off_tz = 99*60 + 59;
> > > +
> > > +                av_strlcatf(datestr, sizeof(datestr), "%c%02d%02d", ch_sign, off_tz / 60, off_tz % 60);
> >
> > I still believe this is wrong, since the spec says the timestamp is in
> > UTC. The code you quoted seems to conform to that.
> 
> Not to my understanding and testing.
> 
> This Ruby program
> 
>     t1 = Time.now()
>     t2 = Time.now.utc()
>     print t1,   " - ", t1.to_f, "\n"
>     print t2, "   - ", t2.to_f, "\n"
> 
> yields for example:
> 
>     2021-05-15 20:05:19 +0200 - 1621101919.509961
>     2021-05-15 18:05:19 UTC   - 1621101919.509966

This is just proving my point: the timestamp is UTC in both cases. The
only difference is that one of them has a non-zero timezone offset
attached.

> 
> 
> 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.
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.

-- 
Anton Khirnov