[FFmpeg-devel] [PATCH] avformat/matroskadec: use a linked list to queue packets

[James Almer]

It's more robust and efficient.

Signed-off-by: James Almer <jamrial at gmail.com>
---
 libavformat/matroskadec.c | 90 +++++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 38 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 2faaf9dfb8..241ee5fed1 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -338,9 +338,8 @@ typedef struct MatroskaDemuxContext {
     int64_t segment_start;
 
     /* the packet queue */
-    AVPacket **packets;
-    int num_packets;
-    AVPacket *prev_pkt;
+    AVPacketList *queue;
+    AVPacketList *queue_end;
 
     int done;
 
@@ -2722,11 +2721,14 @@ fail:
 static int matroska_deliver_packet(MatroskaDemuxContext *matroska,
                                    AVPacket *pkt)
 {
-    if (matroska->num_packets > 0) {
+    if (matroska->queue) {
         MatroskaTrack *tracks = matroska->tracks.elem;
         MatroskaTrack *track;
-        memcpy(pkt, matroska->packets[0], sizeof(AVPacket));
-        av_freep(&matroska->packets[0]);
+        AVPacketList *pktl = matroska->queue;
+
+        av_packet_move_ref(pkt, &pktl->pkt);
+        matroska->queue = pktl->next;
+        av_free(pktl);
         track = &tracks[pkt->stream_index];
         if (track->has_palette) {
             uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE);
@@ -2737,41 +2739,45 @@ static int matroska_deliver_packet(MatroskaDemuxContext *matroska,
             }
             track->has_palette = 0;
         }
-        if (matroska->num_packets > 1) {
-            void *newpackets;
-            memmove(&matroska->packets[0], &matroska->packets[1],
-                    (matroska->num_packets - 1) * sizeof(AVPacket *));
-            newpackets = av_realloc(matroska->packets,
-                                    (matroska->num_packets - 1) *
-                                    sizeof(AVPacket *));
-            if (newpackets)
-                matroska->packets = newpackets;
-        } else {
-            av_freep(&matroska->packets);
-            matroska->prev_pkt = NULL;
-        }
-        matroska->num_packets--;
+        if (!matroska->queue)
+            matroska->queue_end = NULL;
         return 0;
     }
 
     return -1;
 }
 
+static int matroska_queue_packet(MatroskaDemuxContext *matroska, AVPacket *pkt)
+{
+    AVPacketList *pktl = av_malloc(sizeof(*pktl));
+
+    if (!pktl)
+        return AVERROR(ENOMEM);
+    av_packet_move_ref(&pktl->pkt, pkt);
+    pktl->next = NULL;
+
+    if (matroska->queue_end)
+        matroska->queue_end->next = pktl;
+    else
+        matroska->queue = pktl;
+    matroska->queue_end = pktl;
+
+    return 0;
+}
+
 /*
  * Free all packets in our internal queue.
  */
 static void matroska_clear_queue(MatroskaDemuxContext *matroska)
 {
-    matroska->prev_pkt = NULL;
-    if (matroska->packets) {
-        int n;
-        for (n = 0; n < matroska->num_packets; n++) {
-            av_packet_unref(matroska->packets[n]);
-            av_freep(&matroska->packets[n]);
-        }
-        av_freep(&matroska->packets);
-        matroska->num_packets = 0;
+    AVPacketList *pktl;
+
+    while (pktl = matroska->queue) {
+        av_packet_unref(&pktl->pkt);
+        matroska->queue = pktl->next;
+        av_free(pktl);
     }
+    matroska->queue_end = NULL;
 }
 
 static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
@@ -2953,7 +2959,11 @@ static int matroska_parse_rm_audio(MatroskaDemuxContext *matroska,
         track->audio.buf_timecode = AV_NOPTS_VALUE;
         pkt->pos                  = pos;
         pkt->stream_index         = st->index;
-        dynarray_add(&matroska->packets, &matroska->num_packets, pkt);
+        ret = matroska_queue_packet(matroska, pkt);
+        if (ret < 0) {
+            av_packet_free(&pkt);
+            return AVERROR(ENOMEM);
+        }
     }
 
     return 0;
@@ -3152,8 +3162,11 @@ static int matroska_parse_webvtt(MatroskaDemuxContext *matroska,
     pkt->duration = duration;
     pkt->pos = pos;
 
-    dynarray_add(&matroska->packets, &matroska->num_packets, pkt);
-    matroska->prev_pkt = pkt;
+    err = matroska_queue_packet(matroska, pkt);
+    if (err < 0) {
+        av_packet_free(&pkt);
+        return AVERROR(ENOMEM);
+    }
 
     return 0;
 }
@@ -3268,8 +3281,11 @@ FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
-    dynarray_add(&matroska->packets, &matroska->num_packets, pkt);
-    matroska->prev_pkt = pkt;
+    res = matroska_queue_packet(matroska, pkt);
+    if (res < 0) {
+        av_packet_free(&pkt);
+        return AVERROR(ENOMEM);
+    }
 
     return 0;
 
@@ -3433,7 +3449,6 @@ static int matroska_parse_cluster_incremental(MatroskaDemuxContext *matroska)
         memset(&matroska->current_cluster, 0, sizeof(MatroskaCluster));
         matroska->current_cluster_num_blocks = 0;
         matroska->current_cluster_pos        = avio_tell(matroska->ctx->pb);
-        matroska->prev_pkt                   = NULL;
         /* sizeof the ID which was already read */
         if (matroska->current_id)
             matroska->current_cluster_pos -= 4;
@@ -3486,7 +3501,6 @@ static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
     if (!matroska->contains_ssa)
         return matroska_parse_cluster_incremental(matroska);
     pos = avio_tell(matroska->ctx->pb);
-    matroska->prev_pkt = NULL;
     if (matroska->current_id)
         pos -= 4;  /* sizeof the ID which was already read */
     res         = ebml_parse(matroska, matroska_clusters, &cluster);
@@ -3671,10 +3685,10 @@ static int webm_clusters_start_with_keyframe(AVFormatContext *s)
         matroska->current_id = 0;
         matroska_clear_queue(matroska);
         if (matroska_parse_cluster(matroska) < 0 ||
-            matroska->num_packets <= 0) {
+            !matroska->queue) {
             break;
         }
-        pkt = matroska->packets[0];
+        pkt = &matroska->queue->pkt;
         cluster_pos += cluster_length + 12; // 12 is the offset of the cluster id and length.
         if (!(pkt->flags & AV_PKT_FLAG_KEY)) {
             rv = 0;
-- 
2.16.2