[FFmpeg-devel] [PATCH] Allow autodetecting PS and SBR for AAC without extradata.

[Reimar Döffinger]

On Tue, Aug 16, 2011 at 10:47:37AM -0700, Aℓex Converse wrote:
> On Mon, Aug 15, 2011 at 4:59 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Aug 10, 2011 at 09:02:08PM +0200, Reimar Döffinger wrote:
> >> This was possible before and was broken when this code was added.
> >>
> >> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> >> ---
> >>  libavcodec/aacdec.c |    6 +++++-
> >>  1 files changed, 5 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/libavcodec/aacdec.c b/libavcodec/aacdec.c
> >> index 944be9f..b8d5865 100644
> >> --- a/libavcodec/aacdec.c
> >> +++ b/libavcodec/aacdec.c
> >> @@ -578,6 +578,10 @@ static av_cold int aac_decode_init(AVCodecContext *avctx)
> >>          int sr, i;
> >>          enum ChannelPosition new_che_pos[4][MAX_ELEM_ID];
> >>
> >> +        // Allow these to be detected later
> >> +        ac->m4ac.sbr = -1;
> >> +        ac->m4ac.ps  = -1;
> >> +
> >>          sr = sample_rate_idx(avctx->sample_rate);
> >>          ac->m4ac.sampling_index = sr;
> >>          ac->m4ac.channels = avctx->channels;
> >> @@ -593,7 +597,7 @@ static av_cold int aac_decode_init(AVCodecContext *avctx)
> >>          if (ac->m4ac.chan_config) {
> >>              int ret = set_default_channel_config(avctx, new_che_pos, ac->m4ac.chan_config);
> >>              if (!ret)
> >> -                output_configure(ac, ac->che_pos, new_che_pos, ac->m4ac.chan_config, OC_GLOBAL_HDR);
> >> +                output_configure(ac, ac->che_pos, new_che_pos, ac->m4ac.chan_config, OC_NONE);
> >>              else if (avctx->error_recognition >= FF_ER_EXPLODE)
> >>                  return AVERROR_INVALIDDATA;
> >>          }
> >
> > iam CCing that to alex, as i think he wrote this and might have some
> > comments
> >
> 
> The real problem with this file is that there is half a packet of
> garbage that looks like an ADTS header followed by the AAC END syntax
> element. I think my patch for libav handles this better.

I disagree.
Your patch probably is a good idea, however when a global header is available
there should really be no further headers, and ignoring anything that
looks like one should be a good strategy if in doubt.
In the other case, things are set up with essentially purely made up
data.
Setting output_configured to the same value for those IMO very different
cases seems rather unreasonable to me.

> (and all this output configuration code should be adjusted to try a
> new configuration and if it fails fall back to the old one).

Probably, but even the it might be useful to have some information
on how accurate the current configuration is.
And one "made up" from nothing still shouldn't get the same marks as one
based on extradata.