Hi, On Wed, Apr 13, 2011 at 2:49 AM, Martin Storsjö <[email protected]> wrote: > On Tue, 12 Apr 2011, Ronald S. Bultje wrote: >> On Tue, Apr 12, 2011 at 3:44 AM, Martin Storsjö <[email protected]> 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. Ronald _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
