[FFmpeg-devel] [PATCH] AAC Decoder round 3

[Michael Niedermayer]

On Mon, Jul 07, 2008 at 02:50:36AM +0100, Robert Swain wrote:
> 2008/7/6 Michael Niedermayer <michaelni at gmx.at>:
> > On Tue, Jul 01, 2008 at 12:37:11PM +0100, Robert Swain wrote:
> > [...]
> >> +/**
> >> + * Free a channel element.
> >> + */
> >> +static void che_freep(ChannelElement **s) {
> >> +    if(!*s)
> >> +        return;
> >> +    av_freep(s);
> >> +}
> >
> > am i too tired or is the if-return useless and this a senseless wraper
> > function?
> 
> I'll remove it for the next submission but it's needed for SSR/LTP in
> SoC so I won't remove it there.

Its needed to check p == NULL before av_free(p) ?


[...]
> > [...]
> >> +static void spec_to_sample(AACContext * ac) {
> >
> > what is "spec to sample" ? the name does not say anythig to me
> 
> A function that converts spectral data to time domain sample data?
> I'll annotate it as I will do for other functions.

freq2time_domain()
spectral_to_sample()
spec is a abbreviation for specification as well ...


> 
> > [...]
> >
> >> +    i = 1024 * avccontext->channels * sizeof(uint16_t);
> >> +    if(*data_size < i)
> >> +        return -1;
> >
> > i wonder if this shouldnt be checked earlier (closer to where the number of
> > channels is set)
> 
> data_size is only known in aac_decode_frame() and avctx->channels is
> known at the end of output_configure() which is called at the end of
> program_config_element() and program_config_element_default().
> 
> The latter is called during aac_decode_init() and imposes the default
> channel mapping. A stream doesn't have to have a program config
> element to reconfigure the channel mappings from the default (which is
> the whole point of having default mappings I guess).
> 
> We have to be certain of the number of channels before we can do this check.
> 
> The spec says that a program config element must come before all other
> syntactic elements in the while() loop in aac_decode_frame(). So, I
> can add a flag var which can be toggled after the first syntactic
> element has been decoded and check data_size then or I can add the
> check just after the while() loop or maybe you have another
> suggestion?

leave the check where it is

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080708/7d772d29/attachment.pgp>