Addressing all the FIXMEs in the code

Includes better error handling in fft/mdct init

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;

libcelt/header.c

@@ -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);
-}
-

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 )

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)

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 */

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 (clt_mdct_init(&mode->mdct, 2*mode->shortMdctSize*mode->nbShortMdcts,
-   if ((mode->mdct.trig==NULL)
+           mode->maxLM) == 0)
-#ifndef ENABLE_TI_DSPLIB55
-         || (mode->mdct.kfft==NULL)
-#endif
-   )
       goto failure;
 
    if (error)
       *error = CELT_OK;
 
    return mode;
-failure: 
+failure:
    if (error)
       *error = CELT_ALLOC_FAIL;
    if (mode!=NULL)