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

[Anton Khirnov]

Quoting Andreas Rheinhardt (2020-03-26 01:41:43)
> 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.

Maybe they are not illegal in matroska, but I have strong doubts that
they would be illegal to export from a demuxer. We do have packets with
no actual data (only side data), but those still contain _something_.
I suspect many users wouldn't handle outright empty packets well.

-- 
Anton Khirnov