From 27169ca9d0749d2d93c46fd250136c8d464d3f43 Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin <jean-marc.valin@octasic.com> Date: Mon, 16 May 2011 14:10:04 -0400 Subject: [PATCH] Addressing all the FIXMEs in the code Includes better error handling in fft/mdct init --- libcelt/celt.c | 2 +- libcelt/header.c | 124 --------------------------------------------- libcelt/kiss_fft.c | 10 ++-- libcelt/mdct.c | 7 +-- libcelt/mdct.h | 2 +- libcelt/modes.c | 10 ++-- 6 files changed, 16 insertions(+), 139 deletions(-) delete mode 100644 libcelt/header.c diff --git a/libcelt/celt.c b/libcelt/celt.c index 58ae2ece7..c2cf970de 100644 --- a/libcelt/celt.c +++ b/libcelt/celt.c @@ -671,7 +671,7 @@ static int tf_analysis(const CELTMode *m, celt_word16 *bandLogE, celt_word16 *ol *tf_sum += metric[i]; } /*printf("\n");*/ - /* FIXME: Figure out how to set this */ + /* TODO: Detect the extreme transients that require tf_select = 1 */ tf_select = 0; cost0 = 0; diff --git a/libcelt/header.c b/libcelt/header.c deleted file mode 100644 index 37b11bf5c..000000000 --- a/libcelt/header.c +++ /dev/null @@ -1,124 +0,0 @@ -/* Copyright (c) 2007 CSIRO - Copyright (c) 2007-2009 Xiph.Org Foundation - Written by Jean-Marc Valin */ -/* - Redistribution and use in source and binary forms, with or without - modification, are permitted provided that the following conditions - are met: - - - Redistributions of source code must retain the above copyright - notice, this list of conditions and the following disclaimer. - - - Redistributions in binary form must reproduce the above copyright - notice, this list of conditions and the following disclaimer in the - documentation and/or other materials provided with the distribution. - - THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR - CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, - EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, - PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR - PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF - LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING - NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -*/ - -#ifdef HAVE_CONFIG_H -#include "config.h" -#endif - -#include "celt_header.h" -#include "os_support.h" -#include "modes.h" - -static celt_uint32 -_le_32 (celt_uint32 i) -{ - celt_uint32 ret=i; -#if !defined(__LITTLE_ENDIAN__) && ( defined(WORDS_BIGENDIAN) || defined(__BIG_ENDIAN__) ) - ret = (i>>24); - ret += (i>>8) & 0x0000ff00; - ret += (i<<8) & 0x00ff0000; - ret += (i<<24); -#endif - return ret; -} - -int celt_header_init(CELTHeader *header, const CELTMode *m, int frame_size, int channels) -{ - if (header==NULL) - return CELT_BAD_ARG; - - CELT_COPY(header->codec_id, "CELT ", 8); - CELT_COPY(header->codec_version, "experimental ", 20); - - /* FIXME: Set that to zero when we freeze */ - header->version_id = 0x80001000; - header->header_size = 56; - header->sample_rate = m->Fs; - header->nb_channels = channels; - /*FIXME: This won't work for variable frame size */ - header->frame_size = frame_size; - header->overlap = m->overlap; - header->bytes_per_packet = -1; - header->extra_headers = 0; - return CELT_OK; -} - -int celt_header_to_packet(const CELTHeader *header, unsigned char *packet, celt_uint32 size) -{ - celt_int32 * h; - - if ((size < 56) || (header==NULL) || (packet==NULL)) - return CELT_BAD_ARG; /* FAIL */ - - CELT_MEMSET(packet, 0, sizeof(*header)); - /* FIXME: Do it in an alignment-safe manner */ - - /* Copy ident and version */ - CELT_COPY(packet, (unsigned char*)header, 28); - - /* Copy the int32 fields */ - h = (celt_int32*)(packet+28); - *h++ = _le_32 (header->version_id); - *h++ = _le_32 (header->header_size); - *h++ = _le_32 (header->sample_rate); - *h++ = _le_32 (header->nb_channels); - *h++ = _le_32 (header->frame_size); - *h++ = _le_32 (header->overlap); - *h++ = _le_32 (header->bytes_per_packet); - *h = _le_32 (header->extra_headers); - - return sizeof(*header); -} - -int celt_header_from_packet(const unsigned char *packet, celt_uint32 size, CELTHeader *header) -{ - celt_int32 * h; - - if ((size < 56) || (header==NULL) || (packet==NULL)) - return CELT_BAD_ARG; /* FAIL */ - - CELT_MEMSET((unsigned char*)header, 0, sizeof(*header)); - /* FIXME: Do it in an alignment-safe manner */ - - /* Copy ident and version */ - CELT_COPY((unsigned char*)header, packet, 28); - - /* Copy the int32 fields */ - h = (celt_int32*)(packet+28); - header->version_id = _le_32(*h++); - header->header_size = _le_32(*h++); - header->sample_rate = _le_32(*h++); - header->nb_channels = _le_32(*h++); - header->frame_size = _le_32(*h++); - header->overlap = _le_32(*h++); - header->bytes_per_packet = _le_32(*h++); - header->extra_headers = _le_32(*h); - - return sizeof(*header); -} - diff --git a/libcelt/kiss_fft.c b/libcelt/kiss_fft.c index 49092314e..660b7c25d 100644 --- a/libcelt/kiss_fft.c +++ b/libcelt/kiss_fft.c @@ -616,9 +616,8 @@ kiss_fft_state *kiss_fft_alloc_twiddles(int nfft,void * mem,size_t * lenmem, co st->shift = 0; while (nfft<<st->shift != base->nfft && st->shift < 32) st->shift++; - /* FIXME: Report error and do proper cleanup */ if (st->shift>=32) - return NULL; + goto fail; } else { st->twiddles = twiddles = (kiss_twiddle_cpx*)KISS_FFT_MALLOC(sizeof(kiss_twiddle_cpx)*nfft); compute_twiddles(twiddles, nfft); @@ -627,14 +626,19 @@ kiss_fft_state *kiss_fft_alloc_twiddles(int nfft,void * mem,size_t * lenmem, co if (!kf_factor(nfft,st->factors)) { kiss_fft_free(st); - return NULL; + goto fail; } /* bitrev */ st->bitrev = bitrev = (celt_int16*)KISS_FFT_MALLOC(sizeof(celt_int16)*nfft); + if (st->bitrev==NULL) + goto fail; compute_bitrev_table(0, bitrev, 1,1, st->factors,st); } return st; +fail: + kiss_fft_free(st); + return NULL; } kiss_fft_state *kiss_fft_alloc(int nfft,void * mem,size_t * lenmem ) diff --git a/libcelt/mdct.c b/libcelt/mdct.c index 024248970..b67a31bcd 100644 --- a/libcelt/mdct.c +++ b/libcelt/mdct.c @@ -59,7 +59,7 @@ #ifdef CUSTOM_MODES -void clt_mdct_init(mdct_lookup *l,int N, int maxshift) +int clt_mdct_init(mdct_lookup *l,int N, int maxshift) { int i; int N4, N2; @@ -76,12 +76,12 @@ void clt_mdct_init(mdct_lookup *l,int N, int maxshift) l->kfft[i] = kiss_fft_alloc_twiddles(N>>2>>i, 0, 0, l->kfft[0]); #ifndef ENABLE_TI_DSPLIB55 if (l->kfft[i]==NULL) - return; + return 0; #endif } l->trig = trig = (kiss_twiddle_scalar*)celt_alloc((N4+1)*sizeof(kiss_twiddle_scalar)); if (l->trig==NULL) - return; + return 0; /* We have enough points that sine isn't necessary */ #if defined(FIXED_POINT) for (i=0;i<=N4;i++) @@ -90,6 +90,7 @@ void clt_mdct_init(mdct_lookup *l,int N, int maxshift) for (i=0;i<=N4;i++) trig[i] = (kiss_twiddle_scalar)cos(2*M_PI*i/N); #endif + return 1; } void clt_mdct_clear(mdct_lookup *l) diff --git a/libcelt/mdct.h b/libcelt/mdct.h index 752c194d6..226e1db1d 100644 --- a/libcelt/mdct.h +++ b/libcelt/mdct.h @@ -52,7 +52,7 @@ typedef struct { const kiss_twiddle_scalar * restrict trig; } mdct_lookup; -void clt_mdct_init(mdct_lookup *l,int N, int maxshift); +int clt_mdct_init(mdct_lookup *l,int N, int maxshift); void clt_mdct_clear(mdct_lookup *l); /** Compute a forward MDCT and scale by 4/N */ diff --git a/libcelt/modes.c b/libcelt/modes.c index 968bc2ff3..0ba8d16c4 100644 --- a/libcelt/modes.c +++ b/libcelt/modes.c @@ -381,19 +381,15 @@ CELTMode *celt_mode_create(celt_int32 Fs, int frame_size, int *error) compute_pulse_cache(mode, mode->maxLM); - clt_mdct_init(&mode->mdct, 2*mode->shortMdctSize*mode->nbShortMdcts, mode->maxLM); - if ((mode->mdct.trig==NULL) -#ifndef ENABLE_TI_DSPLIB55 - || (mode->mdct.kfft==NULL) -#endif - ) + if (clt_mdct_init(&mode->mdct, 2*mode->shortMdctSize*mode->nbShortMdcts, + mode->maxLM) == 0) goto failure; if (error) *error = CELT_OK; return mode; -failure: +failure: if (error) *error = CELT_ALLOC_FAIL; if (mode!=NULL) -- GitLab