[Ffmpeg-cvslog] Re: ffmpeg-cvslog Digest, Vol 12, Issue 46

[Michael Niedermayer]

Hi

On Tue, Mar 28, 2006 at 07:42:49AM -0800, Steve Lhomme wrote:
> >Modified Files:
> >	matroska.c 
> >Log Message:
> >fixing timebase
> >
> >
> >Index: matroska.c
> >===================================================================
> >RCS file: /cvsroot/ffmpeg/ffmpeg/libavformat/matroska.c,v
> >retrieving revision 1.25
> >retrieving revision 1.26
> >diff -u -d -r1.25 -r1.26
> >--- matroska.c	25 Mar 2006 13:37:34 -0000	1.25
> >+++ matroska.c	25 Mar 2006 19:40:27 -0000	1.26
> >@@ -2206,7 +2206,7 @@
> >             st = av_new_stream(s, track->stream_index);
> >             if (st == NULL)
> >                 return AVERROR_NOMEM;
> >-            av_set_pts_info(st, 24, 1, 1000); /* 24 bit pts in ms */
> >+            av_set_pts_info(st, 64, matroska->time_scale, 
> >1000*1000*1000); /* 64 bit pts in ns */
> > 
> >             st->codec->codec_id = codec_id;
> > 
> >@@ -2295,7 +2295,7 @@
> >             case MATROSKA_ID_BLOCK: {
> >                 uint8_t *data, *origdata;
> >                 int size;
> >-                uint64_t block_time;
> >+                int16_t block_time;
> >                 uint32_t *lace_size = NULL;
> >                 int n, track, flags, laces = 0;
> >                 uint64_t num;
> >@@ -2329,7 +2329,7 @@
> >                 }
> > 
> >                 /* block_time (relative to cluster time) */
> >-                block_time = ((data[0] << 8) | data[1]) * 
> >matroska->time_scale;
> >+                block_time = (data[0] << 8) | data[1];
> >                 data += 2;
> >                 size -= 2;
> >                 flags = *data;
> >@@ -2440,7 +2440,7 @@
> >                         pkt->stream_index =
> >                             matroska->tracks[track]->stream_index;
> > 
> >-                        pkt->pts = timecode / 1000000; /* ns to ms */
> >+                        pkt->pts = timecode;
> >                         pkt->pos= pos;
> > 
> >                         matroska_queue_packet(matroska, pkt);
> >@@ -2514,7 +2514,7 @@
> >                 uint64_t num;
> >                 if ((res = ebml_read_uint(matroska, &id, &num)) < 0)
> >                     break;
> >-                cluster_time = num * matroska->time_scale;
> >+                cluster_time = num;
> >                 break;
> >             }
> 
> While I assume you have tested this and it works, there is a dangerous 
> change in this code. For the moment lavf doesn't handle the "Track 
> Timecode Scale" (used to mix audio from 30fps & 25fps with the same 
> video) and it's a float. I'm not sure it will fit that new design. You 
> might lose some precision when you do :
> 
> block_time = ((data[0] << 8) | data[1]) * track->timecode_scale;

*  this is the old code, not the new and yes i agree that the old code has
   the issue you describe

*  32bit floats are not precisse enough as timebase (or timebase adjustment)
   you will have very noticeable AV-deync after a few days, this is an issue
   with the spec not an implementation

[...]

-- 
Michael