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

[Diego Biurrun]

On Tue, Jul 03, 2007 at 10:16:49PM +0200, reynaldo wrote:
> 
> Log:
> Some structural code frame, only thing I would consider ready for initial peer review here is the parsing and reordering routine. feel free to destroy everything along with it thouhg ;)

Huh, that's long, did your enter key break? ;-p

Below are some nits I noticed while reading through the code.

> --- (empty file)
> +++ qcelp/doc/NOTES	Tue Jul  3 22:16:49 2007
> @@ -0,0 +1,36 @@
> +This notes are abstracts from the standard that might

these

> +be usesfull in understanding whats going on.

useful, what's

> +Parameters codes transmited for each rate packet (Page 25)

parameter codes transmitted

> --- (empty file)
> +++ qcelp/qcelp.h	Tue Jul  3 22:16:49 2007

Why not call this file qcelpdec.h?

> @@ -0,0 +1,242 @@
> +/*
> + * QCELP Decoder

decoder

> +/**
> + * @file qcelp.h
> + * QCELP decoder.

stray period

> +    I_F_Q,        /* insuficient frame quality */

insufficient

> + * YOU WONT SEE ANY mention of a REFERENCE nor an UNIVERSAL frame

Your keyboard seems to have the same problem as Michael's, stuck ' key.

> + * This are the reference frame slices. Each touple will be mapped to a

these, tuple

> + * QCELPBitmap showing the location of each bit in the input * with
> + * respect to a transmision code in the 'universal frame'.

transmission

I don't understand this sentence.  What does the * mean?

> + * it would be really nice if someone reviewed this numbers :)

these

> + * Oce frame is parsed all data gets stored in QCELPFrame.data acording
> + * to this structure:

Once the frame?

> + * Position of the transmision codes inside the universal frame.

see above

> +/* rest its currently unused */

is

> --- (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_SMACKER_DECODER)         += smacker.o
> ++OBJS-$(CONFIG_QCELP_DECODER)           += qcelpdec.o
> + OBJS-$(CONFIG_SMC_DECODER)             += smc.o
> +Index: libavcodec/allcodecs.c
> +===================================================================
> +--- libavcodec/allcodecs.c	(revision 9314)
> ++++ libavcodec/allcodecs.c	(working copy)
> +@@ -196,6 +196,7 @@
> +     REGISTER_DECODER(SMACKAUD, smackaud);
> ++    REGISTER_DECODER(QCELP, qcelp);
> +     REGISTER_ENCDEC (SONIC, sonic);
> +Index: libavcodec/allcodecs.h
> +===================================================================
> +--- libavcodec/allcodecs.h	(revision 9314)
> ++++ libavcodec/allcodecs.h	(working copy)
> +@@ -161,6 +161,7 @@
> + extern AVCodec smacker_decoder;
> ++extern AVCodec qcelp_decoder;
> + extern AVCodec smc_decoder;

<default comment>

> --- (empty file)
> +++ qcelp/qcelpdec.c	Tue Jul  3 22:16:49 2007
> @@ -0,0 +1,235 @@
> +/*
> + * QCELP Decoder

see above

> + * @file qcelpdec.c
> + * QCELP decoder.

see above

> +typedef struct {
> +    GetBitContext gb;
> +    QCELPFrame    *frame;
> +    uint8_t       erasure_count;

This variable name is not very good.  What is it supposed to represent?

> +    /*
> +     * FIXME this comment should actually make some sence ..

sense

> +     * inside the big REFERECE_FRAME array. We then proceed with

referenNce

> +    /* particularities for rate 1/8 */

I think you mean "peculiarities" here if you are talking about special cases.
The English word "particularity" is close to "precisión" in Spanish.

Diego