[FFmpeg-devel] [PATCH] G.729 and G.729D decoders
Michael Niedermayer
michaelni
Sun Apr 20 00:03:00 CEST 2008
On Sun, Apr 20, 2008 at 12:41:26AM +0700, Vladimir Voroshilov wrote:
> Hi, All again
>
> Here is current version of G.729 decoder.
>
> 1 G.729A (reduced complexity decoder) was replaced with full G.729
> (G.729 @8kHz)
> 2. G.729D (G.729 @6.4kHz) is implemented (is dynamic switching between
> 8k and 6.4k mode required?)
> 3. Code was splitted into three parts: header, main body and lookup tables.
> 4. All unnecessary clippings were removed (the rest are required for
> passing ITU's tests).
> 5. Decoder passes ITU tests (both G.729 and G.729D)
>
> I tried also to fix all issues mentioned by Michael.
>
> g729h_15.diff - header
> g729dec_15.diff.gz - decoder
> g729tab_15.diff.gz - lookup tables
> g729_build_15.diff - build part
>
> P.S. Files are gzipped due to overall size >80k
quick review of the non gziped parts is below
i will review the rest as soon as i see a non compressed patch <100k
which is selfcontained, that is it is usefull as it is, a header alone
is not a selfcontanied patch it is useless as it is.
[...]
> +#define Q15_MAX 0x3fffffff
> +#define Q15_MIN (-Q15_MAX-1)
> +#define Q15_Q0_ROUND(a) (((a)+0x4000) >> 15)
> +
> +#define Q14_MAX 0x1fffffff
> +#define Q14_MIN (-Q14_MAX-1)
> +#define Q14_Q0_ROUND(a) (((a)+0x2000) >> 14)
> +
> +#define Q13_MAX 0xfffffff
> +#define Q13_MIN (-Q13_MAX-1)
> +
> +#define Q12_MAX 0x7ffffff
> +#define Q12_MIN (-Q12_MAX-1)
> +#define Q12_Q0_ROUND(a) (((a)+0x800) >> 12)
I would just write the constants in the code, i mean
0x3fffffff is clear Q15_MAX is something one has to look up
the same is true for MIN and ROUND
if you dont like 0x3fffffff, you could always write (1<<30)-1
[...]
> +#define MIN_GPLT 21845 /* LT gain minimum (Q15) */
> +
> +/**
> + * Fractional delay resolution
> + */
> +#define F_UP_PST 8
these 2 could have more descriptive names
> +
> +/**
> + * Short interpolation filter length
> + */
> +#define SHORT_INT_FILT_LEN 4
> +
> +/**
> + * Long interpolation filter length
> + */
> +#define LONG_INT_FILT_LEN 16
> +
> +/**
> + * Maximum size of one subframe over supported formats
> + */
> +#define MAX_SUBFRAME_SIZE 44
> +#define MEM_RES2 (PITCH_LAG_MAX+9)
and that is what? Please add a comment or improve the name
> +
> +#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_IND_1ST_BITS_8K 8 ///< adaptive codebook index for first subframe, 8k mode (size in bits)
> +#define AC_IND_2ND_BITS_8K 5 ///< adaptive codebook index for second subframe, 8k mode (size in bits)
index is generally abbreviated as idx or ndx
> +#define GC_1ST_IND_BITS_8K 3 ///< gain codebook (first stage) index, 8k mode (size in bits)
> +#define GC_2ND_IND_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 AC_IND_1ST_BITS_64 8 ///< adaptive codebook index for first subframe, 6.4k mode (size in bits)
i would abbreviate 6.4k as 6K4 not 64
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/20080420/4ca84d30/attachment.pgp>
More information about the ffmpeg-devel
mailing list