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

[Vladimir Voroshilov]

2008/8/16 Vladimir Voroshilov <voroshil at gmail.com>:
> 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?

Explicitly ok'ed parts were committed.

>>> +#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];
>>> +

One extra byte in buffer for misc flags (will be set by demuxer
accordinally) will avoid
code above. This will make also possible to support dynamic switching
between various formats
in the same stream.

>>> +    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);

Second parameter in above lines was replaced by constants.

>>> +
>>> +    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

Here is patch.

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



-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 04_g729dec_80.diff
Type: text/x-diff
Size: 25710 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080817/c2f65df7/attachment.diff>