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

2021-05-14 Thread Marton Balint 




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 
---
 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 * 
6000LL + 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2021-05-14 Thread Anton Khirnov 
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 
> ---
>  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 * 
> 6000LL + 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.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2021-05-12 Thread Marton Balint 




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 
---
  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 * 
6000LL + 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=0x|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=3/1001|avg_frame_rate=30/1|time_base=1/1000|start_pts=0|start_time=0.00|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=0x|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.00|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.00|duration=210.20|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:h

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

2021-05-12 Thread James Almer 

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 
---
  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 * 
6000LL + 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=0x|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=3/1001|avg_frame_rate=30/1|time_base=1/1000|start_pts=0|start_time=0.00|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=0x|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.00|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.00|duration=210.20|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_Principl

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

2021-05-12 Thread James Almer 

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 
---
  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 * 6000LL + 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=0x|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=3/1001|avg_frame_rate=30/1|time_base=1/1000|start_pts=0|start_time=0.00|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=0x|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.00|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.00|duration=210.20|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:me