[FFmpeg-devel] [PATCH 5/7] avformat/matroskaenc: write a MaxBlockAdditionID element

[Anton Khirnov]

Quoting James Almer (2023-03-24 12:41:27)
> On 3/24/2023 8:37 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-03-21 18:06:35)
> >> A non zero value is mandatory for Matroska if the track has blocks with BlockAdditions.
> >>
> >> Signed-off-by: James Almer <jamrial at gmail.com>
> >> ---
> >>   libavformat/matroskaenc.c | 36 ++++++++++++++++++++++++++++++++----
> >>   1 file changed, 32 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> >> index 46f4331a18..0687d9c32e 100644
> >> --- a/libavformat/matroskaenc.c
> >> +++ b/libavformat/matroskaenc.c
> >> @@ -188,6 +188,8 @@ typedef struct mkv_track {
> >>       int64_t         last_timestamp;
> >>       int64_t         duration;
> >>       int64_t         duration_offset;
> >> +    uint64_t        max_blockaddid;
> >> +    int64_t         blockadditionmapping_offset;
> >>       int             codecpriv_offset;
> >>       unsigned        codecpriv_size;     ///< size reserved for CodecPrivate excluding header+length field
> >>       int64_t         ts_offset;
> >> @@ -1597,12 +1599,21 @@ static int mkv_write_stereo_mode(AVFormatContext *s, EbmlWriter *writer,
> >>       return 0;
> >>   }
> >>   
> >> -static void mkv_write_dovi(AVFormatContext *s, AVIOContext *pb, AVStream *st)
> >> +static void mkv_write_blockadditionmapping(AVFormatContext *s, MatroskaMuxContext *mkv,
> >> +                                           AVIOContext *pb, mkv_track *track, AVStream *st)
> >>   {
> >>   #if CONFIG_MATROSKA_MUXER
> >>       AVDOVIDecoderConfigurationRecord *dovi = (AVDOVIDecoderConfigurationRecord *)
> >>                                                av_stream_get_side_data(st, AV_PKT_DATA_DOVI_CONF, NULL);
> >>   
> >> +    if (IS_SEEKABLE(s->pb, mkv)) {
> >> +        track->blockadditionmapping_offset = avio_tell(pb);
> >> +        // We can't know at this point if there will be a block with BlockAdditions, so
> >> +        // we either write the default value here, or a void element. Either of them will
> >> +        // be overwritten when finishing the track.
> > 
> > IIRC matroska uints are variable-size, so the value you write later may
> > have a longer representation than the dummy placeholder you're writing
> > here.
> > Or am I wrong?
> 
> It might if we were to write any arbitrary value passed to the muxer in 
> AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL side data, but we will not. We're 
> only going to write 1 (opaque, codec specific, currently used for alpha 
> channel) at this point in the patchset, or 4 (itu-t t35) after patch 
> 6/7, for now.

Ok, might still be safer against future changes to check that the
updated value is not too large.

-- 
Anton Khirnov