diff mbox

[FFmpeg-devel,17/37] avformat/matroskadec: Don't abort resyncing upon seek failure

Message ID 20190516223018.30827-18-andreas.rheinhardt@gmail.com
State Accepted
Commit 27f40b1dcdff426ed25e7c690ec907d18239ff49
Headers show

Commit Message

Andreas Rheinhardt May 16, 2019, 10:30 p.m. UTC
When an error happens, the Matroska demuxer tries to resync to level 1
elements from an earlier position onwards. If the seek to said earlier
position fails, the demuxer currently treats this as an unrecoverable
error. And that behaviour is suboptimal as said failure is nothing
unrecoverable or unexpected (when the input isn't seekable).
It is preferable to simply resync from the earliest position available
(i.e. the start of the AVIOContext's buffer) onwards if the seek failed.

Here are some scenarios that might be treated as unrecoverable errors
by the current code if the input isn't seekable. They all have in
common that the current position is so far away from the desired
position that the seek can't be fulfilled from the AVIOContext's buffer:

1. Blocks (both SimpleBlocks as well as a Block in a BlockGroup) for
which reading them as binary EBML elements succeeds, but whose parsing
triggers an error (e.g. an invalid TrackNumber). In this case the
earlier position from which resyncing begins is at the start of the block
(or even earlier).
2. BlockGroups, whose parsing fails in one of the latter elements. Just
as in 1., the start of the BlockGroup (the target of the seek) might be
so far away from the current position that it is no longer in the
3. At the beginning of parsing a cluster, the cluster is parsed until a
SimpleBlock or a BlockGroup is encountered. So if the input is damaged
between the beginning of the cluster and the first occurrence of a
SimpleBlock/BlockGroup and if said damage makes the demuxer read/skip so
much data that the beginning of the cluster is no longer in the buffer,
demuxing will currently fail completely.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
 libavformat/matroskadec.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
diff mbox


diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 93bf72661c..b14f168c2c 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -742,15 +742,18 @@  static int matroska_read_close(AVFormatContext *s);
 static int matroska_resync(MatroskaDemuxContext *matroska, int64_t last_pos)
     AVIOContext *pb = matroska->ctx->pb;
-    int64_t ret;
     uint32_t id;
     matroska->current_id = 0;
     matroska->num_levels = 0;
-    /* seek to next position to resync from */
-    if ((ret = avio_seek(pb, last_pos + 1, SEEK_SET)) < 0) {
-        matroska->done = 1;
-        return ret;
+    /* Try to seek to the last position to resync from. If this doesn't work,
+     * we resync from the earliest position available: The start of the buffer. */
+    if (last_pos < avio_tell(pb) && avio_seek(pb, last_pos + 1, SEEK_SET) < 0) {
+        av_log(matroska->ctx, AV_LOG_WARNING,
+               "Seek to desired resync point failed. Seeking to "
+               "earliest point available instead.\n");
+        avio_seek(pb, FFMAX(avio_tell(pb) + (pb->buffer - pb->buf_ptr),
+                            last_pos + 1), SEEK_SET);
     id = avio_rb32(pb);
@@ -768,7 +771,7 @@  static int matroska_resync(MatroskaDemuxContext *matroska, int64_t last_pos)
     matroska->done = 1;
-    return AVERROR_EOF;
+    return pb->error ? pb->error : AVERROR_EOF;