[FFmpeg-devel] [PATCH] make stream-switching work with MOV demuxer

[Reimar Döffinger]

On Tue, Jun 23, 2009 at 09:19:38PM +0200, Reimar D?ffinger wrote:
> On Tue, Jun 23, 2009 at 11:59:21AM -0700, Baptiste Coudurier wrote:
> > I think discard variable can be avoided by checking
> > s->streams[sc->ffindex]->discard.
> > 
> > We could even declare AVStream *st = s->streams[sc->ffindex] when sample
> > is found and factore because it is already done when computing duration.
> > 
> > We could even get rid of sc->ffindex by choosing AVStream in the loop
> > and use ->priv_data, but well that can be done separately.
> 
> I think it is uglier, but here it is.

Ok, I think the real issue is that things that belong together
(incrementing current_sample and incrementing ctts_sample) are done in
different places.
I also think it is wrong that on read error only current_sample is
incremented.
Attached patch fixes it.
It should of course be applied in multiple steps, e.g. I think that
>        if (dv_get_packet(mov->dv_demux, pkt) < 0)
>            return -1;
should actually be
>        ret = dv_get_packet(mov->dv_demux, pkt);
>        if (ret < 0)
>            return ret;
regardless of what you think of the other changes.
-------------- next part --------------
Index: libavformat/mov.c
===================================================================
--- libavformat/mov.c	(revision 19256)
+++ libavformat/mov.c	(working copy)
@@ -1977,18 +1977,33 @@
     return 0;
 }
 
+static void inc_samplepos(MOVStreamContext *sc)
+{
+    sc->current_sample++;
+    if (sc->ctts_data) {
+        /* update ctts context */
+        sc->ctts_sample++;
+        if (sc->ctts_index < sc->ctts_count &&
+            sc->ctts_data[sc->ctts_index].count == sc->ctts_sample) {
+            sc->ctts_index++;
+            sc->ctts_sample = 0;
+        }
+    }
+}
+
 static int mov_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     MOVContext *mov = s->priv_data;
     MOVStreamContext *sc = 0;
-    AVIndexEntry *sample = 0;
+    AVIndexEntry *sample;
     int64_t best_dts = INT64_MAX;
     int i, ret;
  retry:
+    sample = NULL;
     for (i = 0; i < s->nb_streams; i++) {
         AVStream *st = s->streams[i];
         MOVStreamContext *msc = st->priv_data;
-        if (st->discard != AVDISCARD_ALL && msc->pb && msc->current_sample < st->nb_index_entries) {
+        if (msc->pb && msc->current_sample < st->nb_index_entries) {
             AVIndexEntry *current_sample = &st->index_entries[msc->current_sample];
             int64_t dts = av_rescale(current_sample->timestamp, AV_TIME_BASE, msc->time_scale);
             dprintf(s, "stream %d, sample %d, dts %"PRId64"\n", i, msc->current_sample, dts);
@@ -2012,36 +2027,33 @@
         dprintf(s, "read fragments, offset 0x%llx\n", url_ftell(s->pb));
         goto retry;
     }
-    /* must be done just before reading, to avoid infinite loop on sample */
-    sc->current_sample++;
+    if (s->streams[sc->ffindex]->discard == AVDISCARD_ALL) {
+        inc_samplepos(sc);
+        goto retry;
+    }
     if (url_fseek(sc->pb, sample->pos, SEEK_SET) != sample->pos) {
         av_log(mov->fc, AV_LOG_ERROR, "stream %d, offset 0x%"PRIx64": partial file\n",
                sc->ffindex, sample->pos);
-        return -1;
+        ret = -1;
+        goto out;
     }
     ret = av_get_packet(sc->pb, pkt, sample->size);
     if (ret < 0)
-        return ret;
+        goto out;
 #if CONFIG_DV_DEMUXER
     if (mov->dv_demux && sc->dv_audio_container) {
         dv_produce_packet(mov->dv_demux, pkt, pkt->data, pkt->size);
         av_free(pkt->data);
         pkt->size = 0;
-        if (dv_get_packet(mov->dv_demux, pkt) < 0)
-            return -1;
+        ret = dv_get_packet(mov->dv_demux, pkt);
+        if (ret < 0)
+            goto out;
     }
 #endif
     pkt->stream_index = sc->ffindex;
     pkt->dts = sample->timestamp;
     if (sc->ctts_data) {
         pkt->pts = pkt->dts + sc->dts_shift + sc->ctts_data[sc->ctts_index].duration;
-        /* update ctts context */
-        sc->ctts_sample++;
-        if (sc->ctts_index < sc->ctts_count &&
-            sc->ctts_data[sc->ctts_index].count == sc->ctts_sample) {
-            sc->ctts_index++;
-            sc->ctts_sample = 0;
-        }
         if (sc->wrong_dts)
             pkt->dts = AV_NOPTS_VALUE;
     } else {
@@ -2055,7 +2067,11 @@
     pkt->pos = sample->pos;
     dprintf(s, "stream %d, pts %"PRId64", dts %"PRId64", pos 0x%"PRIx64", duration %d\n",
             pkt->stream_index, pkt->pts, pkt->dts, pkt->pos, pkt->duration);
-    return 0;
+    ret = 0;
+out:
+    // make sure we always advance one sample even on an error to avoid an endless loop
+    inc_samplepos(sc);
+    return ret;
 }
 
 static int mov_seek_stream(AVStream *st, int64_t timestamp, int flags)
@@ -2108,7 +2124,7 @@
 
     for (i = 0; i < s->nb_streams; i++) {
         st = s->streams[i];
-        if (stream_index == i || st->discard == AVDISCARD_ALL)
+        if (stream_index == i)
             continue;
 
         timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base);