[FFmpeg-soc] [soc]: r310 - in qcelp: README doc doc/NOTES doc/TODO qcelp.h qcelp_glue.diff qcelpdec.c srcprepare.sh

[Michael Niedermayer]

Hi

On Tue, Jul 03, 2007 at 10:16:49PM +0200, reynaldo wrote:
[...]
> +static uint16_t qcelp_bits_per_type[]={266,124,54,20};

const


[...]
> +/*
> + * Reference frame, finally :-)
> + *
> + */
> +
> +static QCELPBitmap QCELP_REFERENCE_FRAME[]={QCELP_RATE_FULL_BITMAP,
> +                                            QCELP_RATE_HALF_BITMAP,
> +                                            QCELP_RATE_4THR_BITMAP,
> +                                            QCELP_RATE_8THR_BITMAP};

doxygen hates you, because it has a stomach ache from your comments :)

also this one is missing const


[...]
> Added: qcelp/qcelp_glue.diff
> ==============================================================================
> --- (empty file)
> +++ qcelp/qcelp_glue.diff	Tue Jul  3 22:16:49 2007
> @@ -0,0 +1,48 @@
> +Index: libavcodec/Makefile
> +===================================================================
> +--- libavcodec/Makefile	(revision 9314)
> ++++ libavcodec/Makefile	(working copy)
> +@@ -147,6 +147,7 @@
> + OBJS-$(CONFIG_SHORTEN_DECODER)         += shorten.o golomb.o
> + OBJS-$(CONFIG_SMACKAUD_DECODER)        += smacker.o
> + OBJS-$(CONFIG_SMACKER_DECODER)         += smacker.o
> ++OBJS-$(CONFIG_QCELP_DECODER)           += qcelpdec.o
> + OBJS-$(CONFIG_SMC_DECODER)             += smc.o

i already hear diegos head exploding short after some "alphabetical order"
scream

[...]

> +#include <stdio.h>

which part needs this?


> +
> +/* The following includes have the bitstream reader, various dsp functions and the various defaults */

> +#define ALT_BITSTREAM_READER

why?


[...]
> +    switch(buf_size)
> +    {
> +        case 34:
> +            q->frame->type = RATE_FULL;
> +            q->frame->bits = qcelp_bits_per_type[RATE_FULL];
> +            order = QCELP_REFERENCE_FRAME + QCELP_FULLPKT_REFERENCE_POS;
> +            break;
> +        case 16:
> +            q->frame->type = RATE_HALF;
> +            q->frame->bits = qcelp_bits_per_type[RATE_HALF];
> +            order = QCELP_REFERENCE_FRAME + QCELP_HALFPKT_REFERENCE_POS;
> +            break;
> +        case 7:
> +            q->frame->type = RATE_QUARTER;
> +            q->frame->bits = qcelp_bits_per_type[RATE_QUARTER];
> +            order = QCELP_REFERENCE_FRAME + QCELP_4THRPKT_REFERENCE_POS;
> +            break;
> +        case 3:
> +            q->frame->type = RATE_OCTAVE;
> +            q->frame->bits = qcelp_bits_per_type[RATE_OCTAVE];
> +            order = QCELP_REFERENCE_FRAME + QCELP_8THRPKT_REFERENCE_POS;
> +            break;
> +        case 0: /* FIXME */
> +            q->frame->type = BLANK;
> +            q->frame->bits = 0;
> +            break;
> +        default:
> +            q->frame->type = RATE_UNKNOWN;
> +            q->frame->bits = 0;

> +            printf("UNKNOWN PACKET RATE\n");

av_log()


> +            break;
> +    }

q->frame->bits= qcelp_bits_per_type[q->frame->type]
can be factored out


[...]

> +        q->frame->data[ order[bitcount].index ] |=
> +        get_bits1(&q->gb)>>(order[bitcount].bitpos);

whatever this is supposed to do it wont work
get_bits1() >> X for X>0 is 0
also the () is superflous


[...]
> +AVCodec qcelp_decoder =
> +{
> +    .name = "qcelp",
> +    .type = CODEC_TYPE_AUDIO,
> +    .id = CODEC_ID_QCELP,
> +    .priv_data_size = sizeof(QCELPContext),
> +    .init = qcelp_decode_init,
> +    .close = qcelp_decode_close,
> +    .decode = qcelp_decode_frame,
> +};

you could align the = vertically here


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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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-soc/attachments/20070703/982998af/attachment.pgp>