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

[Diego Biurrun]

On Mon, Jul 07, 2008 at 05:57:54PM +0100, Robert Swain wrote:
> [...] 

Here is the review you asked for..

> --- aac.c	(revision 2716)
> +++ aac.c	(working copy)
> @@ -731,6 +731,10 @@
>  
> @@ -921,6 +925,8 @@
>  
>  /**
>   * Decode Individual Channel Stream info; reference: table 4.6.
> + *
> + * @param   common_window   Channels have independent [0], or share [1], Individual Channel Stream information.

shared

Why do you capitalize (Individual Channel Stream information)?

> @@ -1042,6 +1058,13 @@
>  
>  /**
>   * Decode scale_factor_data; reference: table 4.47.
> + *
> + * @param   mix_gain    Channel gain (not used by AAC bitstream).

lowercase

> + * @param   cb_run_end  array of the last scalefactor band of a codebook run for a window group's scalefactor band

Wow, that's complicated..  No idea how to simplify it, though..

> @@ -1110,6 +1136,9 @@
>  
> +/**
> + * Decode Temporal Noise Shaping data; reference: table 4.48.

Why do you capitalize?

> @@ -1165,6 +1194,9 @@
>  
> +/**
> + * Decode Mid/Side data; reference: table 4.54.

Why do you capitalize?

> @@ -1276,6 +1326,10 @@
>  
>  /**
>   * Decode an individual_channel_stream payload; reference: table 4.44.
> + *
> + * @param   common_window   Channels have independent [0], or share [1], Individual Channel Stream information.

see above

> @@ -1353,7 +1410,9 @@
>  
> +/**
> + * intensity stereo decoding; reference: 4.6.8.2.3.

Skip the period :)

> @@ -1382,6 +1441,9 @@
>  
>  /**
>   * Decode a channel_pair_element; reference: table 4.4.
> + *
> + * @param   id  identifies the instance of a syntax element

Capitalize.

> @@ -1415,6 +1477,12 @@
>  
> +/**
> + * Decode coupling_channel_element; reference: table 4.8.
> + *
> + * @param   id  identifies the instance of a syntax element

ditto

> @@ -1479,14 +1547,25 @@
>  
> +/**
> + * Decode whether channels are to be excluded from Dynamic Range Compression; reference: table 4.53.

Decode?  Not "find out" or similar?

> @@ -1577,6 +1662,12 @@
>  
> + * @param   decode  1 if tool is used normally, 0 if tool is used in LTP

.

> @@ -1992,6 +2104,9 @@
>  
> +/**
> + * Single Channel Element transformation interface

capitalization once again

> @@ -2005,6 +2120,9 @@
>  
> +/**
> + * Convert spectral data to float samples, applying all supported tools as appropriate.

long line, same in many other places

> @@ -2026,7 +2144,13 @@
>  
> +/**
> + * Conduct matrix mix-down and float to int16 conversion

.

Diego