On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
Before this commit, the Matroska muxer would read a block when required
to do so, parse the block, create and return the necessary AVPackets and
yet keep the blocks (in a dynamically allocated list), although they
aren't used at all any more. This has been changed. There is no list any
more and the block is immediately discarded after parsing.

My understanding of the code is that the blocks are put in a queue, that's whatwebm_clusters_start_with_keyframe() uses to check that the first frame is a keyframe (it doesn't check the type of the frame though...). But since there's only one read inmatroska_parse_cluster_incremental()there's only one kept in memory.

So LGTM.


Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@googlemail.com>
---
  libavformat/matroskadec.c | 87 +++++++++++++++++----------------------
  1 file changed, 38 insertions(+), 49 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 9198fa1bea..997c96d78f 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -304,9 +304,20 @@ typedef struct MatroskaLevel {
      uint64_t length;
  } MatroskaLevel;
+typedef struct MatroskaBlock {
+    uint64_t duration;
+    int64_t  reference;
+    uint64_t non_simple;
+    EbmlBin  bin;
+    uint64_t additional_id;
+    EbmlBin  additional;
+    int64_t discard_padding;
+} MatroskaBlock;
+
  typedef struct MatroskaCluster {
+    MatroskaBlock block;
      uint64_t timecode;
-    EbmlList blocks;
+    int64_t pos;
  } MatroskaCluster;
typedef struct MatroskaLevel1Element {
@@ -356,8 +367,6 @@ typedef struct MatroskaDemuxContext {
      MatroskaLevel1Element level1_elems[64];
      int num_level1_elems;
- int current_cluster_num_blocks;
-    int64_t current_cluster_pos;
      MatroskaCluster current_cluster;
/* WebM DASH Manifest live flag */
@@ -367,16 +376,6 @@ typedef struct MatroskaDemuxContext {
      int bandwidth;
  } MatroskaDemuxContext;
-typedef struct MatroskaBlock {
-    uint64_t duration;
-    int64_t  reference;
-    uint64_t non_simple;
-    EbmlBin  bin;
-    uint64_t additional_id;
-    EbmlBin  additional;
-    int64_t discard_padding;
-} MatroskaBlock;
-
  static const EbmlSyntax ebml_header[] = {
      { EBML_ID_EBMLREADVERSION,    EBML_UINT, 0, offsetof(Ebml, version),      
   { .u = EBML_VERSION } },
      { EBML_ID_EBMLMAXSIZELENGTH,  EBML_UINT, 0, offsetof(Ebml, max_size),     
   { .u = 8 } },
@@ -705,9 +704,9 @@ static const EbmlSyntax matroska_blockgroup[] = {
  };
static const EbmlSyntax matroska_cluster_parsing[] = {
-    { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,                     
offsetof(MatroskaCluster, timecode) },
-    { MATROSKA_ID_BLOCKGROUP,      EBML_NEST, sizeof(MatroskaBlock), 
offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
-    { MATROSKA_ID_SIMPLEBLOCK,     EBML_PASS, sizeof(MatroskaBlock), 
offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
+    { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster, 
timecode) },
+    { MATROSKA_ID_BLOCKGROUP,      EBML_NEST, 0, 0, { .n = matroska_blockgroup 
} },
+    { MATROSKA_ID_SIMPLEBLOCK,     EBML_PASS, 0, 0, { .n = matroska_blockgroup 
} },
      { MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
      { MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
      { MATROSKA_ID_INFO,            EBML_NONE },
@@ -3443,57 +3442,48 @@ end:
static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
  {
-    EbmlList *blocks_list;
-    MatroskaBlock *blocks;
-    int i, res;
+    MatroskaCluster *cluster = &matroska->current_cluster;
+    MatroskaBlock     *block = &cluster->block;
+    int res;
      res = ebml_parse(matroska,
                       matroska_cluster_parsing,
-                     &matroska->current_cluster);
+                     cluster);
      if (res == 1) {
          /* New Cluster */
-        if (matroska->current_cluster_pos)
+        if (cluster->pos)
              ebml_level_end(matroska);
-        ebml_free(matroska_cluster_parsing, &matroska->current_cluster);
-        memset(&matroska->current_cluster, 0, sizeof(MatroskaCluster));
-        matroska->current_cluster_num_blocks = 0;
-        matroska->current_cluster_pos        = avio_tell(matroska->ctx->pb);
+        cluster->pos = avio_tell(matroska->ctx->pb);
          /* sizeof the ID which was already read */
          if (matroska->current_id)
-            matroska->current_cluster_pos -= 4;
+            cluster->pos -= 4;
          res = ebml_parse(matroska,
                           matroska_clusters,
-                         &matroska->current_cluster);
+                         cluster);
          /* Try parsing the block again. */
          if (res == 1)
              res = ebml_parse(matroska,
                               matroska_cluster_parsing,
-                             &matroska->current_cluster);
+                             cluster);
      }
- if (!res &&
-        matroska->current_cluster_num_blocks <
-        matroska->current_cluster.blocks.nb_elem) {
-        blocks_list = &matroska->current_cluster.blocks;
-        blocks      = blocks_list->elem;
+    if (!res && block->bin.size > 0) {
+            int is_keyframe = block->non_simple ? block->reference == 
INT64_MIN : -1;
+            uint8_t* additional = block->additional.size > 0 ?
+                                    block->additional.data : NULL;
- matroska->current_cluster_num_blocks = blocks_list->nb_elem;
-        i                                    = blocks_list->nb_elem - 1;
-        if (blocks[i].bin.size > 0 && blocks[i].bin.data) {
-            int is_keyframe = blocks[i].non_simple ? blocks[i].reference == 
INT64_MIN : -1;
-            uint8_t* additional = blocks[i].additional.size > 0 ?
-                                    blocks[i].additional.data : NULL;
-
-            res = matroska_parse_block(matroska, blocks[i].bin.buf, 
blocks[i].bin.data,
-                                       blocks[i].bin.size, blocks[i].bin.pos,
+            res = matroska_parse_block(matroska, block->bin.buf, 
block->bin.data,
+                                       block->bin.size, block->bin.pos,
                                         matroska->current_cluster.timecode,
-                                       blocks[i].duration, is_keyframe,
-                                       additional, blocks[i].additional_id,
-                                       blocks[i].additional.size,
-                                       matroska->current_cluster_pos,
-                                       blocks[i].discard_padding);
-        }
+                                       block->duration, is_keyframe,
+                                       additional, block->additional_id,
+                                       block->additional.size,
+                                       cluster->pos,
+                                       block->discard_padding);
      }
+ ebml_free(matroska_blockgroup, block);
+    memset(block, 0, sizeof(*block));
+
      return res;
  }
@@ -3591,7 +3581,6 @@ static int matroska_read_close(AVFormatContext *s)
      for (n = 0; n < matroska->tracks.nb_elem; n++)
          if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)
              av_freep(&tracks[n].audio.buf);
-    ebml_free(matroska_cluster_parsing, &matroska->current_cluster);
      ebml_free(matroska_segment, matroska);
return 0;
--
2.19.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to