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

[Michael Niedermayer]

On Tue, Aug 16, 2011 at 01:30:07PM -0700, Aℓex Converse wrote:
> 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.

I did not know that, could you explain why it can lead to permanent
desync. Most other decoders (like mp3) should recover very quickly
on corrupted data. I would naively have assumed that this is also
how aac behaves, which is also why the whole locking thing didnt
make much sense to me from the first time i saw it.

[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110818/82ba7069/attachment.asc>