diff mbox series

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

Message ID 20200501170948.153825-1-jstebbins@jetheaddev.com
State New
Headers show
Series [FFmpeg-devel] avformat/matroskaenc: always reserve max aac private data
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

John Stebbins May 1, 2020, 5:09 p.m. UTC
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(-)

Comments

Andreas Rheinhardt May 1, 2020, 5:22 p.m. UTC | #1
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
James Almer May 1, 2020, 5:27 p.m. UTC | #2
On 5/1/2020 2:09 PM, John Stebbins wrote:
> 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.

In what scenario there's extradata during init() and then new extradata
propagated in a packet side data for AAC? And should the side data one
take priority over the original one?

With FLAC we know it must have priority because the encoder sends it at
the end of the encoding process with information that was not available
during init(), but afaik that's not the case with AAC.

> ---
>  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);
> +    }

Why are you adding this here instead of adapting the code in
mkv_write_native_codecprivate()?

Can't you just do something like

case AV_CODEC_ID_AAC:
    if (par->extradata_size)
        avio_write(dyn_cp, par->extradata, par->extradata_size);
    put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4 - avio_tell(dyn_cp));
    break;

>      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);
>
Andreas Rheinhardt May 1, 2020, 5:35 p.m. UTC | #3
James Almer:
> On 5/1/2020 2:09 PM, John Stebbins wrote:
>> 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.
> 
> In what scenario there's extradata during init() and then new extradata
> propagated in a packet side data for AAC? And should the side data one
> take priority over the original one?
> 
> With FLAC we know it must have priority because the encoder sends it at
> the end of the encoding process with information that was not available
> during init(), but afaik that's not the case with AAC.
> 
>> ---
>>  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);
>> +    }
> 
> Why are you adding this here instead of adapting the code in
> mkv_write_native_codecprivate()?
> 
> Can't you just do something like
> 
> case AV_CODEC_ID_AAC:
>     if (par->extradata_size)
>         avio_write(dyn_cp, par->extradata, par->extradata_size);
>     put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4 - avio_tell(dyn_cp));
>     break;
> 
Then mkv_write_codecprivate() would overwrite the beginning of the void
element when it writes the CodecPrivate, creating an invalid file.
Instead one could do something like

if (par->extradata_size)
    put_ebml_binary(pb, MATROSKA_ID_CODECPRIVATE, par->extradata,
par->extradata_size);
put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4 - what was just written (not only
par->extradata);

- Andreas
John Stebbins May 1, 2020, 5:51 p.m. UTC | #4
On Fri, 2020-05-01 at 19:22 +0200, Andreas Rheinhardt wrote:
> 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)?)
> 

The test case is a TS file with aac audio.  It TS files, it is
certainly possible for stream parameters to change on the fly. But, if
the extradata changes, there's really no way to handle it in mkv since
this data is global in mkv.  So perhaps a better solution is to ignore
side data extradata if it's already been written once?
Nicolas George May 1, 2020, 5:53 p.m. UTC | #5
John Stebbins (12020-05-01):
> The test case is a TS file with aac audio.  It TS files, it is
> certainly possible for stream parameters to change on the fly. But, if
> the extradata changes, there's really no way to handle it in mkv since
> this data is global in mkv.  So perhaps a better solution is to ignore
> side data extradata if it's already been written once?

Would it not lead to a corrupted file, possibly unplayable?

Regards,
Andreas Rheinhardt May 1, 2020, 5:55 p.m. UTC | #6
John Stebbins:
> On Fri, 2020-05-01 at 19:22 +0200, Andreas Rheinhardt wrote:
>> 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)?)
>>
> 
> The test case is a TS file with aac audio.  It TS files, it is
> certainly possible for stream parameters to change on the fly. But, if
> the extradata changes, there's really no way to handle it in mkv since
> this data is global in mkv.  So perhaps a better solution is to ignore
> side data extradata if it's already been written once?
> 
In this scenario there is no way to know whether the new extradata is
better than the old one. If we ignore the extradata in such scenarios,
then is such a file even decodable?

- Andreas
John Stebbins May 1, 2020, 5:58 p.m. UTC | #7
On Fri, 2020-05-01 at 14:27 -0300, James Almer wrote:
> On 5/1/2020 2:09 PM, John Stebbins wrote:
> > 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.
> 
> In what scenario there's extradata during init() and then new
> extradata
> propagated in a packet side data for AAC? And should the side data
> one
> take priority over the original one?
> 

This is partially a HandBrake issue.  Before ffmpeg ever had side data,
HandBrake parsed the extradata out of the aac stream in TS files
ourselves before initializing the muxer.  We still do this, so
extradata is supplied by us when the muxer it initialized and again by
side data generated in aac_adtstoasc bsf.

So remuxing ts to mkv with ffmpeg exe does not have this problem.  But
it is a problem when using the API in valid ways.

I don't think it matters which takes priority.  If stream parameters
change, there really is no way to handle in mkv since this is global
data in mkv.  So perhaps just ignore side data when extradata has
already been set once?

> With FLAC we know it must have priority because the encoder sends it
> at
> the end of the encoding process with information that was not
> available
> during init(), but afaik that's not the case with AAC.
> 
> > ---
> >  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);
> > +    }
> 
> Why are you adding this here instead of adapting the code in
> mkv_write_native_codecprivate()?
> 
> Can't you just do something like
> 
> case AV_CODEC_ID_AAC:
>     if (par->extradata_size)
>         avio_write(dyn_cp, par->extradata, par->extradata_size);
>     put_ebml_void(pb, MAX_PCE_SIZE + 2 + 4 - avio_tell(dyn_cp));
>     break;
> 
> >      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);
> > 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Andreas Rheinhardt May 1, 2020, 6:03 p.m. UTC | #8
John Stebbins:
> On Fri, 2020-05-01 at 14:27 -0300, James Almer wrote:
>> On 5/1/2020 2:09 PM, John Stebbins wrote:
>>> 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.
>>
>> In what scenario there's extradata during init() and then new
>> extradata
>> propagated in a packet side data for AAC? And should the side data
>> one
>> take priority over the original one?
>>
> 
> This is partially a HandBrake issue.  Before ffmpeg ever had side data,
> HandBrake parsed the extradata out of the aac stream in TS files
> ourselves before initializing the muxer.  We still do this, so
> extradata is supplied by us when the muxer it initialized and again by
> side data generated in aac_adtstoasc bsf.
> 
Does this mean that HandBrake's extradata and the aac_adtstoasc bsf
side-data extradata are actually one and the same?

- Andreas

> So remuxing ts to mkv with ffmpeg exe does not have this problem.  But
> it is a problem when using the API in valid ways.
> 
> I don't think it matters which takes priority.  If stream parameters
> change, there really is no way to handle in mkv since this is global
> data in mkv.  So perhaps just ignore side data when extradata has
> already been set once?
>
John Stebbins May 1, 2020, 6:04 p.m. UTC | #9
On Fri, 2020-05-01 at 19:53 +0200, Nicolas George wrote:
> John Stebbins (12020-05-01):
> > The test case is a TS file with aac audio.  It TS files, it is
> > certainly possible for stream parameters to change on the fly. But,
> > if
> > the extradata changes, there's really no way to handle it in mkv
> > since
> > this data is global in mkv.  So perhaps a better solution is to
> > ignore
> > side data extradata if it's already been written once?
> 
> Would it not lead to a corrupted file, possibly unplayable?
> 
> 

If the parameters change on the fly, some part of the audio stream is
going to be unplayable.  Either the part after the change will be
unplayable if you ignore extradata changes, or everything preceeding 
the last extradata change will be unplayable if you rewrite codec
private on every change.
Nicolas George May 1, 2020, 6:10 p.m. UTC | #10
John Stebbins (12020-05-01):
> If the parameters change on the fly, some part of the audio stream is
> going to be unplayable.  Either the part after the change will be
> unplayable if you ignore extradata changes, or everything preceeding 
> the last extradata change will be unplayable if you rewrite codec
> private on every change.

Then the only acceptable solution is to report an error.

If we can detect that the new extradata is compatible and will not cause
the file to be unplayable, we can let it pass, but we should try to
NEVER allow to lose data without notifying users.

Regards,
John Stebbins May 1, 2020, 6:12 p.m. UTC | #11
On Fri, 2020-05-01 at 19:55 +0200, Andreas Rheinhardt wrote:
> John Stebbins:
> > On Fri, 2020-05-01 at 19:22 +0200, Andreas Rheinhardt wrote:
> > > 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)?)
> > > 
> > 
> > The test case is a TS file with aac audio.  It TS files, it is
> > certainly possible for stream parameters to change on the fly. But,
> > if
> > the extradata changes, there's really no way to handle it in mkv
> > since
> > this data is global in mkv.  So perhaps a better solution is to
> > ignore
> > side data extradata if it's already been written once?
> > 
> In this scenario there is no way to know whether the new extradata is
> better than the old one. If we ignore the extradata in such
> scenarios,
> then is such a file even decodable?
> 

It depends on if the extradata actually changed or not.  In my actual
test case, there are not stream parameter changes.  The same data is
being delivered when initializing the muxer and by side data.

If stream parameters actually change on the fly in a TS file, ffmpeg
currently generates a broken stream. aac_adtstoasc bsf only generates
extradata side data once.  It currently ignores any other changes.  So
if incompatible aac parameters exist in the stream after the first
packet, the output stream will be broken.
John Stebbins May 1, 2020, 6:14 p.m. UTC | #12
On Fri, 2020-05-01 at 20:03 +0200, Andreas Rheinhardt wrote:
> John Stebbins:
> > On Fri, 2020-05-01 at 14:27 -0300, James Almer wrote:
> > > On 5/1/2020 2:09 PM, John Stebbins wrote:
> > > > 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.
> > > 
> > > In what scenario there's extradata during init() and then new
> > > extradata
> > > propagated in a packet side data for AAC? And should the side
> > > data
> > > one
> > > take priority over the original one?
> > > 
> > 
> > This is partially a HandBrake issue.  Before ffmpeg ever had side
> > data,
> > HandBrake parsed the extradata out of the aac stream in TS files
> > ourselves before initializing the muxer.  We still do this, so
> > extradata is supplied by us when the muxer it initialized and again
> > by
> > side data generated in aac_adtstoasc bsf.
> > 
> Does this mean that HandBrake's extradata and the aac_adtstoasc bsf
> side-data extradata are actually one and the same?
> 

Yes
John Stebbins May 1, 2020, 6:20 p.m. UTC | #13
On Fri, 2020-05-01 at 20:10 +0200, Nicolas George wrote:
> John Stebbins (12020-05-01):
> > If the parameters change on the fly, some part of the audio stream
> > is
> > going to be unplayable.  Either the part after the change will be
> > unplayable if you ignore extradata changes, or everything
> > preceeding 
> > the last extradata change will be unplayable if you rewrite codec
> > private on every change.
> 
> Then the only acceptable solution is to report an error.
> 
> If we can detect that the new extradata is compatible and will not
> cause
> the file to be unplayable, we can let it pass, but we should try to
> NEVER allow to lose data without notifying users.
> 

Well, current code in aac_adtstoasc silently ignores any changes.  It
only generates extradata from the initial packet.  It's not checked in
subsequent packets.

So this would have to be "fixed" in aac_adtstoasc as well if you want
to add error logging.

This is also a bit beyond my expertise.  I don't know what would
constitute incompitible parameters beyond a few obvious things.  I'm
not well versed in aac details.
Nicolas George May 1, 2020, 6:28 p.m. UTC | #14
John Stebbins (12020-05-01):
> Well, current code in aac_adtstoasc silently ignores any changes.  It
> only generates extradata from the initial packet.  It's not checked in
> subsequent packets.
> 
> So this would have to be "fixed" in aac_adtstoasc as well if you want
> to add error logging.

Probably. I want to emphasize that bugs in our current code cannot be
considered precedent to accept similar bugs in new code.

> This is also a bit beyond my expertise.  I don't know what would
> constitute incompitible parameters beyond a few obvious things.  I'm
> not well versed in aac details.

Then for now, I would say that we can only accept when it is
bit-identical with the extradata already there. If we cannot test for
compatibility more finely, then we have to assume incompatibility and
reject every AV_PKT_DATA_NEW_EXTRADATA with an error.

Regards,
John Stebbins May 1, 2020, 7:13 p.m. UTC | #15
On Fri, 2020-05-01 at 20:28 +0200, Nicolas George wrote:
> John Stebbins (12020-05-01):
> > Well, current code in aac_adtstoasc silently ignores any
> > changes.  It
> > only generates extradata from the initial packet.  It's not checked
> > in
> > subsequent packets.
> > 
> > So this would have to be "fixed" in aac_adtstoasc as well if you
> > want
> > to add error logging.
> 
> Probably. I want to emphasize that bugs in our current code cannot be
> considered precedent to accept similar bugs in new code.
> 
> > This is also a bit beyond my expertise.  I don't know what would
> > constitute incompitible parameters beyond a few obvious
> > things.  I'm
> > not well versed in aac details.
> 
> Then for now, I would say that we can only accept when it is
> bit-identical with the extradata already there. If we cannot test for
> compatibility more finely, then we have to assume incompatibility and
> reject every AV_PKT_DATA_NEW_EXTRADATA with an error.
> 
> 

Seems reasonable.  To be clear, this should result in an error returned
up the call stack (i.e. av_interleaved_write_frame would ultimately
return an error), and an error log?
Carl Eugen Hoyos May 1, 2020, 7:39 p.m. UTC | #16
Am Fr., 1. Mai 2020 um 21:13 Uhr schrieb John Stebbins
<jstebbins@jetheaddev.com>:
>
> On Fri, 2020-05-01 at 20:28 +0200, Nicolas George wrote:
> > John Stebbins (12020-05-01):
> > > Well, current code in aac_adtstoasc silently ignores any
> > > changes.  It
> > > only generates extradata from the initial packet.  It's not checked
> > > in
> > > subsequent packets.
> > >
> > > So this would have to be "fixed" in aac_adtstoasc as well if you
> > > want
> > > to add error logging.
> >
> > Probably. I want to emphasize that bugs in our current code cannot be
> > considered precedent to accept similar bugs in new code.
> >
> > > This is also a bit beyond my expertise.  I don't know what would
> > > constitute incompitible parameters beyond a few obvious
> > > things.  I'm
> > > not well versed in aac details.
> >
> > Then for now, I would say that we can only accept when it is
> > bit-identical with the extradata already there. If we cannot test for
> > compatibility more finely, then we have to assume incompatibility and
> > reject every AV_PKT_DATA_NEW_EXTRADATA with an error.
> >
> >
>
> Seems reasonable.  To be clear, this should result in an error returned
> up the call stack (i.e. av_interleaved_write_frame would ultimately
> return an error), and an error log?

Only in an error log.

Carl Eugen
Andreas Rheinhardt May 1, 2020, 7:42 p.m. UTC | #17
John Stebbins:
> On Fri, 2020-05-01 at 20:28 +0200, Nicolas George wrote:
>> John Stebbins (12020-05-01):
>>> Well, current code in aac_adtstoasc silently ignores any
>>> changes.  It
>>> only generates extradata from the initial packet.  It's not checked
>>> in
>>> subsequent packets.
>>>
>>> So this would have to be "fixed" in aac_adtstoasc as well if you
>>> want
>>> to add error logging.
>>
>> Probably. I want to emphasize that bugs in our current code cannot be
>> considered precedent to accept similar bugs in new code.
>>
>>> This is also a bit beyond my expertise.  I don't know what would
>>> constitute incompitible parameters beyond a few obvious
>>> things.  I'm
>>> not well versed in aac details.
>>
>> Then for now, I would say that we can only accept when it is
>> bit-identical with the extradata already there. If we cannot test for
>> compatibility more finely, then we have to assume incompatibility and
>> reject every AV_PKT_DATA_NEW_EXTRADATA with an error.
>>
>>
> 
> Seems reasonable.  To be clear, this should result in an error returned
> up the call stack (i.e. av_interleaved_write_frame would ultimately
> return an error), and an error log?
> 
I'd rather opt to only warn in such a case unless strict_std_compliance
is >= FF_COMPLIANCE_STRICT. And maybe we should also discard all future
packets from this stream until we get one with side data matching the
CodecPrivate one?

- Andreas
Nicolas George May 1, 2020, 7:55 p.m. UTC | #18
Andreas Rheinhardt (12020-05-01):
> I'd rather opt to only warn in such a case unless strict_std_compliance
> is >= FF_COMPLIANCE_STRICT. And maybe we should also discard all future
> packets from this stream until we get one with side data matching the
> CodecPrivate one?

It is not a matter of standard compliance, it is a matter of lost data.
If only one extradata is written, then the other is discarded, and
possibly all subsequent packets are un-decodable.

Without an AVERROR code up the stack, a GUI application (most of them
just let av_log go to stderr) could open a dialog "Data successfully
saved. Delete original? [Yes] No" and let actual data be lost.

We can have an option to ignore the error, but as it is, it really must
be an error condition by default. Exactly the same as a write error on
the file.

Regards,
diff mbox series

Patch

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);