diff mbox series

[FFmpeg-devel] avformat/flvdec: enhance parsing timestamps

Message ID 20210512185545.18602-1-cus@passwd.hu
State New
Headers show
Series [FFmpeg-devel] avformat/flvdec: enhance parsing timestamps | expand

Checks

Context Check Description
andriy/configure warning Failed to apply patch

Commit Message

Marton Balint May 12, 2021, 6:55 p.m. UTC 
Take into account timezone. Use millisecond precision. Maybe we could also use
nanosecond, but there were some float rounding concerns.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/flvdec.c     | 13 ++-----------
 tests/ref/fate/flv-demux |  2 +-
 2 files changed, 3 insertions(+), 12 deletions(-)

-format|filename=Enigma_Principles_of_Lust-part.flv|nb_streams=2|nb_programs=0|format_name=flv|start_time=0.000000|duration=210.209999|size=512000|bit_rate=19485|probe_score=100|tag:hasKeyframes=true|tag:hasMetadata=true|tag:datasize=11970544|tag:hasVideo=true|tag:canSeekToEnd=false|tag:lasttimestamp=210|tag:lastkeyframetimestamp=210|tag:audiosize=1791332|tag:hasAudio=true|tag:audiodelay=0|tag:videosize=10176110|tag:metadatadate=2011-02-27T11:00:33Z|tag:metadatacreator=inlet media FLVTool2 v1.0.6 - http://www.inlet-media.de/flvtool2|tag:hasCuePoints=false
+format|filename=Enigma_Principles_of_Lust-part.flv|nb_streams=2|nb_programs=0|format_name=flv|start_time=0.000000|duration=210.209999|size=512000|bit_rate=19485|probe_score=100|tag:hasKeyframes=true|tag:hasMetadata=true|tag:datasize=11970544|tag:hasVideo=true|tag:canSeekToEnd=false|tag:lasttimestamp=210|tag:lastkeyframetimestamp=210|tag:audiosize=1791332|tag:hasAudio=true|tag:audiodelay=0|tag:videosize=10176110|tag:metadatadate=2011-02-27T17:00:33.125000Z|tag:metadatacreator=inlet media FLVTool2 v1.0.6 - http://www.inlet-media.de/flvtool2|tag:hasCuePoints=false

Comments

James Almer May 12, 2021, 7:04 p.m. UTC | #1 
On 5/12/2021 3:55 PM, Marton Balint wrote:
> Take into account timezone. Use millisecond precision. Maybe we could also use
> nanosecond, but there were some float rounding concerns.

Alexander Strasser wrote an alternative approach to using timezone in 
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-May/280179.html 
yesterday.

And regarding the rounding concerns, we could maybe check for the 
bitexact flag in avctx->flags and use it to either print 
milli/nanoseconds or leave it as seconds.

> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   libavformat/flvdec.c     | 13 ++-----------
>   tests/ref/fate/flv-demux |  2 +-
>   2 files changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index ddaceaafe4..3791687072 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -28,6 +28,7 @@
>   #include "libavutil/channel_layout.h"
>   #include "libavutil/dict.h"
>   #include "libavutil/opt.h"
> +#include "libavutil/internal.h"
>   #include "libavutil/intfloat.h"
>   #include "libavutil/mathematics.h"
>   #include "libavutil/time_internal.h"
> @@ -682,17 +683,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream,
>           } else if (amf_type == AMF_DATA_TYPE_STRING) {
>               av_dict_set(&s->metadata, key, str_val, 0);
>           } else if (amf_type == AMF_DATA_TYPE_DATE) {
> -            time_t time;
> -            struct tm t;
> -            char datestr[128];
> -            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);
> -
> -            av_dict_set(&s->metadata, key, datestr, 0);
> +            avpriv_dict_set_timestamp(&s->metadata, key, -date.timezone * 60000000LL + 1000 * (int64_t)date.milliseconds);
>           }
>       }
>   
> diff --git a/tests/ref/fate/flv-demux b/tests/ref/fate/flv-demux
> index 827b56ea09..3eede6eb00 100644
> --- a/tests/ref/fate/flv-demux
> +++ b/tests/ref/fate/flv-demux
> @@ -605,4 +605,4 @@ packet|codec_type=audio|stream_index=1|pts=11656|pts_time=11.656000|dts=11656|dt
>   packet|codec_type=video|stream_index=0|pts=11678|pts_time=11.678000|dts=11678|dts_time=11.678000|duration=33|duration_time=0.033000|size=1190|pos=510794|flags=__|data_hash=CRC32:a0206c90
>   stream|index=0|codec_name=h264|profile=77|codec_type=video|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|width=426|height=240|coded_width=426|coded_height=240|closed_captions=0|has_b_frames=1|sample_aspect_ratio=1:1|display_aspect_ratio=71:40|pix_fmt=yuv420p|level=21|color_range=unknown|color_space=unknown|color_transfer=unknown|color_primaries=unknown|chroma_location=left|field_order=progressive|refs=1|is_avc=true|nal_length_size=4|id=N/A|r_frame_rate=30000/1001|avg_frame_rate=30/1|time_base=1/1000|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=393929|max_bit_rate=N/A|bits_per_raw_sample=8|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=351|extradata_hash=CRC32:07b85ca9|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbn
 ail
>   s=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0
>   stream|index=1|codec_name=aac|profile=1|codec_type=audio|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|sample_fmt=fltp|sample_rate=22050|channels=2|channel_layout=stereo|bits_per_sample=0|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/1000|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=67874|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=252|extradata_hash=CRC32:d039c029|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0
> -format|filename=Enigma_Principles_of_Lust-part.flv|nb_streams=2|nb_programs=0|format_name=flv|start_time=0.000000|duration=210.209999|size=512000|bit_rate=19485|probe_score=100|tag:hasKeyframes=true|tag:hasMetadata=true|tag:datasize=11970544|tag:hasVideo=true|tag:canSeekToEnd=false|tag:lasttimestamp=210|tag:lastkeyframetimestamp=210|tag:audiosize=1791332|tag:hasAudio=true|tag:audiodelay=0|tag:videosize=10176110|tag:metadatadate=2011-02-27T11:00:33Z|tag:metadatacreator=inlet media FLVTool2 v1.0.6 - http://www.inlet-media.de/flvtool2|tag:hasCuePoints=false
> +format|filename=Enigma_Principles_of_Lust-part.flv|nb_streams=2|nb_programs=0|format_name=flv|start_time=0.000000|duration=210.209999|size=512000|bit_rate=19485|probe_score=100|tag:hasKeyframes=true|tag:hasMetadata=true|tag:datasize=11970544|tag:hasVideo=true|tag:canSeekToEnd=false|tag:lasttimestamp=210|tag:lastkeyframetimestamp=210|tag:audiosize=1791332|tag:hasAudio=true|tag:audiodelay=0|tag:videosize=10176110|tag:metadatadate=2011-02-27T17:00:33.125000Z|tag:metadatacreator=inlet media FLVTool2 v1.0.6 - http://www.inlet-media.de/flvtool2|tag:hasCuePoints=false
>
James Almer May 12, 2021, 7:05 p.m. UTC | #2 
On 5/12/2021 4:04 PM, James Almer wrote:
> On 5/12/2021 3:55 PM, Marton Balint wrote:
>> Take into account timezone. Use millisecond precision. Maybe we could 
>> also use
>> nanosecond, but there were some float rounding concerns.
> 
> Alexander Strasser wrote an alternative approach to using timezone in 
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-May/280179.html 
> yesterday.
> 
> And regarding the rounding concerns, we could maybe check for the 
> bitexact flag in avctx->flags and use it to either print 
> milli/nanoseconds or leave it as seconds.

Nevermind this part, this is lavf not lavc, and AVFormatContext bitexact 
flag is not meant for demuxers, only muxers.

> 
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>   libavformat/flvdec.c     | 13 ++-----------
>>   tests/ref/fate/flv-demux |  2 +-
>>   2 files changed, 3 insertions(+), 12 deletions(-)
>>
>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>> index ddaceaafe4..3791687072 100644
>> --- a/libavformat/flvdec.c
>> +++ b/libavformat/flvdec.c
>> @@ -28,6 +28,7 @@
>>   #include "libavutil/channel_layout.h"
>>   #include "libavutil/dict.h"
>>   #include "libavutil/opt.h"
>> +#include "libavutil/internal.h"
>>   #include "libavutil/intfloat.h"
>>   #include "libavutil/mathematics.h"
>>   #include "libavutil/time_internal.h"
>> @@ -682,17 +683,7 @@ static int amf_parse_object(AVFormatContext *s, 
>> AVStream *astream,
>>           } else if (amf_type == AMF_DATA_TYPE_STRING) {
>>               av_dict_set(&s->metadata, key, str_val, 0);
>>           } else if (amf_type == AMF_DATA_TYPE_DATE) {
>> -            time_t time;
>> -            struct tm t;
>> -            char datestr[128];
>> -            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);
>> -
>> -            av_dict_set(&s->metadata, key, datestr, 0);
>> +            avpriv_dict_set_timestamp(&s->metadata, key, 
>> -date.timezone * 60000000LL + 1000 * (int64_t)date.milliseconds);
>>           }
>>       }
>> diff --git a/tests/ref/fate/flv-demux b/tests/ref/fate/flv-demux
>> index 827b56ea09..3eede6eb00 100644
>> --- a/tests/ref/fate/flv-demux
>> +++ b/tests/ref/fate/flv-demux
>> @@ -605,4 +605,4 @@ 
>> packet|codec_type=audio|stream_index=1|pts=11656|pts_time=11.656000|dts=11656|dt 
>>
>>   
>> packet|codec_type=video|stream_index=0|pts=11678|pts_time=11.678000|dts=11678|dts_time=11.678000|duration=33|duration_time=0.033000|size=1190|pos=510794|flags=__|data_hash=CRC32:a0206c90 
>>
>>   
>> stream|index=0|codec_name=h264|profile=77|codec_type=video|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|width=426|height=240|coded_width=426|coded_height=240|closed_captions=0|has_b_frames=1|sample_aspect_ratio=1:1|display_aspect_ratio=71:40|pix_fmt=yuv420p|level=21|color_range=unknown|color_space=unknown|color_transfer=unknown|color_primaries=unknown|chroma_location=left|field_order=progressive|refs=1|is_avc=true|nal_length_size=4|id=N/A|r_frame_rate=30000/1001|avg_frame_rate=30/1|time_base=1/1000|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=393929|max_bit_rate=N/A|bits_per_raw_sample=8|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=351|extradata_hash=CRC32:07b85ca9|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnail 
>>
>>   
>> s=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0 
>>
>>   
>> stream|index=1|codec_name=aac|profile=1|codec_type=audio|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|sample_fmt=fltp|sample_rate=22050|channels=2|channel_layout=stereo|bits_per_sample=0|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/1000|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=67874|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=252|extradata_hash=CRC32:d039c029|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0 
>>
>> -format|filename=Enigma_Principles_of_Lust-part.flv|nb_streams=2|nb_programs=0|format_name=flv|start_time=0.000000|duration=210.209999|size=512000|bit_rate=19485|probe_score=100|tag:hasKeyframes=true|tag:hasMetadata=true|tag:datasize=11970544|tag:hasVideo=true|tag:canSeekToEnd=false|tag:lasttimestamp=210|tag:lastkeyframetimestamp=210|tag:audiosize=1791332|tag:hasAudio=true|tag:audiodelay=0|tag:videosize=10176110|tag:metadatadate=2011-02-27T11:00:33Z|tag:metadatacreator=inlet 
>> media FLVTool2 v1.0.6 - 
>> http://www.inlet-media.de/flvtool2|tag:hasCuePoints=false
>> +format|filename=Enigma_Principles_of_Lust-part.flv|nb_streams=2|nb_programs=0|format_name=flv|start_time=0.000000|duration=210.209999|size=512000|bit_rate=19485|probe_score=100|tag:hasKeyframes=true|tag:hasMetadata=true|tag:datasize=11970544|tag:hasVideo=true|tag:canSeekToEnd=false|tag:lasttimestamp=210|tag:lastkeyframetimestamp=210|tag:audiosize=1791332|tag:hasAudio=true|tag:audiodelay=0|tag:videosize=10176110|tag:metadatadate=2011-02-27T17:00:33.125000Z|tag:metadatacreator=inlet 
>> media FLVTool2 v1.0.6 - 
>> http://www.inlet-media.de/flvtool2|tag:hasCuePoints=false
>>
>
Marton Balint May 12, 2021, 7:25 p.m. UTC | #3 
On Wed, 12 May 2021, James Almer wrote:

> On 5/12/2021 3:55 PM, Marton Balint wrote:
>> Take into account timezone. Use millisecond precision. Maybe we could also 
>> use
>> nanosecond, but there were some float rounding concerns.
>
> Alexander Strasser wrote an alternative approach to using timezone in 
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-May/280179.html 
> yesterday.
>

Yeah, I saw it, I just think it is better to not duplicate the timestamp 
generator code but use the already available function for this purpose.

Regards,
Marton

>
>> 
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>   libavformat/flvdec.c     | 13 ++-----------
>>   tests/ref/fate/flv-demux |  2 +-
>>   2 files changed, 3 insertions(+), 12 deletions(-)
>> 
>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>> index ddaceaafe4..3791687072 100644
>> --- a/libavformat/flvdec.c
>> +++ b/libavformat/flvdec.c
>> @@ -28,6 +28,7 @@
>>   #include "libavutil/channel_layout.h"
>>   #include "libavutil/dict.h"
>>   #include "libavutil/opt.h"
>> +#include "libavutil/internal.h"
>>   #include "libavutil/intfloat.h"
>>   #include "libavutil/mathematics.h"
>>   #include "libavutil/time_internal.h"
>> @@ -682,17 +683,7 @@ static int amf_parse_object(AVFormatContext *s, 
>> AVStream *astream,
>>           } else if (amf_type == AMF_DATA_TYPE_STRING) {
>>               av_dict_set(&s->metadata, key, str_val, 0);
>>           } else if (amf_type == AMF_DATA_TYPE_DATE) {
>> -            time_t time;
>> -            struct tm t;
>> -            char datestr[128];
>> -            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);
>> -
>> -            av_dict_set(&s->metadata, key, datestr, 0);
>> +            avpriv_dict_set_timestamp(&s->metadata, key, -date.timezone * 
>> 60000000LL + 1000 * (int64_t)date.milliseconds);
>>           }
>>       }
>>   diff --git a/tests/ref/fate/flv-demux b/tests/ref/fate/flv-demux
>> index 827b56ea09..3eede6eb00 100644
>> --- a/tests/ref/fate/flv-demux
>> +++ b/tests/ref/fate/flv-demux
>> @@ -605,4 +605,4 @@ 
>> packet|codec_type=audio|stream_index=1|pts=11656|pts_time=11.656000|dts=11656|dt
>>   packet|codec_type=video|stream_index=0|pts=11678|pts_time=11.678000|dts=11678|dts_time=11.678000|duration=33|duration_time=0.033000|size=1190|pos=510794|flags=__|data_hash=CRC32:a0206c90
>>   stream|index=0|codec_name=h264|profile=77|codec_type=video|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|width=426|height=240|coded_width=426|coded_height=240|closed_captions=0|has_b_frames=1|sample_aspect_ratio=1:1|display_aspect_ratio=71:40|pix_fmt=yuv420p|level=21|color_range=unknown|color_space=unknown|color_transfer=unknown|color_primaries=unknown|chroma_location=left|field_order=progressive|refs=1|is_avc=true|nal_length_size=4|id=N/A|r_frame_rate=30000/1001|avg_frame_rate=30/1|time_base=1/1000|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=393929|max_bit_rate=N/A|bits_per_raw_sample=8|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=351|extradata_hash=CRC32:07b85ca9|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:tim 
>> ed_thumbn
> ail
>>   s=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0
>>   stream|index=1|codec_name=aac|profile=1|codec_type=audio|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|sample_fmt=fltp|sample_rate=22050|channels=2|channel_layout=stereo|bits_per_sample=0|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/1000|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=67874|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=252|extradata_hash=CRC32:d039c029|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0
>> 
>> -format|filename=Enigma_Principles_of_Lust-part.flv|nb_streams=2|nb_programs=0|format_name=flv|start_time=0.000000|duration=210.209999|size=512000|bit_rate=19485|probe_score=100|tag:hasKeyframes=true|tag:hasMetadata=true|tag:datasize=11970544|tag:hasVideo=true|tag:canSeekToEnd=false|tag:lasttimestamp=210|tag:lastkeyframetimestamp=210|tag:audiosize=1791332|tag:hasAudio=true|tag:audiodelay=0|tag:videosize=10176110|tag:metadatadate=2011-02-27T11:00:33Z|tag:metadatacreator=inlet 
>> media FLVTool2 v1.0.6 - 
>> http://www.inlet-media.de/flvtool2|tag:hasCuePoints=false
>> +format|filename=Enigma_Principles_of_Lust-part.flv|nb_streams=2|nb_programs=0|format_name=flv|start_time=0.000000|duration=210.209999|size=512000|bit_rate=19485|probe_score=100|tag:hasKeyframes=true|tag:hasMetadata=true|tag:datasize=11970544|tag:hasVideo=true|tag:canSeekToEnd=false|tag:lasttimestamp=210|tag:lastkeyframetimestamp=210|tag:audiosize=1791332|tag:hasAudio=true|tag:audiodelay=0|tag:videosize=10176110|tag:metadatadate=2011-02-27T17:00:33.125000Z|tag:metadatacreator=inlet 
>> media FLVTool2 v1.0.6 - 
>> http://www.inlet-media.de/flvtool2|tag:hasCuePoints=false
>> 
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Anton Khirnov May 14, 2021, 8:34 a.m. UTC | #4 
Quoting Marton Balint (2021-05-12 20:55:45)
> Take into account timezone. Use millisecond precision. Maybe we could also use
> nanosecond, but there were some float rounding concerns.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/flvdec.c     | 13 ++-----------
>  tests/ref/fate/flv-demux |  2 +-
>  2 files changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index ddaceaafe4..3791687072 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -28,6 +28,7 @@
>  #include "libavutil/channel_layout.h"
>  #include "libavutil/dict.h"
>  #include "libavutil/opt.h"
> +#include "libavutil/internal.h"
>  #include "libavutil/intfloat.h"
>  #include "libavutil/mathematics.h"
>  #include "libavutil/time_internal.h"
> @@ -682,17 +683,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream,
>          } else if (amf_type == AMF_DATA_TYPE_STRING) {
>              av_dict_set(&s->metadata, key, str_val, 0);
>          } else if (amf_type == AMF_DATA_TYPE_DATE) {
> -            time_t time;
> -            struct tm t;
> -            char datestr[128];
> -            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);
> -
> -            av_dict_set(&s->metadata, key, datestr, 0);
> +            avpriv_dict_set_timestamp(&s->metadata, key, -date.timezone * 60000000LL + 1000 * (int64_t)date.milliseconds);

This is wrong -- date.milliseconds is in UTC, if you add the timezone
offset it will no longer be in UTC.
Marton Balint May 14, 2021, 7:23 p.m. UTC | #5 
On Fri, 14 May 2021, Anton Khirnov wrote:

> Quoting Marton Balint (2021-05-12 20:55:45)
>> Take into account timezone. Use millisecond precision. Maybe we could also use
>> nanosecond, but there were some float rounding concerns.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/flvdec.c     | 13 ++-----------
>>  tests/ref/fate/flv-demux |  2 +-
>>  2 files changed, 3 insertions(+), 12 deletions(-)
>>
>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>> index ddaceaafe4..3791687072 100644
>> --- a/libavformat/flvdec.c
>> +++ b/libavformat/flvdec.c
>> @@ -28,6 +28,7 @@
>>  #include "libavutil/channel_layout.h"
>>  #include "libavutil/dict.h"
>>  #include "libavutil/opt.h"
>> +#include "libavutil/internal.h"
>>  #include "libavutil/intfloat.h"
>>  #include "libavutil/mathematics.h"
>>  #include "libavutil/time_internal.h"
>> @@ -682,17 +683,7 @@ static int amf_parse_object(AVFormatContext *s, AVStream *astream,
>>          } else if (amf_type == AMF_DATA_TYPE_STRING) {
>>              av_dict_set(&s->metadata, key, str_val, 0);
>>          } else if (amf_type == AMF_DATA_TYPE_DATE) {
>> -            time_t time;
>> -            struct tm t;
>> -            char datestr[128];
>> -            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);
>> -
>> -            av_dict_set(&s->metadata, key, datestr, 0);
>> +            avpriv_dict_set_timestamp(&s->metadata, key, -date.timezone * 60000000LL + 1000 * (int64_t)date.milliseconds);
>
> This is wrong -- date.milliseconds is in UTC, if you add the timezone
> offset it will no longer be in UTC.

Indeed, I misunderstood the specs... MediaInfo also ignores the time zone. 
Will send a v2.

Thanks,
Marton
diff mbox series

Patch

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index ddaceaafe4..3791687072 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -28,6 +28,7 @@ 
 #include "libavutil/channel_layout.h"
 #include "libavutil/dict.h"
 #include "libavutil/opt.h"
+#include "libavutil/internal.h"
 #include "libavutil/intfloat.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/time_internal.h"
@@ -682,17 +683,7 @@  static int amf_parse_object(AVFormatContext *s, AVStream *astream,
         } else if (amf_type == AMF_DATA_TYPE_STRING) {
             av_dict_set(&s->metadata, key, str_val, 0);
         } else if (amf_type == AMF_DATA_TYPE_DATE) {
-            time_t time;
-            struct tm t;
-            char datestr[128];
-            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);
-
-            av_dict_set(&s->metadata, key, datestr, 0);
+            avpriv_dict_set_timestamp(&s->metadata, key, -date.timezone * 60000000LL + 1000 * (int64_t)date.milliseconds);
         }
     }
 
diff --git a/tests/ref/fate/flv-demux b/tests/ref/fate/flv-demux
index 827b56ea09..3eede6eb00 100644
--- a/tests/ref/fate/flv-demux
+++ b/tests/ref/fate/flv-demux
@@ -605,4 +605,4 @@  packet|codec_type=audio|stream_index=1|pts=11656|pts_time=11.656000|dts=11656|dt
 packet|codec_type=video|stream_index=0|pts=11678|pts_time=11.678000|dts=11678|dts_time=11.678000|duration=33|duration_time=0.033000|size=1190|pos=510794|flags=__|data_hash=CRC32:a0206c90
 stream|index=0|codec_name=h264|profile=77|codec_type=video|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|width=426|height=240|coded_width=426|coded_height=240|closed_captions=0|has_b_frames=1|sample_aspect_ratio=1:1|display_aspect_ratio=71:40|pix_fmt=yuv420p|level=21|color_range=unknown|color_space=unknown|color_transfer=unknown|color_primaries=unknown|chroma_location=left|field_order=progressive|refs=1|is_avc=true|nal_length_size=4|id=N/A|r_frame_rate=30000/1001|avg_frame_rate=30/1|time_base=1/1000|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=393929|max_bit_rate=N/A|bits_per_raw_sample=8|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=351|extradata_hash=CRC32:07b85ca9|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnail
 s=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0
 stream|index=1|codec_name=aac|profile=1|codec_type=audio|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|sample_fmt=fltp|sample_rate=22050|channels=2|channel_layout=stereo|bits_per_sample=0|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/1000|start_pts=0|start_time=0.000000|duration_ts=N/A|duration=N/A|bit_rate=67874|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=252|extradata_hash=CRC32:d039c029|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0