[FFmpeg-devel] [PATCH 3/6] avformat/matroskaenc: Split updating CodecPrivate from writing it

[Andreas Rheinhardt]

Michael Niedermayer:
> On Tue, Jun 21, 2022 at 04:34:15AM +0200, Andreas Rheinhardt wrote:
>> Up until now, updating extradata was very ad-hoc: The amount of
>> space reserved for extradata was not recorded when writing the
>> header; instead the AAC code simply presumed that it was enough.
>> This commit changes this by recording how much space is available.
>>
>> This brings with it that the code for writing of and reserving space
>> for the CodecPrivate and for updating it diverges. They are therefore
>> split; this allows to put other common tasks like seeking to
>> right offset as well as writing padding (in case the new extradata did
>> not fill the whole reserved space) to this common function.
>>
>> The code for filling up the reserved space is smarter than the code
>> it replaces; therefore it is no longer necessary to reserve more
>> than necessary just to be sure that one can add an EBML Void element
>> (whose minimum size is two) lateron. This is the reason for the change
>> to the aac-autobsf-adtstoasc test.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>> ---
>>  libavformat/matroskaenc.c            | 120 ++++++++++++++++++---------
>>  tests/ref/fate/aac-autobsf-adtstoasc |   4 +-
>>  2 files changed, 84 insertions(+), 40 deletions(-)
> 
> triggers assert in fate
> 
> ffmpeg/ffmpeg -nostdin         -i fate-suite//audiomatch/tones_afconvert_16000_mono_aac_lc.m4a         -f segment -segment_time 1 -map 0 -flags +bitexact -codec copy -segment_format_options live=1         -segment_list ffmpeg/tests/data/adts-to-mkv.m3u8 -y ffmpeg/tests/data/adts-to-mkv-%03d.mkv
> 
>   Metadata:
>     major_brand     : M4A 
>     minor_version   : 0
>     compatible_brands: M4A mp42isom
>     creation_time   : 2024-12-23T09:22:20.000000Z
>     iTunSMPB        :  00000000 00000840 000002C0 0000000000007D00 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>   Duration: 00:00:02.18, start: 0.132000, bitrate: 35 kb/s
>   Stream #0:0[0x1](eng): Audio: aac (LC) (mp4a / 0x6134706D), 16000 Hz, mono, fltp, 20 kb/s (default)
>     Metadata:
>       creation_time   : 2024-12-23T09:22:20.000000Z
>       vendor_id       : [0][0][0][0]
> [segment @ 0x562c56aa3f40] Opening 'ffmpeg/tests/data/adts-to-mkv-000.mkv' for writing
> Assertion codecpriv_size < max_payload_size failed at libavformat/matroskaenc.c:1239
> Aborted (core dumped)
> 

Thanks for this. An earlier version stored the complete amount of bytes
reserved in the header including the bytes reserved for header and
length fields, whereas this version only stores the bytes necessary for
the payload and leaves the details of how much bytes to actually write
to mkv_put_codecprivate(). When I changed this, I forgot to correctly
update the assert (it is now of course entirely normal for
codecpriv_size and max_payload_size to coincide, so it should be a <=
instead of <) and it seems I also only ran the final version with an
assert-level 0 build only; earlier versions were checked with
--assert-level=2. How embarrassing.
Anyway, thanks for testing.

- Andreas