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

[Reynaldo Verdejo]

On 7/4/07, Benjamin Larsson <banan at ludd.ltu.se> wrote:
>
> Hi!


Hi Benjamin

> +#define QCELP_RATE_FULL_BITMAP \
> >
> And make tables of these in the future.


That's the plan. As of now, its easier for me to have it this way


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


'Once', will correct it, thanks

> + * LSPVs    60    61 62 63 64
> > + * RSVD     65
> > + * LSP      66    67 68 69 70 71 72 73 74 75
> > + * CBSEED   76
> > + *
> > +
> *-------------------------------------------------------------------------*/
> > +
> > +typedef struct
> > +{
> > +    qcelp_packet_type type;
> > +    uint8_t data[76];       /* holds all data from a frame (_once_
> parsed) */
> > +    uint8_t bits;
> > +} QCELPFrame;
> >
>
> Move this to the c file, and use simpler types(int). And rename
> qcelp_packet_type to qcelp_packet_rate.


ok but ..

a) do you want me to move the comment too?
b) whats wrong with uint8_t ? guess I could use unsigned char too but why?

> +
> > +typedef struct {
> > +    GetBitContext gb;
> > +    QCELPFrame    *frame;
> > +    uint8_t       erasure_count;
> > +    uint8_t       ifq_count;
> > +} QCELPContext;
> >
>
> Use the int type and don't allocate frame.


again , see b) above.

Use int where you can.


and again ..


> > +            q->frame->type = RATE_FULL;
> > +            q->frame->bits = qcelp_bits_per_type[RATE_FULL];
> >

To not use the qcelp_bits_per_type table and just write the value here
> might be more clear if the table is only referenced once.
>

I'm unsure right now but I will keep it in mind

> +    bitcount=0;
> > +    while(bitcount < q->frame->bits)
> >
>
> Can you use a for loop instead ? Should give more compact and
> understandable code.


Sure, why not.

That's all for now, keep up the good work.


Will try ;)

Best regards

      Reynaldo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20070704/d4d0fb42/attachment.htm>