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

[Ronald S. Bultje]

Hi Kostya,

On Tue, Mar 10, 2009 at 2:26 AM, Kostya <kostya.shishkov at gmail.com> wrote:
>> ? ? ? ? ?if (ff_rm_parse_packet (s, s->pb, st, st->priv_data, len, pkt,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&seq, &flags, &timestamp) < 0)
>> ? ? ? ? ? ? ?goto resync;
>> + ? ?}
>
> } while(ff_rm_parse_packet (...) < 0);
>
> I like goto's but this one is rather unneeded (and is not that qualified to
> be called resync now).
>
>> ? ? ? ? ?if( ?(st->discard >= AVDISCARD_NONKEY && !(flags&2))
>> ? ? ? ? ? ? || st->discard >= AVDISCARD_ALL){
>
> And you can move discarding logic into the loop without need for emptying
> cache inside it.

These two don't go together, not literally at least. I guess a for(;;)
loop is OK also. Let me know if the attached is what you had in mind
(minus the do{}while();).

Ronald
-------------- next part --------------
Index: ffmpeg-svn/libavformat/rmdec.c
===================================================================
--- ffmpeg-svn.orig/libavformat/rmdec.c	2009-03-10 08:26:56.000000000 -0400
+++ ffmpeg-svn/libavformat/rmdec.c	2009-03-10 08:46:24.000000000 -0400
@@ -691,71 +691,48 @@
 static int rm_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     RMDemuxContext *rm = s->priv_data;
-    ByteIOContext *pb = s->pb;
     AVStream *st;
-    int i, len;
+    int i, len, seq = 1;
     int64_t timestamp, pos;
     int flags;
 
+    for (;;) {
     if (rm->audio_pkt_cnt) {
         // If there are queued audio packet return them first
         st = s->streams[rm->audio_stream_num];
         ff_rm_retrieve_cache(s, s->pb, st, st->priv_data, pkt);
-    } else if (rm->old_format) {
+    } else {
+        if (rm->old_format) {
         RMStream *ast;
 
         st = s->streams[0];
         ast = st->priv_data;
-        if (st->codec->codec_id == CODEC_ID_RA_288) {
-            int x, y;
-
-            for (y = 0; y < ast->sub_packet_h; y++)
-                for (x = 0; x < ast->sub_packet_h/2; x++)
-                    if (get_buffer(pb, ast->pkt.data+x*2*ast->audio_framesize+y*ast->coded_framesize, ast->coded_framesize) <= 0)
-                        return AVERROR(EIO);
-            rm->audio_stream_num = 0;
-            rm->audio_pkt_cnt = ast->sub_packet_h * ast->audio_framesize / st->codec->block_align - 1;
-            // Release first audio packet
-            av_new_packet(pkt, st->codec->block_align);
-            memcpy(pkt->data, ast->pkt.data, st->codec->block_align); //FIXME avoid this
-            pkt->flags |= PKT_FLAG_KEY; // Mark first packet as keyframe
-            pkt->stream_index = 0;
-        } else {
-            /* just read raw bytes */
-            len = RAW_PACKET_SIZE;
-            len= av_get_packet(pb, pkt, len);
-            pkt->stream_index = 0;
-            if (len <= 0) {
-                return AVERROR(EIO);
-            }
-            pkt->size = len;
-        }
-        rm_ac3_swap_bytes(st, pkt);
+            timestamp = AV_NOPTS_VALUE;
+            len = ast->audio_framesize ?
+                ast->coded_framesize * ast->sub_packet_h / 2 : RAW_PACKET_SIZE;
+            flags = (seq++ == 1) ? 2 : 0;
     } else {
-        int seq=1;
-resync:
         len=sync(s, &timestamp, &flags, &i, &pos);
-        if(len<0)
-            return AVERROR(EIO);
         st = s->streams[i];
+        }
+
+        if(len<0 || url_feof(s->pb))
+            return AVERROR(EIO);
 
         if (ff_rm_parse_packet (s, s->pb, st, st->priv_data, len, pkt,
                                 &seq, &flags, &timestamp) < 0)
-            goto resync;
+            continue;
+    }
 
         if(  (st->discard >= AVDISCARD_NONKEY && !(flags&2))
            || st->discard >= AVDISCARD_ALL){
             av_free_packet(pkt);
-            while (rm->audio_pkt_cnt > 0) {
-                ff_rm_retrieve_cache(s, s->pb, st, st->priv_data, pkt);
-                av_free_packet(pkt);
-            }
-            goto resync;
-        }
+        } else
+            break;
+    }
 
         if((flags&2) && (seq&0x7F) == 1)
             av_add_index_entry(st, pos, timestamp, 0, 0, AVINDEX_KEYFRAME);
-    }
 
     return 0;
 }