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

[Aℓex Converse]

On Tue, Aug 16, 2011 at 12:52 PM, Reimar Döffinger
<Reimar.Doeffinger at gmx.de> wrote:
>
> 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.

i'm not sure, I see lots of feature requests to be able to reconfigure on a PCE.

OTOH I think a lot of those people really want ADTS reconfiguration

> Setting output_configured to the same value for those IMO very different
> cases seems rather unreasonable to me.
>

agreed

> > (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.

agreed

However, in your fix a file without a header will never lock and a
tiny bit of corruption will permanently desynchronize the decode.

This defeats the purpose of having the output configuration locking at all.