[FFmpeg-devel] [PATCH 2/3] avformat/matroskadec: Improve handling of circular SeekHeads

[Andreas Rheinhardt]

There can be more than one SeekHead in a Matroska file, but most of the
other level 1 elements can only occur once.* Therefore the Matroska
demuxer only allows one entry per ID in its internal list of level 1
elements known to it; the only exception to this are SeekHeads.

The only exception to this are SeekHeads: When one is encountered
(either directly or in the list of entries read from SeekHeads),
a new entry in the list of known level-1 elements is always added,
even when this entry is actually already known.

This leads to lots of seeks in case of circular SeekHeads: Each time a
SeekHead is parsed, a new entry for a SeekHead will be added to the list
of entries read from SeekHeads. The exception for SeekHeads mentioned
above now implies that this SeekHead will always appear new and unparsed
and parsing will be attempted. This continued until the list of known
level-1 elements is full.

Fixing this is pretty simple: Don't add a new entry for a SeekHead if
its position matches the position of an already known SeekHead.

*: Actually, there can be multiple Tags and several other level 1
elements are "identically recurring" which means they may be resent
multiple times, but each instance must be absolutely identical to the
previous.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
 libavformat/matroskadec.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index dea8f14f9e..25eed210c5 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1134,7 +1134,7 @@ static int is_ebml_id_valid(uint32_t id)
  * an entry already exists, return the existing entry.
  */
 static MatroskaLevel1Element *matroska_find_level1_elem(MatroskaDemuxContext *matroska,
-                                                        uint32_t id)
+                                                        uint32_t id, int64_t pos)
 {
     int i;
     MatroskaLevel1Element *elem;
@@ -1147,18 +1147,17 @@ static MatroskaLevel1Element *matroska_find_level1_elem(MatroskaDemuxContext *ma
         return NULL;
 
     // There can be multiple seekheads.
-    if (id != MATROSKA_ID_SEEKHEAD) {
-        for (i = 0; i < matroska->num_level1_elems; i++) {
-            if (matroska->level1_elems[i].id == id)
+    for (i = 0; i < matroska->num_level1_elems; i++) {
+        if (matroska->level1_elems[i].id == id) {
+            if (matroska->level1_elems[i].pos == pos ||
+                id != MATROSKA_ID_SEEKHEAD)
                 return &matroska->level1_elems[i];
         }
     }
 
     // Only a completely broken file would have more elements.
-    // It also provides a low-effort way to escape from circular seekheads
-    // (every iteration will add a level1 entry).
     if (matroska->num_level1_elems >= FF_ARRAY_ELEMS(matroska->level1_elems)) {
-        av_log(matroska->ctx, AV_LOG_ERROR, "Too many level1 elements or circular seekheads.\n");
+        av_log(matroska->ctx, AV_LOG_ERROR, "Too many level1 elements.\n");
         return NULL;
     }
 
@@ -1407,7 +1406,7 @@ static int ebml_parse(MatroskaDemuxContext *matroska,
         if (id == MATROSKA_ID_CUES)
             matroska->cues_parsing_deferred = 0;
         if (syntax->type == EBML_LEVEL1 &&
-            (level1_elem = matroska_find_level1_elem(matroska, syntax->id))) {
+            (level1_elem = matroska_find_level1_elem(matroska, syntax->id, pos))) {
             if (!level1_elem->pos) {
                 // Zero is not a valid position for a level 1 element.
                 level1_elem->pos = pos;
@@ -1870,7 +1869,7 @@ static void matroska_execute_seekhead(MatroskaDemuxContext *matroska)
         if (id != seekheads[i].id || pos < matroska->segment_start)
             continue;
 
-        elem = matroska_find_level1_elem(matroska, id);
+        elem = matroska_find_level1_elem(matroska, id, pos);
         if (!elem || elem->parsed)
             continue;
 
-- 
2.20.1