diff mbox series

[FFmpeg-devel,4/9] avformat/smacker: Stop caching and copying audio frames

Message ID 20200624134705.14833-4-andreas.rheinhardt@gmail.com
State Accepted
Commit 316dc0674eeb3419e560913ac9bc130b6dae919a
Headers show
Series [FFmpeg-devel,1/9] avformat/smacker: Don't increase packet counter prematurely
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt June 24, 2020, 1:47 p.m. UTC
The layout of a Smacker frame is as follows: For some frames, the
beginning of the frame contained a palette for the video stream; then
there are potentially several audio frames, followed by the data for the
video stream.

The Smacker demuxer used to read the palette, then cache every audio frame
into a buffer (that gets reallocated to the desired size every time a
frame is read into this buffer), then read and return the video frame
(together with the palette). The cached audio frames are then returned
by copying the data into freshly allocated buffers; if there are none
left, the next frame is read.

This commit changes this: At the beginning of a frame, the palette is
read and cached as now. But audio frames are no longer cached at all;
they are returned immediately. This gets rid of copying and also allows
to remove the code for the buffer-to-AVStream correspondence.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/smacker.c | 94 ++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 51 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/smacker.c b/libavformat/smacker.c
index c803ecbec9..4990b7d920 100644
--- a/libavformat/smacker.c
+++ b/libavformat/smacker.c
@@ -48,14 +48,16 @@  typedef struct SmackerContext {
     uint32_t *frm_size;
     uint8_t  *frm_flags;
     /* internal variables */
+    int64_t next_frame_pos;
     int cur_frame;
     /* current frame for demuxing */
+    uint32_t frame_size;
+    int flags;
+    int next_audio_index;
+    int new_palette;
     uint8_t pal[768];
     int indexes[7];
     int videoindex;
-    uint8_t *bufs[7];
-    int buf_sizes[7];
-    int stream_id[7];
     int curstream;
     int64_t aud_pts[7];
 } SmackerContext;
@@ -235,21 +237,19 @@  static int smacker_read_header(AVFormatContext *s)
 static int smacker_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     SmackerContext *smk = s->priv_data;
-    int64_t next_frame_pos;
     int flags;
     int ret;
-    int i;
-    int frame_size = 0;
-    int palchange = 0;
 
     if (avio_feof(s->pb) || smk->cur_frame >= smk->frames)
         return AVERROR_EOF;
 
     /* if we demuxed all streams, pass another frame */
-    if (smk->curstream <= 0) {
-        frame_size = smk->frm_size[smk->cur_frame] & (~3);
-        next_frame_pos = avio_tell(s->pb) + (unsigned)frame_size;
+    if (!smk->next_audio_index) {
+        smk->frame_size = smk->frm_size[smk->cur_frame] & (~3);
+        smk->next_frame_pos = avio_tell(s->pb) + smk->frame_size;
+        smk->curstream = 0;
         flags = smk->frm_flags[smk->cur_frame];
+        smk->flags = flags >> 1;
         /* handle palette change event */
         if(flags & SMACKER_PAL){
             int size, sz, t, off, j, pos;
@@ -258,13 +258,12 @@  static int smacker_read_packet(AVFormatContext *s, AVPacket *pkt)
 
             memcpy(oldpal, pal, 768);
             size = avio_r8(s->pb);
-            size = size * 4 - 1;
-            if (size + 1 > frame_size) {
+            size = size * 4;
+            if (size > smk->frame_size) {
                 ret = AVERROR_INVALIDDATA;
                 goto next_frame;
             }
-            frame_size -= size;
-            frame_size--;
+            smk->frame_size -= size--;
             sz = 0;
             pos = avio_tell(s->pb) + size;
             while(sz < 256){
@@ -298,69 +297,65 @@  static int smacker_read_packet(AVFormatContext *s, AVPacket *pkt)
                 }
             }
             avio_seek(s->pb, pos, 0);
-            palchange |= 1;
+            smk->new_palette = 1;
         }
-        flags >>= 1;
-        smk->curstream = 0;
-        /* if audio chunks are present, put them to stack and retrieve later */
-        for(i = 0; i < 7; i++) {
-            if(flags & 1) {
+    }
+
+    for (int i = smk->next_audio_index; i < 7; i++) {
+        if (smk->flags & (1 << i)) {
                 uint32_t size;
 
                 size = avio_rl32(s->pb);
-                if ((int)size < 8 || size > frame_size) {
+            if ((int)size < 8 || size > smk->frame_size) {
                     av_log(s, AV_LOG_ERROR, "Invalid audio part size\n");
                     ret = AVERROR_INVALIDDATA;
                     goto next_frame;
                 }
-                frame_size -= size;
+            smk->frame_size -= size;
                 size       -= 4;
-                if ((ret = av_reallocp(&smk->bufs[smk->curstream], size)) < 0) {
-                    smk->buf_sizes[smk->curstream] = 0;
+
+            if (smk->indexes[i] < 0) {
+                avio_skip(s->pb, size);
+                continue;
+            }
+            if ((ret = av_get_packet(s->pb, pkt, size)) != size) {
+                ret = ret < 0 ? ret : AVERROR_INVALIDDATA;
                     goto next_frame;
                 }
-                smk->buf_sizes[smk->curstream] = size;
-                ret = ffio_read_size(s->pb, smk->bufs[smk->curstream], size);
-                if (ret < 0)
-                    goto next_frame;
-                smk->stream_id[smk->curstream] = smk->indexes[i];
+            pkt->stream_index = smk->indexes[i];
+            pkt->pts = smk->aud_pts[smk->curstream];
+            smk->aud_pts[smk->curstream] += AV_RL32(pkt->data);
+            smk->next_audio_index = i + 1;
                 smk->curstream++;
+            return 0;
             }
-            flags >>= 1;
         }
-        if (frame_size < 0 || frame_size >= INT_MAX/2) {
+
+    if (smk->frame_size >= INT_MAX/2) {
             ret = AVERROR_INVALIDDATA;
             goto next_frame;
         }
-        if ((ret = av_new_packet(pkt, frame_size + 769)) < 0)
+    if ((ret = av_new_packet(pkt, smk->frame_size + 769)) < 0)
             goto next_frame;
+    flags = smk->new_palette;
         if(smk->frm_size[smk->cur_frame] & 1)
-            palchange |= 2;
-        pkt->data[0] = palchange;
+        flags |= 2;
+    pkt->data[0] = flags;
         memcpy(pkt->data + 1, smk->pal, 768);
-        ret = ffio_read_size(s->pb, pkt->data + 769, frame_size);
+    ret = ffio_read_size(s->pb, pkt->data + 769, smk->frame_size);
         if (ret < 0)
             goto next_frame;
         pkt->stream_index = smk->videoindex;
         pkt->pts          = smk->cur_frame;
         pkt->size = ret + 769;
+    smk->next_audio_index = 0;
+    smk->new_palette = 0;
         smk->cur_frame++;
-    } else {
-        smk->curstream--;
-        if (smk->stream_id[smk->curstream] < 0 || !smk->bufs[smk->curstream])
-            return AVERROR_INVALIDDATA;
-        if ((ret = av_new_packet(pkt, smk->buf_sizes[smk->curstream])) < 0)
-            return ret;
-        memcpy(pkt->data, smk->bufs[smk->curstream], smk->buf_sizes[smk->curstream]);
-        pkt->size = smk->buf_sizes[smk->curstream];
-        pkt->stream_index = smk->stream_id[smk->curstream];
-        pkt->pts = smk->aud_pts[smk->curstream];
-        smk->aud_pts[smk->curstream] += AV_RL32(pkt->data);
-    }
 
     return 0;
 next_frame:
-    avio_seek(s->pb, next_frame_pos, SEEK_SET);
+    avio_seek(s->pb, smk->next_frame_pos, SEEK_SET);
+    smk->next_audio_index = 0;
     smk->cur_frame++;
     return ret;
 }
@@ -368,10 +363,7 @@  next_frame:
 static int smacker_read_close(AVFormatContext *s)
 {
     SmackerContext *smk = s->priv_data;
-    int i;
 
-    for(i = 0; i < 7; i++)
-        av_freep(&smk->bufs[i]);
     av_freep(&smk->frm_size);
     av_freep(&smk->frm_flags);