[FFmpeg-devel] [PATCH] AAC: reduce memory footprint

[Robert Swain]

On 17/2/09 23:33, Alex Converse wrote:
> For post release consideration, this patch reduces the memory footprint of
> the AAC decoder by only allocating certain large structures if they are
> necessary for the given configuration.

Well, no release yet, but as we're branched, I'll review this now. :)

> ------------------------------------------------------------------------
>
> diff --git a/libavcodec/aac.c b/libavcodec/aac.c
> index 311dd69..57c4919 100644
> --- a/libavcodec/aac.c
> +++ b/libavcodec/aac.c
> @@ -96,6 +96,17 @@
>   static VLC vlc_scalefactors;
>   static VLC vlc_spectral[11];
>
> +static void channel_element_freep(ChannelElement ** che) {
> +    if (*che) {
> +        if ((*che)->ch) {
> +            av_freep(&(*che)->ch->predictor_state);
> +            av_freep(&(*che)->ch);
> +        }
> +        av_freep(&(*che)->coup);
> +        av_freep(che);
> +    }
> +    *che = NULL;
> +}

If I recall correctly, av_freep() is preferred over av_free() precisely 
because it assigns NULL to the pointer after it has been freed, so the 
last line is redundant.

>   /**
>    * Configure output channel order based on the current program configuration element.
> @@ -109,6 +120,7 @@ static int output_configure(AACContext *ac, enum ChannelPosition che_pos[4][MAX_
>           enum ChannelPosition new_che_pos[4][MAX_ELEM_ID]) {
>       AVCodecContext *avctx = ac->avccontext;
>       int i, type, channels = 0;
> +    ChannelElement * ce;

Why not che for consistency?

>       if(!memcmp(che_pos, new_che_pos, 4 * MAX_ELEM_ID * sizeof(new_che_pos[0][0])))
>           return 0; /* no change */
> @@ -129,6 +141,18 @@ static int output_configure(AACContext *ac, enum ChannelPosition che_pos[4][MAX_
>               if(che_pos[type][i]) {
>                   if(!ac->che[type][i]&&  !(ac->che[type][i] = av_mallocz(sizeof(ChannelElement))))
>                       return AVERROR(ENOMEM);
> +                ce = ac->che[type][i];
> +                if(!ce->ch&&  !(ce->ch = av_mallocz(((type==TYPE_CPE)+1)*sizeof(SingleChannelElement))))

Please use white space around operators. There are more instances below. 
I do see that there are some pre-existing but the code isn't 
cosmetically perfect anyway so I'll have to go over it sometime and 
clean it up.

[...]

It needs updating a little because of some of the coup.gain changes you 
made but other than that, patch OK assuming it's been regtested etc. In 
your commit message it might be nice to note how much memory is saved, 
though I realise it will vary depending on the channel configuration and 
bitstream features used.

Regards,
Rob