[FFmpeg-devel] [PATCH 2/8] avformat/matroskadec: Make code more robust

[Andreas Rheinhardt]

8a286e74 made sure that when a seek is performed to parse a level 1
element, the appropriate level is set (a special function for this
purpose is used: matroska_reset_status). This function can fail just
like the seeks that it replaced; and just like the seeks that it replaced,
it was left unchecked.
This became a problem with 865c5370, because with this commit the level
has explicitly been used in order to determine how to parse further.
This meant that when these seeks failed, the level was not updated. And
in certain edge cases it was possible to run into a situation where a
call to matroska_parse_cluster was performed when the level was > 2;
but this mustn't happen (as one doesn't know how to parse in this
situation) and therefore there was an assert for this.

The obvious way to check for this is by checking all the calls to
matroska_reset_status, i.e. by erroring out if the seek failed and let
the generic seeking code take over.
Unfortunately, this would break seeking forward in situations where
the AVIOContext can only be seeked forward and where no index to seek
is available: In this situation matroska_read_seek would first attempt
to seek back to the cluster of the last keyframe and begin reading the
stream from there for the next keyframe. Said seek would fail in this
scenario and parsing would commence from the current position (it works
because the level has not been modified yet) until it finds a keyframe
whose pts is big enough. So matroska_read_seek works despite the seeking
failures.
But when one lets the generic seeking code take over, it will try to
seek to the last keyframe and fail just as well and the seek itself
will fail.

So another solution to this problem is necessary. It is surprisingly
simple: Instead of asserting that the level is <= 2, resync when it is
not; after all, an error must have happened already.

Chromium bug 995833 was about this assert; of course, one can no longer
run into it after this commit.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
Honestly, chromium bug 995833 now runs into another bug (a broken pipe
(SIGPIPE)) outside of the FFmpeg related code.

 libavformat/matroskadec.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 0c881a67e8..47386b2631 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -3676,7 +3676,13 @@ static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
     MatroskaBlock     *block = &cluster->block;
     int res;
 
-    av_assert0(matroska->num_levels <= 2);
+    if (matroska->num_levels > 2) {
+        /* This can only happen if a reset has failed.
+         * So resync now in order to be able to parse again. */
+         res = matroska_resync(matroska, avio_tell(matroska->ctx->pb));
+         if (res < 0)
+             return res;
+    }
 
     if (matroska->num_levels == 1) {
         res = ebml_parse(matroska, matroska_segment, NULL);
-- 
2.21.0