[FFmpeg-devel] [PATCH 2/3] avformat/matroskadec: Don't discard valid packets

[Andreas Rheinhardt]

A Block (meaning both a Block in a BlockGroup as well as a SimpleBlock)
must have at least three bytes after the field containing the encoded
TrackNumber. So a if there are <= 3 bytes, the Matroska demuxer would
skip this block, believing it to be an empty, but valid Block.

This might discard valid Blocks, even nonempty ones (namely if the track
uses header stripping). And certain definitely spec-incompliant Blocks
don't raise errors: Those with two or less bytes left after the encoded
TrackNumber and those with three bytes left, but with flags indicating
that the Block uses lacing (in which case there has to be further data
describing the lacing).

Furthermore, zero-sized packets were still possible because only the
size of the last entry of a lace was checked.

This commit fixes this. All spec-compliant Blocks are now returned to
the caller, even those with a size of zero. This is in accordance with the
documentation of av_read_frame(): "This function returns what is stored
in the file, and does not validate that what is there are valid frames
for the decoder".

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
mkclean can produce blocks where the payload has a size of zero before
readding the removed header. E.g. it does this if a stream only has one
frame in total (although in such a case the overhead of header removal
compression is greater than the number of bytes saved in the Blocks);
this can in particular happen with forced subtitle tracks with only one
frame (which is how I noticed this).

I am unsure what to do with size-zero packets; I don't know any
Matroska Codec Mapping where this would be allowed. But there is nothing
explicitly stating that they are generally illegal, so this commit
returns them. The only thing I am certain is that stripping these
packets away needs to happen after the content compressions have been
reversed.

 libavformat/matroskadec.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 6425973954..7aea13dda0 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2991,7 +2991,9 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
         return 0;
     }
 
-    av_assert0(size > 0);
+    if (size <= 0)
+        return AVERROR_INVALIDDATA;
+
     *laces    = *data + 1;
     data     += 1;
     size     -= 1;
@@ -3017,7 +3019,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
                     break;
             }
         }
-        if (size <= total) {
+        if (size < total) {
             return AVERROR_INVALIDDATA;
         }
 
@@ -3064,7 +3066,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
         }
         data += offset;
         size -= offset;
-        if (size <= total) {
+        if (size < total) {
             return AVERROR_INVALIDDATA;
         }
         lace_size[*laces - 1] = size - total;
@@ -3524,8 +3526,8 @@ static int matroska_parse_block(MatroskaDemuxContext *matroska, AVBufferRef *buf
         av_log(matroska->ctx, AV_LOG_INFO,
                "Invalid stream %"PRIu64"\n", num);
         return AVERROR_INVALIDDATA;
-    } else if (size <= 3)
-        return 0;
+    } else if (size < 3)
+        return AVERROR_INVALIDDATA;
     st = track->stream;
     if (st->discard >= AVDISCARD_ALL)
         return res;
-- 
2.20.1