[FFmpeg-devel] [PATCH] rmdec.c: merge old/new packet reading code

[Ronald S. Bultje]

Hi,

On Sat, Mar 14, 2009 at 1:27 AM, Kostya <kostya.shishkov at gmail.com> wrote:
> Probably rm_assemble_video_frame() should set the flags according to the
> frame it read.

Thinking about it, attached is a quickfix with what the original code
intended to do. It's imperfect because it might not index all
indexable chunks (i.e. false negatives), but at least it won't index
more bad stuff than what the original intention was (the only one I
can think of right now is video frames with remaining_len set). I'd
like to apply this before anything else because I fear that this
self-indexing will silently break some special cases and cause
hard-to-debug/track seeking bugs. I hope the patch is small enough.
Basically, it:

- makes the 'seq' parameter to ff_rm_parse_packet() work
- moves the index writing to directly after ff_rm_parse_packet(),
rather than a few calls later, this is just because even if we skip
this, we still want to write the index entry (without this, we miss
pretty much miss all chances to write an index entry)
- uses the flags parameter before rather than after the
ff_rm_parse_packet() call to decide whether to write an index entry,
because ff_rm_parse_packet() may overwrite its value (we need the
overwritten value also)

Ronald
-------------- next part --------------
Index: ffmpeg-svn/libavformat/rmdec.c
===================================================================
--- ffmpeg-svn.orig/libavformat/rmdec.c	2009-03-15 09:42:19.000000000 -0400
+++ ffmpeg-svn/libavformat/rmdec.c	2009-03-15 09:46:58.000000000 -0400
@@ -492,7 +492,7 @@
 
 static int rm_assemble_video_frame(AVFormatContext *s, ByteIOContext *pb,
                                    RMDemuxContext *rm, RMStream *vst,
-                                   AVPacket *pkt, int len)
+                                   AVPacket *pkt, int len, int *pseq)
 {
     int hdr, seq, pic_num, len2, pos;
     int type;
@@ -527,6 +527,7 @@
     }
     //now we have to deal with single slice
 
+    *pseq = seq;
     if((seq & 0x7F) == 1 || vst->curpic_num != pic_num){
         vst->slices = ((hdr & 0x3F) << 1) + 1;
         vst->videobufsize = len2 + 8*vst->slices + 1;
@@ -593,7 +594,7 @@
 
     if (st->codec->codec_type == CODEC_TYPE_VIDEO) {
         rm->current_stream= st->id;
-        if(rm_assemble_video_frame(s, pb, rm, ast, pkt, len))
+        if(rm_assemble_video_frame(s, pb, rm, ast, pkt, len, seq))
             return -1; //got partial frame
     } else if (st->codec->codec_type == CODEC_TYPE_AUDIO) {
         if ((st->codec->codec_id == CODEC_ID_RA_288) ||
@@ -705,9 +706,9 @@
     RMDemuxContext *rm = s->priv_data;
     ByteIOContext *pb = s->pb;
     AVStream *st;
-    int i, len;
+    int i, len, res;
     int64_t timestamp, pos;
-    int flags;
+    int old_flags, flags;
 
     if (rm->audio_pkt_cnt) {
         // If there are queued audio packet return them first
@@ -751,8 +752,12 @@
             return AVERROR(EIO);
         st = s->streams[i];
 
-        if (ff_rm_parse_packet (s, s->pb, st, st->priv_data, len, pkt,
-                                &seq, &flags, &timestamp) < 0)
+        old_flags = flags;
+        res = ff_rm_parse_packet (s, s->pb, st, st->priv_data, len, pkt,
+                                  &seq, &flags, &timestamp);
+        if((old_flags&2) && (seq&0x7F) == 1)
+            av_add_index_entry(st, pos, timestamp, 0, 0, AVINDEX_KEYFRAME);
+        if (res < 0)
             goto resync;
 
         if(  (st->discard >= AVDISCARD_NONKEY && !(flags&2))
@@ -764,9 +769,6 @@
             }
             goto resync;
         }
-
-        if((flags&2) && (seq&0x7F) == 1)
-            av_add_index_entry(st, pos, timestamp, 0, 0, AVINDEX_KEYFRAME);
     }
 
     return 0;