[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [4/7] - G.729 core

[Vladimir Voroshilov]

2008/8/14 Michael Niedermayer <michaelni at gmx.at>:
> On Sat, Jul 12, 2008 at 02:31:29PM +0700, Vladimir Voroshilov wrote:

[...]

> ok, and do not hesitate to commit part

Am i right that g279.h (except one issue with #define , see below)
with fixed enum issue can be committed right now?
Does you also mean that i can commit g729dec partially? If so, should
i commit only ok'ed parts or code which was not explicitly ok'ed but
not mentioned in your mail can be applied too?

>> +#define VQ_1ST_BITS          7  ///< first stage vector of quantizer (size in bits)
>> +#define VQ_2ND_BITS          5  ///< second stage vector of quantizer (size in bits)

This...

>> +#define GC_1ST_IDX_BITS_8K   3  ///< gain codebook (first stage) index, 8k mode (size in bits)
>> +#define GC_2ND_IDX_BITS_8K   4  ///< gain codebook (second stage) index, 8k mode (size in bits)

...and this are also used in g729data.h (as table sizes), the rest are
already removed in local tree.
I prefer leave  them here. I can remove duplicating variable
(vq_1st_bits member and similar) from format structure, since values
are equal for all formats supported by this code.

> are these defines really usefull?
>
> they are a little like X=get_bits(5) vs X=get_bits(X_BITS)

See above

>
>
> [...]
>> +/**
>> + * \brief Decodes one G.729 frame (10 bytes long) into parameter vector.
>> + * \param format used format (8k/4.4k/etc)
>> + * \param buf 10 bytes of decoder parameters
>> + * \param buf_size size of input buffer
>> + * \param parm [out] decoded codec parameters
>> + *
>> + * \return 1 if frame erasure detected, 0 - otherwise
>> + */
>> +static int g729_bytes2parm(int format, const uint8_t *buf, int buf_size, G729_parameters *parm)
>> +{
>> +    GetBitContext gb;
>> +    int i, frame_nonzero;
>> +
>> +    frame_nonzero = 0;
>> +    for(i=0; i<buf_size; i++)
>> +        frame_nonzero |= buf[i];
>> +
>> +    if(!frame_nonzero)
>> +    {
>> +        memset(parm, 0, sizeof(G729_parameters));
>> +        return 1;
>> +    }
>> +
>> +    init_get_bits(&gb, buf, buf_size);
>> +
>
>> +    parm->ma_predictor     = get_bits(&gb, formats[format].ma_predictor_bits);
>> +    parm->quantizer_1st    = get_bits(&gb, formats[format].vq_1st_bits);
>> +    parm->quantizer_2nd_lo = get_bits(&gb, formats[format].vq_2nd_bits);
>> +    parm->quantizer_2nd_hi = get_bits(&gb, formats[format].vq_2nd_bits);
>> +
>> +    parm->ac_index[0]      = get_bits(&gb, formats[format].ac_index_1st_bits);
>> +    parm->parity           = get_bits(&gb, formats[format].parity_bits);
>> +    parm->fc_indexes[0]    = get_bits(&gb, formats[format].fc_indexes_bits);
>> +    parm->pulses_signs[0]  = get_bits(&gb, formats[format].fc_signs_bits);
>> +    parm->gc_1st_index[0]  = get_bits(&gb, formats[format].gc_1st_index_bits);
>> +    parm->gc_2nd_index[0]  = get_bits(&gb, formats[format].gc_2nd_index_bits);
>> +
>> +    parm->ac_index[1]      = get_bits(&gb, formats[format].ac_index_2nd_bits);
>> +    parm->fc_indexes[1]    = get_bits(&gb, formats[format].fc_indexes_bits);
>> +    parm->pulses_signs[1]  = get_bits(&gb, formats[format].fc_signs_bits);
>> +    parm->gc_1st_index[1]  = get_bits(&gb, formats[format].gc_1st_index_bits);
>> +    parm->gc_2nd_index[1]  = get_bits(&gb, formats[format].gc_2nd_index_bits);
>
> isnt it possible to read these values where they are needed like
> in done in every other decoder ?

VOC format (based on G.729 AnnexD) uses slightly different packet coding.
But it can be handled by my g729d code after replacing only this one routine.
With inlined readings it will be harder to implement, imho.

Please answer on commit question.

P.S. I'll post updated patch later

-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719