[FFmpeg-devel] [PATCH] avcodec: add native Speex decoder

[Andreas Rheinhardt]

Paul B Mahol:
> +typedef struct SpeexBMode {
> +    int frame_size; /**< Size of frames used for decoding */
> +    int subframe_size; /**< Size of sub-frames used for decoding */
> +    int lpc_size; /**< Order of LPC filter */
> +    float folding_gain; /**< Folding gain */
> +    const SpeexSubmode *submodes[NB_SUBMODES]; /**< Sub-mode data for the mode */
> +    int default_submode; /**< Default sub-mode to use when decoding */
> +} SpeexBMode;
> +
> +
> +typedef struct SpeexMode {
> +    const void *mode; /** Pointer to the low-level mode data */
> +    int modeID; /** ID of the mode */
> +    int (*decode)(AVCodecContext *avctx, DecoderState *dec, GetBitContext *gb, float *out);
> +} SpeexMode;
> +
> +
> +static const SpeexBMode narrowband_mode =
> +{
> +    .frame_size = NB_FRAME_SIZE,
> +    .subframe_size = NB_SUBFRAME_SIZE,
> +    .lpc_size = NB_ORDER,
> +    .submodes = {
> +        NULL, &nb_submode1, &nb_submode2, &nb_submode3, &nb_submode4,
> +        &nb_submode5, &nb_submode6, &nb_submode7, &nb_submode8
> +    },
> +    .default_submode = 5,
> +};
> +
> +static const SpeexBMode wideband_mode = {
> +    .frame_size = NB_FRAME_SIZE,
> +    .subframe_size = NB_SUBFRAME_SIZE,
> +    .lpc_size = 8,
> +    .folding_gain = 0.9f,
> +    .submodes = {
> +        NULL, &wb_submode1, &wb_submode2, &wb_submode3, &wb_submode4
> +    },
> +    .default_submode = 3,
> +};
> +
> +static const SpeexBMode ultrawideband_mode = {
> +    .frame_size = 320,
> +    .subframe_size = 80,
> +    .lpc_size = 8,
> +    .folding_gain = 0.7f,
> +    .submodes = {
> +        NULL, &wb_submode1
> +    },
> +    .default_submode = 1,
> +};
> +
> +
> +static const SpeexMode narrowband = {
> +    .mode = &narrowband_mode,
> +    .modeID = 0,
> +    .decode = nb_decode,
> +};
> +
> +static const SpeexMode wideband = {
> +    .mode = &wideband_mode,
> +    .modeID = 1,
> +    .decode = sb_decode,
> +};
> +
> +static const SpeexMode ultrawideband = {
> +    .mode = &ultrawideband_mode,
> +    .modeID = 2,
> +    .decode = sb_decode,
> +};

SpeexMode.mode always points to a SpeexBMode, so there is no reason for
it to be a pointer to const void; furthermore, there is a 1-1
correspondence between SpeexModes and SpeexBModes, so you can just put
the relevant SpeexBModes into SpeexModes. Or even remove SpeexBModes
completely and put its current contents directly in SpeexModes.

> +
> +static const SpeexMode *const speex_modes[SPEEX_NB_MODES] = {
> +    &narrowband,
> +    &wideband,
> +    &ultrawideband,
> +};

The pointed-to objects here have always the same type and size, so one
could put them directly into an array of SpeexModes and remove the array
of pointers.

> +static int parse_speex_extradata(AVCodecContext *avctx,
> +    const uint8_t *extradata, int extradata_size)
> +{
> +    SpeexContext *s = avctx->priv_data;
> +    const uint8_t *buf = extradata;
> +    uint8_t header[8];
> +
> +    bytestream_get_buffer(&buf, header, sizeof(header));
> +
> +    if (memcmp(header, "Speex   ", sizeof(header)))
> +        return AVERROR_INVALIDDATA;

Why the extra header buffer?

- Andreas