On Wed, 13 Apr 2011, Ronald S. Bultje wrote:
> On Wed, Apr 13, 2011 at 2:49 AM, Martin Storsjö <mar...@martin.st> wrote:
> > On Tue, 12 Apr 2011, Ronald S. Bultje wrote:
> >> On Tue, Apr 12, 2011 at 3:44 AM, Martin Storsjö <mar...@martin.st> wrote:
> >> > ---
> >> > libavcodec/libvo-aacenc.c | 18 ++++++++++--------
> >> > 1 files changed, 10 insertions(+), 8 deletions(-)
> >> >
> >> > diff --git a/libavcodec/libvo-aacenc.c b/libavcodec/libvo-aacenc.c
> >> > index 3c7dde7..7c1738d 100644
> >> > --- a/libavcodec/libvo-aacenc.c
> >> > +++ b/libavcodec/libvo-aacenc.c
> >> > @@ -62,12 +62,6 @@ static av_cold int aac_encode_init(AVCodecContext
> >> > *avctx)
> >> > return AVERROR_UNKNOWN;
> >> > }
> >> >
> >> > - avctx->extradata_size = 2;
> >> > - avctx->extradata = av_mallocz(avctx->extradata_size +
> >> > - FF_INPUT_BUFFER_PADDING_SIZE);
> >> > - if (!avctx->extradata)
> >> > - return AVERROR(ENOMEM);
> >> > -
> >> > for (index = 0; index < 16; index++)
> >> > if (avctx->sample_rate == ff_mpeg4audio_sample_rates[index])
> >> > break;
> >> > @@ -76,8 +70,16 @@ static av_cold int aac_encode_init(AVCodecContext
> >> > *avctx)
> >> > avctx->sample_rate);
> >> > return AVERROR_NOTSUPP;
> >> > }
> >> > - avctx->extradata[0] = 0x02 << 3 | index >> 1;
> >> > - avctx->extradata[1] = (index & 0x01) << 7 | avctx->channels << 3;
> >> > + if (avctx->flags & CODEC_FLAG_GLOBAL_HEADER) {
> >> > + avctx->extradata_size = 2;
> >> > + avctx->extradata = av_mallocz(avctx->extradata_size +
> >> > +
> >> > FF_INPUT_BUFFER_PADDING_SIZE);
> >> > + if (!avctx->extradata)
> >> > + return AVERROR(ENOMEM);
> >> > +
> >> > + avctx->extradata[0] = 0x02 << 3 | index >> 1;
> >> > + avctx->extradata[1] = (index & 0x01) << 7 | avctx->channels <<
> >> > 3;
> >> > + }
> >> > return 0;
> >> > }
> >>
> >> This isn't really what the flag means. If it's data that can be
> >> discarded at choice, then the flag can be ignored. Formats that don't
> >> like it, simply don't set extradata.
> >>
> >> The idea of the flag is that it creates extradata if set, and then not
> >> interleave that same data in the bitstream, and if the flag is not
> >> set, it interleaves that extradata in the bitstream instead. This
> >> patch doesn't appear to do that, so then it's not necessary...
> >
> > Yes, we do that already, by enabling ADTS if the global header flag isn't
> > set - the ADTS header contains the same data (but is produced by the
> > encoder library when the adtsUsed flag is set). But if the global header
> > flag isn't set, we perhaps shouldn't produce any extradata at all.
> >
> > Does anyone else have an opinion, or even better, insight into whether it
> > actually is ok to produce extradata even if the global header flag isn't
> > set? The data set in the extradata is MPEG4 Audio Specific Config - I
> > don't think it makes sense to output such extradata while outputting ADTS
> > data.
>
> I didn't see that you used the same flag elsewhere in this file
> already. Just checked, turns out it's true, so then patch is fine.
> Sorry I didn't see that before.
Thanks - pushed.
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
