[FFmpeg-devel] [PATCH] avformat/matroskaenc: always reserve max aac private data

[Andreas Rheinhardt]

John Stebbins:
> When extra data is updated using AV_PKT_DATA_NEW_EXTRADATA, the data is
> written plus extra space is reserved for the max possible size extradata.
> But when extradata is written during mkv_write_header, no extra space is
> reserved. So the side data update overwrites whatever follows the
> extradata in the track header and results in an invalid file.
> ---
>  libavformat/matroskaenc.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 784973a951..e6917002c4 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -728,8 +728,6 @@ static int mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext *pb,
>      case AV_CODEC_ID_AAC:
>          if (par->extradata_size)
>              avio_write(dyn_cp, par->extradata, par->extradata_size);
> -        else
> -            put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4);
>          break;
>      default:
>          if (par->codec_id == AV_CODEC_ID_PRORES &&
> @@ -749,6 +747,7 @@ static int mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
>      AVIOContext *dyn_cp;
>      uint8_t *codecpriv;
>      int ret, codecpriv_size;
> +    int64_t offset;
>  
>      ret = avio_open_dyn_buf(&dyn_cp);
>      if (ret < 0)
> @@ -802,9 +801,15 @@ static int mkv_write_codecprivate(AVFormatContext *s, AVIOContext *pb,
>      }
>  
>      codecpriv_size = avio_get_dyn_buf(dyn_cp, &codecpriv);
> +    offset = avio_tell(pb);
>      if (codecpriv_size)
>          put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, codecpriv,
>                          codecpriv_size);
> +    if (par->codec_id == AV_CODEC_ID_AAC) {
> +        int filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(pb) - offset);
> +        if (filler)
> +            put_ebml_void(pb, filler);
> +    }
>      ffio_free_dyn_buf(&dyn_cp);
>      return ret;
>  }
> @@ -2182,7 +2187,7 @@ static int mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
>      switch (par->codec_id) {
>      case AV_CODEC_ID_AAC:
>          if (side_data_size && (s->pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
> -            int filler, output_sample_rate = 0;
> +            int output_sample_rate = 0;
>              ret = get_aac_sample_rates(s, side_data, side_data_size, &track->sample_rate,
>                                         &output_sample_rate);
>              if (ret < 0)
> @@ -2195,9 +2200,6 @@ static int mkv_check_new_extra_data(AVFormatContext *s, const AVPacket *pkt)
>              memcpy(par->extradata, side_data, side_data_size);
>              avio_seek(mkv->tracks_bc, track->codecpriv_offset, SEEK_SET);
>              mkv_write_codecprivate(s, mkv->tracks_bc, par, 1, 0);
> -            filler = MAX_PCE_SIZE + 2 + 4 - (avio_tell(mkv->tracks_bc) - track->codecpriv_offset);
> -            if (filler)
> -                put_ebml_void(mkv->tracks_bc, filler);
>              avio_seek(mkv->tracks_bc, track->sample_rate_offset, SEEK_SET);
>              put_ebml_float(mkv->tracks_bc, MATROSKA_ID_AUDIOSAMPLINGFREQ, track->sample_rate);
>              put_ebml_float(mkv->tracks_bc, MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, output_sample_rate);
> 
If we already had extradata when writing the header, then what
guarantees us that the new extradata is better and should be preferred
to the old extradata? (I don't know the details of AAC extradata, but is
it possible that the extradata is simply incompatible to the old one
(e.g. when a user splices together actually incompatible streams)?)

- Andreas