[FFmpeg-devel] [PATCH] Some cleanups for libavcodec/alac.c

[Michael Niedermayer]

Hi

On Fri, Jul 06, 2007 at 03:18:45PM +0200, Vitor Sessak wrote:
> Hi,
> 
> These are my first patches to clean up the ALAC decoder. There are four 
> of them:
> 
> alac1.diff: Use proper bytestream functions

see below


> alac2.diff: Port some useful comments from newest version on David 
> Hammerton's page

ok


> alac3.diff: Remove some duplicated code

see below


> alac4.diff: Prepare to remove more duplicated code (first chunk of 
> Matthieu's patch)

ok (without the trailing whitespace)


[...]
> Index: libavcodec/alac.c
> ===================================================================
> --- libavcodec/alac.c	(revision 9487)
> +++ libavcodec/alac.c	(working copy)
> @@ -55,6 +55,7 @@
>  
>  #include "avcodec.h"
>  #include "bitstream.h"
> +#include "bytestream.h"
>  
>  #define ALAC_EXTRADATA_SIZE 36
>  
> @@ -110,26 +111,27 @@
>      ptr += 4; /* alac */
>      ptr += 4; /* 0 ? */
>  
> -    if(AV_RB32(ptr) >= UINT_MAX/4){
> +    /* buffer size / 2 ? */
> +    alac->setinfo_max_samples_per_frame = bytestream_get_be32(&ptr);
> +    alac->setinfo_7a                    = bytestream_get_byte(&ptr);
> +    alac->setinfo_sample_size           = bytestream_get_byte(&ptr);
> +    alac->setinfo_rice_historymult      = bytestream_get_byte(&ptr);
> +    alac->setinfo_rice_initialhistory   = bytestream_get_byte(&ptr);
> +    alac->setinfo_rice_kmodifier        = bytestream_get_byte(&ptr);
> +    /* channels? */
> +    alac->setinfo_7f                    = bytestream_get_byte(&ptr);
> +    alac->setinfo_80                    = bytestream_get_be16(&ptr);
> +    /* max coded frame size */
> +    alac->setinfo_82                    = bytestream_get_be32(&ptr);
> +    /* bitrate ? */
> +    alac->setinfo_86                    = bytestream_get_be32(&ptr);
> +    /* samplerate */
> +    alac->setinfo_8a_rate               = bytestream_get_be32(&ptr);
> +
> +    if(alac->setinfo_max_samples_per_frame >= UINT_MAX/4){
>          av_log(alac->avctx, AV_LOG_ERROR, "setinfo_max_samples_per_frame too large\n");
>          return -1;
>      }
> -    alac->setinfo_max_samples_per_frame = AV_RB32(ptr); /* buffer size / 2 ? */
> -    ptr += 4;
> -    alac->setinfo_7a = *ptr++;
> -    alac->setinfo_sample_size = *ptr++;
> -    alac->setinfo_rice_historymult = *ptr++;
> -    alac->setinfo_rice_initialhistory = *ptr++;
> -    alac->setinfo_rice_kmodifier = *ptr++;
> -    alac->setinfo_7f = *ptr++; // channels?
> -    alac->setinfo_80 = AV_RB16(ptr);
> -    ptr += 2;
> -    alac->setinfo_82 = AV_RB32(ptr); // max coded frame size
> -    ptr += 4;
> -    alac->setinfo_86 = AV_RB32(ptr); // bitrate ?
> -    ptr += 4;
> -    alac->setinfo_8a_rate = AV_RB32(ptr); // samplerate
> -    ptr += 4;

i think its safer to check the validity of setinfo_max_samples_per_frame
before storing it into the struct
also *ptr++ looks fine to me


[...]

> -        /* 2^result = something to do with output waiting.
> -         * perhaps matters if we read > 1 frame in a pass?
> -         */
> -        get_bits(&alac->gb, 4);
>  
> -        get_bits(&alac->gb, 12); /* unknown, skip 12 bits */
> -        hassize = get_bits(&alac->gb, 1); /* the output sample size is stored soon */
> -        wasted_bytes = get_bits(&alac->gb, 2); /* unknown ? */
> +    /* 2^result = something to do with output waiting.
> +     * perhaps matters if we read > 1 frame in a pass?
> +     */
> +    get_bits(&alac->gb, 4);
> +    get_bits(&alac->gb, 12); /* unknown, skip 12 bits */
> +    /* the output sample size is stored soon */
> +    hassize = get_bits(&alac->gb, 1);
>  
> +    wasted_bytes = get_bits(&alac->gb, 2); /* unknown ? */

cosmetic


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070706/9ca43da6/attachment.pgp>