[FFmpeg-devel] [PATCH] QCELP decoder

[Michael Niedermayer]

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


> 
> >
> >
> > [...]
> >> >
> >> > [...]
> >> >
> >> >
> >> >>  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


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081121/e9f281ab/attachment.pgp>