[FFmpeg-devel] [HACK] mov edl desync fix

[Reimar Döffinger]

On Sun, Jan 11, 2009 at 01:49:12AM -0800, Baptiste Coudurier wrote:
> Reimar D?ffinger wrote:
> > That's simple to do. Also time values of -1 should be ignored it seems,
> > since they mean an empty list (sounds weird, since there is also a
> > length entry and it is unclear what other values < 0 are supposed to
> > mean).
> 
> -1 means ignore this track (do not display it for example) for 'length'
> time. This is mentioned in specs IIRC. 'showdown2.mov' is a good example
> of what it is, dunno if it's still available somewhere, I might still
> have it.

Ok, that still means we can not do timestamp-adjustment there.

> > Though it seems more correct to me to also do this for edls with more
> > than one part (of course only the time of the first chunk is relevant),
> > attached patch would do that.
> 
> I prefer not to do so. Edit list is not about hacking dts, it is about
> selectionning specific part of the coded track to take into account.
> Better applying the hack when it is safe to do so.

What do you consider "safe to do so"? To me it sounds like what you are
say is "better it is completely broken than possibly broken".
IMO my suggestion would change things to "the first edl chunk is always
played synced correctly" whereas what you want would be "only files with
only one edl chunk play right" which does not really seem safer to me.
Do you have a specific case in mind where it would be less safe? I do
agree that the warning should not be disabled, but that is true anyway,
since only the timestamps are adjusted, nothing is actually skipped.


> >> - The time value refers to the pts not the dts according to specs,
> >> typically this value is 1 for there is delay (first pts is 1 and first
> >> dts is 0)
> > 
> > The specification pdf from May 1996 that I have at hand says nothing
> > about that. But assuming that were true, the current check for time != 0
> > would be wrong already.
> 
> The check is correct, and the message too, that's why it says "might".
> In some case it does no harm. Latest qt specs are from 2007, and you can
> refer to iso media specs also.

Do you have any direct link to a PDF? I always end up with either HTML
or a PDF that one way or another is wrong.
Nevertheless, given that there are these two ways pts is set in mov.c:
pkt->pts = pkt->dts + sc->ctts_data[sc->sample_to_ctime_index].duration / sc->time_rate;
and
pkt->pts = pkt->dts;
Attached patch should fix that pts/dts discrepancy assuming the ctts
atom comes before the elst atom (probably a bad idea to assume that?).

Greetings,
Reimar D?ffinger
-------------- next part --------------
Index: libavformat/mov.c
===================================================================
--- libavformat/mov.c	(revision 16530)
+++ libavformat/mov.c	(working copy)
@@ -124,6 +124,7 @@
     int *keyframes;
     int time_scale;
     int time_rate;
+    int time_offset;
     int current_sample;
     unsigned int bytes_per_frame;
     unsigned int samples_per_frame;
@@ -1219,7 +1220,7 @@
 {
     MOVStreamContext *sc = st->priv_data;
     int64_t current_offset;
-    int64_t current_dts = 0;
+    int64_t current_dts = -sc->time_offset / sc->time_rate;
     unsigned int stts_index = 0;
     unsigned int stsc_index = 0;
     unsigned int stss_index = 0;
@@ -1735,7 +1738,8 @@
 /* edit list atom */
 static int mov_read_elst(MOVContext *c, ByteIOContext *pb, MOVAtom atom)
 {
-    MOVStreamContext *sc = c->fc->streams[c->fc->nb_streams-1]->priv_data;
+    int streamnr = c->fc->nb_streams-1;
+    MOVStreamContext *sc = c->fc->streams[streamnr]->priv_data;
     int i, edit_count;
 
     get_byte(pb); /* version */
@@ -1747,9 +1751,15 @@
         get_be32(pb); /* Track duration */
         time = get_be32(pb); /* Media time */
         get_be32(pb); /* Media rate */
+        if (time != -1) {
+            if (sc->ctts_data) // convert pts to dts
+                time -= sc->ctts_data[sc->sample_to_ctime_index].duration
+            if (edit_count == 1)
+                sc->time_offset = time;
+        }
         if (time != 0)
-            av_log(c->fc, AV_LOG_WARNING, "edit list not starting at 0, "
-                   "a/v desync might occur, patch welcome\n");
+            av_log(c->fc, AV_LOG_WARNING, "edit list %i for track %i not starting at 0 (but %i), "
+                   "a/v desync might occur, patch welcome\n", i, streamnr, time);
     }
     dprintf(c->fc, "track[%i].edit_count = %i\n", c->fc->nb_streams-1, sc->edit_count);
     return 0;