[FFmpeg-devel] [PATCH] QCELP decoder
Kenan Gillet
kenan.gillet
Fri Nov 21 18:07:18 CET 2008
On Nov 21, 2008, at 3:33 AM, Michael Niedermayer wrote:
> On Thu, Nov 20, 2008 at 08:53:37PM -0800, Kenan Gillet wrote:
>> On Thu, Nov 20, 2008 at 5:08 PM, Michael Niedermayer <michaelni at gmx.at
>> > wrote:
>>> On Thu, Nov 20, 2008 at 10:46:49AM -0800, Kenan Gillet wrote:
>>>> On Sat, Nov 15, 2008 at 3:10 PM, Michael Niedermayer <michaelni at gmx.at
>>>> > wrote:
>>>>> On Fri, Nov 14, 2008 at 03:32:51PM -0800, Kenan Gillet wrote:
>>>>>> Hi,
>>>>>> On Fri, Nov 14, 2008 at 2:27 PM, Michael Niedermayer <michaelni at gmx.at
>>>>>> > wrote:
>>>>>>> On Fri, Nov 14, 2008 at 12:17:50PM -0800, Kenan Gillet wrote:
>>>>>>>>
>>>>>>>> On Nov 14, 2008, at 2:14 AM, Michael Niedermayer wrote:
>>
>> [...]
>>>>>> +
>>>>>> +/// @defgroup qcelp_unpacked_data_frame QCELP unpacked data
>>>>>> frame
>>>>>> +/// @{
>>>>>> + uint8_t cbsign[16];
>>>>>
>>>>>> + uint8_t cbgain[16];
>>>>>> + uint8_t cindex[16];
>>>>>> + uint8_t plag[4];
>>>>>> + uint8_t pfrac[4];
>>>>>> + uint8_t pgain[4];
>>>>>> + uint8_t lspv[10]; /*!< LSP for
>>>>>> RATE_OCTAVE, LSPV for other rates */
>>>>>> + uint8_t reserved; /*!< on all but
>>>>>> rate 1/2 packets */
>>>>>> +/// @}
>>>>>> +
>>>>>> + uint8_t erasure_count;
>>>>>> + uint8_t octave_count; /*!< count the
>>>>>> consecutive RATE_OCTAVE frames */
>>>>>> + float prev_lspf[10];
>>>>>> + float predictor_lspf[10]; /*!< LSP
>>>>>> predictor,
>>>>>> + only use for
>>>>>> RATE_OCTAVE and I_F_Q */
>>>>>> + float pitch_synthesis_filter_mem[303];
>>>>>> + float pitch_pre_filter_mem[303];
>>>>>> + float rnd_fir_filter_mem[180];
>>>>>> + float formant_mem[170];
>>>>>> + float last_codebook_gain;
>>>>>> + int prev_g1[2];
>>>>>> + int prev_framerate;
>>>>>> + float prev_pitch_gain[4];
>>>>>> + uint8_t prev_pitch_lag[4];
>>>>>> + uint16_t first16bits;
>>>>>> +} QCELPContext;
>>>>>
>>>>> i somehow think this struct does not belong in qcelpdata.h
>>>>> but rather qcelpdec.c
>>>>>
>>>>
>>>> I agree, but it is needed by the unpacking table.
>>>> should I just put the struct in qcelpdec.c and include
>>>> qcelpdata.h after ?
>>>
>>> ok (maybe diego will want it to be renamed to .c though ...)
>>
>> ok to keep it the way it is?
>>
>> or to move the QCELPContext in qcelpdec.c ?
>
> ok to move QCELPContext to qcelpdec.c
>
> also if someone wants to rename qcelpdata.h to qcelpdata.c iam ok
> with that
>
> (and no, iam not truely happy about the result if someone has a
> better idea,
> but keeping it .h will likely break check headers)
> spliting the structs though may be an alternative ...
I thought and still think splitting of the struct would be cleaner.
Would QCELPFrame ok for the name of struct ?
>
>>
>>>
>>>
>>> [...]
>>>>>
>>>>> [...]
>>>>>
>>>>>
>>>>>> static void warn_insufficient_frame_quality(AVCodecContext
>>>>>> *avctx,
>>>>>> const char
>>>>>> *message) {
>>>>>> av_log(avctx, AV_LOG_WARNING, "Frame #%d, IFQ: %s\n", avctx-
>>>>>> >frame_number, message);
>>>>>> }
>>>>>>
>>>>>> +static int qcelp_decode_frame(AVCodecContext *avctx,
>>>>>> + void *data,
>>>>>> + int *data_size,
>>>>>> + uint8_t *buf,
>>>>>> + const int buf_size) {
>>>>>> + QCELPContext *q = avctx->priv_data;
>>>>>> + float *outbuffer = data;
>>>>>> + int i;
>>>>>> + float quantized_lspf[10], lpc[10];
>>>>>> + float gain[16];
>>>>>> + float *formant_mem;
>>>>>> +
>>>>>> + if ((q->framerate = determine_framerate(avctx, buf_size,
>>>>>> &buf)) == I_F_Q) {
>>>>>> + warn_insufficient_frame_quality(avctx, "Framerate
>>>>>> cannot be determined.");
>>>>>> + goto erasure;
>>>>>> + }
>>>>>> +
>>>>>> + if (q->framerate == RATE_OCTAVE &&
>>>>>> + (q->first16bits = AV_RB16(buf)) == 0xFFFF) {
>>>>>> + warn_insufficient_frame_quality(avctx, "Framerate is
>>>>>> 1/8 and first 16 bits are on.");
>>>>>> + goto erasure;
>>>>>> + }
>>>>>> +
>>>>>> + if (q->framerate > SILENCE) {
>>>>>> + const QCELPBitmap *bitmaps =
>>>>>> qcelp_unpacking_bitmaps_per_rate[q->framerate];
>>>>>> + const QCELPBitmap *bitmaps_end =
>>>>>> qcelp_unpacking_bitmaps_per_rate[q->framerate]
>>>>>> + + qcelp_bits_per_rate[q-
>>>>>> >framerate];
>>>>>> + uint8_t *unpacked_data = (uint8_t *)q;
>>>>>> +
>>>>>
>>>>>> + init_get_bits(&q->gb, buf, qcelp_bits_per_rate[q-
>>>>>> >framerate]);
>>>>>
>>>>> qcelp_bits_per_rate does not seem correct here nor does its name
>>>>> seem
>>>>> to match what it contains
>>>>
>>>>
>>>> yes changed back to buf_size.
>>>>
>>>> what about changing qcelp_bits_per_rate to
>>>> qcelp_unpacking_bitmaps_per_rate_len
>>>> because it really is the len of the unpacking bitmaps, or do you
>>>> have
>>>> a better suggestion ?
>>>
>>> the suggested name is too long IMHO
>>
>> qcelp_unpacking_bitmaps_lengths ?
>
> ok
>
done
More information about the ffmpeg-devel
mailing list