[FFmpeg-devel] [PATCH v2 07/30] avformat/matroskaenc: Avoid allocations for SeekHead

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Up until e7ddafd5, the Matroska muxer wrote two SeekHeads: One at the
> beginning referencing the main level 1 elements (i.e. not the Clusters)
> and one at the end, referencing the Clusters. This second SeekHead was
> useless and has therefore been removed. Yet the SeekHead-related
> functions and structures are still geared towards this usecase: They
> are built around an allocated array of variable size that gets
> reallocated every time an element is added to it although the maximum
> number of Seek entries is a small compile-time constant, so that one should
> rather include the array in the SeekHead structure itself; and said
> structure should be contained in the MatroskaMuxContext instead of being
> allocated separately.
> 
> The earlier code reserved space for a SeekHead with 10 entries, although
> we currently write at most 6. Reducing said number implied that every
> Matroska/Webm file will be 84 bytes smaller and required to adapt
> several FATE tests; furthermore, the reserved amount overestimated the
> amount needed for for the SeekHead's length field and how many bytes
> need to be reserved to write a EBML Void element, bringing the total
> reduction to 89 bytes.
> 
> This also fixes a potential segfault: If !mkv->is_live and if the
> AVIOContext is initially unseekable when writing the header, the
> SeekHead is already written when writing the header and this used to
> free the SeekHead-related structures that have been allocated. But if
> the AVIOContext happens to be seekable when writing the trailer, it will
> be attempted to write the SeekHead again which will lead to segfaults
> because the corresponding structures have already been freed.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> 1. Changing seekability doesn't seem to happen in real life: Before
> add68dcc (merged last April) the writing process for all level 1
> elements was different depending upon whether the output was seekable
> or not; if it switched between seekable and unseekable, the output would
> be corrupt, yet no one reported this.
> 2. Only the commit message has been modified; the content of this patch
> is unchanged since last time.
> 3. Changing seekability would still not go well. I'll work on it.
> 
>  libavformat/matroskaenc.c            | 119 ++++++++-------------------
>  tests/fate/matroska.mak              |   2 +-
>  tests/fate/wavpack.mak               |   4 +-
>  tests/ref/fate/aac-autobsf-adtstoasc |   4 +-
>  tests/ref/fate/binsub-mksenc         |   2 +-
>  tests/ref/fate/rgb24-mkv             |   4 +-
>  tests/ref/lavf-fate/av1.mkv          |   4 +-
>  tests/ref/lavf/mka                   |   4 +-
>  tests/ref/lavf/mkv                   |   4 +-
>  tests/ref/lavf/mkv_attachment        |   4 +-
>  tests/ref/seek/lavf-mkv              |  44 +++++-----
>  11 files changed, 73 insertions(+), 122 deletions(-)
> 
Ping. (The actual patch has been left out because of its length, but it
can be found at [1].)

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/258390.html