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

Michael Niedermayer michaelni
Thu Aug 14 03:31:55 CEST 2008


On Sat, Jul 12, 2008 at 02:31:29PM +0700, Vladimir Voroshilov wrote:
> 2008/7/4 Diego Biurrun <diego at biurrun.de>:
> > On Tue, Jul 01, 2008 at 03:06:40AM +0700, Vladimir Voroshilov wrote:
> 
> [...]
> 
> >> +/**
> >> + * maximum size of one subframe over supported formats
> >
> > huh?
> 
> maximum possible subframe size
> 
> >> +AVCodec g729_decoder =
> >> +{
> >> +    "g729",
> >> +    CODEC_TYPE_AUDIO,
> >> +    CODEC_ID_G729,
> >> +    sizeof(G729_Context),
> >> +    ff_g729_decoder_init,
> >> +    NULL,
> >> +    NULL,
> >> +    ff_g729_decode_frame,
> >> +};
> >
> > The codec long is missing.
> 
> Can suggest only obvious "G.729"


[...]
> diff --git a/libavcodec/g729.h b/libavcodec/g729.h
> new file mode 100644
> index 0000000..12b6cd8
> --- /dev/null
> +++ b/libavcodec/g729.h
> @@ -0,0 +1,126 @@
> +/*
> + * G.729 decoder
> + * Copyright (c) 2008 Vladimir Voroshilov
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#ifndef FFMPEG_G729_H
> +#define FFMPEG_G729_H
> +
> +#include <stdint.h>

ok, and do not hesitate to commit parts that have a ok below

[...]

> +/// Moving Average (MA) prediction order
> +#define MA_NP                      4
> +
> +/**
> + * minimum quantized LSF value (3.2.4)
> + * 0.005 in Q13
> + */
> +#define LSFQ_MIN                   40
> +
> +/**
> + * maximum quantized LSF value (3.2.4)
> + * 3.135 in Q13
> + */
> +#define LSFQ_MAX                   25681
> +
> +/**
> + * minimum LSF distance (3.2.4)
> + * 0.0391 in Q13
> + */
> +#define LSFQ_DIFF_MIN              321

ok

[...]
> +/**
> + * maximum possible subframe size
> + */
> +#define MAX_SUBFRAME_SIZE 44

ok


> +
> +
> +#define MA_PREDICTOR_BITS    1  ///< switched MA predictor of LSP quantizer (size in bits)
> +#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)
> +
> +#define AC_IDX_1ST_BITS_8K   8  ///< adaptive codebook index for first subframe, 8k mode (size in bits)
> +#define AC_IDX_2ND_BITS_8K   5  ///< adaptive codebook index for second subframe, 8k mode (size in bits)
> +#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)
> +#define FC_SIGNS_BITS_8K     4  ///< fixed-codebook signs, 8k mode (size in bits)
> +#define FC_INDEXES_BITS_8K   13 ///< fixed-codebook indexes, 8k mode (size in bits)
> +
> +#define FC_SIGNS_BITS_4K4    4  ///< number of pulses in fixed-codebook vector, 4.4k mode
> +#define FC_INDEXES_BITS_4K4  17 ///< fixed-codebook indexes, 4.4k mode (size in bits)

are these defines really usefull?

they are a little like X=get_bits(5) vs X=get_bits(X_BITS)


[...]
> +#define FORMAT_G729_8K  0
> +#define FORMAT_G729_4K4  1

enum


> +
> +#define DECISION_NOISE        0
> +#define DECISION_INTERMEDIATE 1
> +#define DECISION_VOICE        2

unused

[...]

> +
> +#endif // FFMPEG_G729_H
> diff --git a/libavcodec/g729dec.c b/libavcodec/g729dec.c
> new file mode 100644
> index 0000000..6236b8d
> --- /dev/null
> +++ b/libavcodec/g729dec.c
> @@ -0,0 +1,708 @@
> +/*
> + * G.729 decoder
> + * Copyright (c) 2008 Vladimir Voroshilov
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include <stdlib.h>
> +#include <inttypes.h>
> +#include <limits.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <math.h>
> +#include <assert.h>
> +
> +#include "avcodec.h"
> +#include "libavutil/avutil.h"
> +#include "bitstream.h"

ok


> +
> +#define FRAC_BITS 15
> +#include "mathops.h"
> +
> +#include "g729.h"
> +#include "lsp.h"
> +#include "acelp_math.h"
> +#include "acelp_filters.h"
> +#include "acelp_pitch_delay.h"
> +#include "acelp_vectors.h"
> +#include "g729data.h"
> +#include "g729postfilter.h"
> +

> +/*
> +
> +Validation results on ITU test vectors for fixed-point G729.A:
> +
> +    Results are bit-exact equal to reference fixed-point G729.A decoder
> +    over all ITU test vectors.

is this still true?


> +
> +Naming conventions:
> +
> +Routines:
> +g729_*    : common for G.729 and annexes
> +ff_       : interface to FFmpeg API
> +no prefix : common routines for miscellaneous tasks (e.g. fixed-point math operations)

rejected


> +
> +Parameters:
> +[out]     : all data in array will be overwritten regardless of previous value
> +[in/out]  : array is filled using previously stored data
> +no mark   : input data only

self explanatory thus rejected


> +
> +Misc:
> +Q<n>      : Means "value * (1<<n)" (i.e. fixed-point value with 2^n base)
> +
> +*/
> +


> +/*
> +-------------------------------------------------------------------------------
> +    Format description
> +-------------------------------------------------------------------------------
> +*/

this (below) is the main context struct not a "Format description"


> +typedef struct
> +{
> +    int format;                 ///< format index from formats array
> +    int subframe_size;          ///< number of samples produced from one subframe
> +    int frame_erasure;          ///< frame erasure detected during decoding
> +    int bad_pitch;              ///< parity check failed
> +    /// past excitation signal buffer
> +    int16_t exc_base[2*MAX_SUBFRAME_SIZE+PITCH_DELAY_MAX+INTERPOL_LEN];
> +    int16_t* exc;               ///< start of past excitation data in buffer
> +    int pitch_delay_int_prev;   ///< integer part of previous subframe's pitch delay (4.1.3)
> +    int16_t lq_prev[MA_NP][10]; ///< (2.13) LSP quantizer output (3.2.4)
> +    int16_t lsp_prev[10];       ///< (0.15) LSP coefficients from previous frame (3.2.5)
> +    int16_t lsf_prev[10];       ///< (2.13) LSF coefficients from previous frame
> +    int16_t quant_energy[4];    ///< (5.10) past quantized energy
> +    int16_t gain_pitch;         ///< (1.14) pitch gain of previous subframe (3.8) [SHARP_MIN ... SHARP_MAX]
> +    int16_t gain_code;          ///< (14.1) gain code of previous subframe
> +    int16_t pitch_sharp;        ///< pitch sharpening of the previous frame
> +    /// residual signal buffer (used in long-term postfilter)
> +    int16_t residual[MAX_SUBFRAME_SIZE + RES_PREV_DATA_SIZE];
> +    /// previous speech data for residual calculation filter
> +    int16_t res_filter_data[MAX_SUBFRAME_SIZE+10];
> +    /// previous speech data for LP synthesis filter
> +    int16_t syn_filter_data[10];
> +    /// previous speech data for short-term postfilter
> +    int16_t pos_filter_data[MAX_SUBFRAME_SIZE+10];
> +    int16_t voicing;            ///< voicing decision from previous frame, G.729
> +    int16_t ht_prev_data;       ///< previous data for 4.2.3, equation 86
> +    int gain_coeff;             ///< (1.14) gain coefficient (4.2.4)
> +    uint16_t rand_value;        ///< random number generator value (4.4.4)
> +    int ma_predictor_prev;      ///< MA predictor from previous frame
> +    //High-pass filter data
> +    int hpf_f[2];               ///< (14.14)
> +    int16_t hpf_z[2];
> +}  G729_Context;

please place all comments to the left of the varables or if the lines really
are too long leave a empty line before the comment and another after the
variable

[...]
> +/**
> + * \brief pseudo random number generator
> + */
> +static inline uint16_t g729_random(uint16_t value)
> +{
> +    return 31821 * value + 13849;
> +}

ok


> +
> +/**
> + * \brief Check parity bit (3.7.2 of G.729).
> + * \param ac_index_1st adaptive codebook index for first subframe
> + * \param parity parity bit for P1
> + *
> + * \return 1 if parity check is OK, 0 - otherwise.
> + */
> +int g729_parity_check(uint8_t ac_index_1st, int parity)
> +{
> +    //Parity is calculated on six most significant bits of P1.
> +   return ((0x6996966996696996ULL >> (ac_index_1st >> 2)) ^ parity) & 1;
> +}

IMHO g729_get_parity(ac_index_1st) == parity is clearer and more flexible


[...]
> +/**
> + * \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 ?


[...]
> +AVCodec g729_decoder =
> +{
> +    "g729",
> +    CODEC_TYPE_AUDIO,
> +    CODEC_ID_G729,
> +    sizeof(G729_Context),
> +    ff_g729_decoder_init,
> +    NULL,
> +    NULL,
> +    ff_g729_decode_frame,
> +    .long_name = NULL_IF_CONFIG_SMALL("G.729"),
> +};

ok

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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/20080814/847bb64e/attachment.pgp>



More information about the ffmpeg-devel mailing list