[FFmpeg-devel] [PATCH] ALS decoder

Michael Niedermayer michaelni
Wed Sep 2 16:10:28 CEST 2009


On Mon, Aug 31, 2009 at 01:24:02AM +0200, Thilo Borgmann wrote:
> Revision 14 attached.
[...]

> +typedef struct {
> +    AVCodecContext *avctx;
> +    ALSSpecificConfig sconf;

> +    GetBitContext gb;          ///< a bitreader context

That comment is not usefull


> +    unsigned int num_frames;   ///< number of frames to decode, 0 if unknown
> +    unsigned int cur_frame_length;  ///< length of the current frame to decode
> +    unsigned int last_frame_length; ///< length of the last frame to decode, 0 if unknown

> +    unsigned int frame_id;     ///< the frame ID / number of the current frame

this should be reset when seeking (flush() being called)
otherwise a random frame in the middle will be treated like the last


[...]
> +/** Reads an ALSSpecificConfig from a buffer into the output struct.
> + */
> +static av_cold int read_specific_config(ALSDecContext *ctx)
> +{
> +    GetBitContext gb;
> +    uint64_t ht_size;
> +    int i, config_offset, crc_enabled;
> +    MPEG4AudioConfig m4ac;
> +    ALSSpecificConfig *sconf = &ctx->sconf;
> +    AVCodecContext *avctx    = ctx->avctx;
> +    const uint8_t *buffer    = avctx->extradata;
> +    uint32_t samples, als_id;
> +
> +    init_get_bits(&gb, buffer, avctx->extradata_size * 8);
> +
> +    config_offset = ff_mpeg4audio_get_config(&m4ac, buffer, avctx->extradata_size);
> +
> +    if (config_offset < 0)
> +        return -1;
> +
> +    skip_bits_long(&gb, config_offset);
> +
> +    if (get_bits_left(&gb) < (30 << 3))
> +        return -1;
> +
> +    // read the fixed items
> +    als_id                      = get_bits_long(&gb, 32);
> +    avctx->sample_rate          = m4ac.sample_rate;
> +    skip_bits_long(&gb, 32); // sample rate already known
> +    samples                     = get_bits_long(&gb, 32);
> +    avctx->channels             = m4ac.channels;
> +    skip_bits(&gb, 16);      // number of channels already knwon
> +    skip_bits(&gb, 3);       // skip file_type
> +    sconf->resolution           = get_bits(&gb, 3);
> +    sconf->floating             = get_bits1(&gb);
> +    skip_bits1(&gb);         // skip msb_first
> +    sconf->frame_length         = get_bits(&gb, 16) + 1;
> +    sconf->ra_distance          = get_bits(&gb, 8);
> +    sconf->ra_flag              = get_bits(&gb, 2);
> +    sconf->adapt_order          = get_bits1(&gb);
> +    sconf->coef_table           = get_bits(&gb, 2);
> +    sconf->long_term_prediction = get_bits1(&gb);
> +    sconf->max_order            = get_bits(&gb, 10);
> +    sconf->block_switching      = get_bits(&gb, 2);
> +    sconf->bgmc                 = get_bits1(&gb);
> +    sconf->sb_part              = get_bits1(&gb);
> +    sconf->joint_stereo         = get_bits1(&gb);
> +    sconf->mc_coding            = get_bits1(&gb);
> +    sconf->chan_config          = get_bits1(&gb);
> +    sconf->chan_sort            = get_bits1(&gb);
> +    crc_enabled                 = get_bits1(&gb);
> +    sconf->rlslms               = get_bits1(&gb);
> +    skip_bits(&gb, 5);       // skip 5 reserved bits
> +    skip_bits1(&gb);         // skip aux_data_enabled
> +
> +
> +    // check for ALSSpecificConfig struct
> +    if (als_id != MKBETAG('A','L','S','\0'))
> +        return -1;
> +
> +    ctx->cur_frame_length = sconf->frame_length;
> +
> +    // allocate quantized parcor coefficient buffer
> +    if (!(ctx->quant_cof = av_malloc(sizeof(*ctx->quant_cof) * sconf->max_order)) ||
> +        !(ctx->lpc_cof   = av_malloc(sizeof(*ctx->lpc_cof)   * sconf->max_order))) {
> +        av_log(avctx, AV_LOG_ERROR, "Allocating buffer memory failed.\n");
> +        return AVERROR(ENOMEM);
> +    }
> +    // calculate total number of frames to decode if possible
> +    if (samples != 0xFFFFFFFF) {
> +        ctx->num_frames        = (samples - 1) / sconf->frame_length + 1;
> +        ctx->last_frame_length = (samples - 1) % sconf->frame_length + 1;
> +    } else {
> +        ctx->num_frames        = 0;
> +        ctx->last_frame_length = 0;
> +    }
> +
> +    // read channel config
> +    if (sconf->chan_config)
> +        sconf->chan_config_info = get_bits(&gb, 16);
> +    // TODO: use this to set avctx->channel_layout
> +
> +

> +    // read channel sorting
> +    if (sconf->chan_sort && avctx->channels > 1) {
> +        int chan_pos_bits = av_ceil_log2(avctx->channels);
> +        int bits_needed  = avctx->channels * chan_pos_bits + 7;
> +        if (get_bits_left(&gb) < bits_needed)
> +            return -1;
> +
> +        if (!(sconf->chan_pos = av_malloc(avctx->channels * sizeof(*sconf->chan_pos))))
> +            return AVERROR(ENOMEM);
> +
> +        for (i = 0; i < avctx->channels; i++)
> +            sconf->chan_pos[i] = get_bits(&gb, chan_pos_bits);
> +
> +        align_get_bits(&gb);
> +        // TODO: use this to actually do channel sorting

I think outputting channels in the correct order is quite important
i dont mind if thats done after the decoder is in svn, but i would mind
if that is not done within a year ...


[...]
> +/** Converts PARCOR coefficient k to direct filter coefficient.
> + */
> +static int parcor_to_lpc(unsigned int k, const int32_t *par, int32_t *cof)
> +{
> +    int i, j;

> +    int64_t tmp1;

int should work as well
also the delaration and init can be merged


> +
> +    for (i = 0, j = k - 1; i < j; i++, j--) {
> +        tmp1    = ((MUL64(par[k], cof[j]) + (1 << 19)) >> 20);
> +        cof[j] += ((MUL64(par[k], cof[i]) + (1 << 19)) >> 20);
> +        cof[i] += tmp1;
> +    }
> +    if (i == j) cof[i] += ((MUL64(par[k], cof[j]) + (1 << 19)) >> 20);
> +
> +    cof[k] = par[k];
> +    return -1;
> +}

that seems to be the wrong return value


[...]
> +/** Reads the block data for a non-constant block
> + */
> +static int read_var_block(ALSDecContext *ctx, unsigned int ra_block,
> +                          int32_t *raw_samples, unsigned int block_length,
> +                          unsigned int *js_blocks, int32_t *raw_other,
> +                          unsigned int *shift_lsbs)
> +{
> +    ALSSpecificConfig *sconf = &ctx->sconf;
> +    AVCodecContext *avctx    = ctx->avctx;
> +    GetBitContext *gb        = &ctx->gb;
> +    unsigned int k;
> +    unsigned int s[8];
> +    unsigned int sub_blocks, sb_length;
> +    unsigned int opt_order  = 1;
> +    int32_t      *quant_cof = ctx->quant_cof;
> +    int32_t      *lpc_cof   = ctx->lpc_cof;

> +    unsigned int start      = 0;

redundant init


> +    int          sb, smp;
> +    int64_t      y;
> +
> +    *js_blocks  = get_bits1(gb);
> +
> +    // determine the number of sub blocks for entropy decoding
> +    if (!sconf->bgmc && !sconf->sb_part)
> +        sub_blocks = 1;
> +    else if (sconf->bgmc && sconf->sb_part)
> +        sub_blocks = 1 << get_bits(gb, 2);
> +    else
> +        sub_blocks = 1 << (2 * get_bits1(gb));
> +
> +    // do not continue in case of a damaged stream since
> +    // block_length must be evenly divisible by sub_blocks
> +    if (block_length % sub_blocks) {
> +        av_log(avctx, AV_LOG_WARNING,
> +               "Block length is not evenly divisible by the number of sub blocks.\n");
> +        return -1;
> +    }
> +
> +    sb_length = block_length / sub_blocks;
> +
> +
> +    if (sconf->bgmc) {
> +        // TODO: BGMC mode
> +    } else {
> +        s[0] = get_bits(gb, 4 + (sconf->resolution > 1));
> +        for (k = 1; k < sub_blocks; k++)
> +            s[k] = s[k - 1] + decode_rice(gb, 0);
> +    }
> +
> +    if (get_bits1(gb))
> +        *shift_lsbs = get_bits(gb, 4) + 1;
> +
> +
> +    if (!sconf->rlslms) {
> +        if (sconf->adapt_order) {
> +            int opt_order_length = FFMAX(av_ceil_log2((block_length >> 3) - 1), 1);
> +            opt_order_length     = FFMIN(av_ceil_log2(sconf->max_order+1), opt_order_length);
> +            opt_order            = get_bits(gb, opt_order_length);
> +        } else {
> +            opt_order = sconf->max_order;
> +        }
> +
> +        if (opt_order) {
> +            int add_base;
> +
> +            if (sconf->coef_table == 3) {
> +                add_base = 0x7F;
> +
> +                // read coefficient 0
> +                quant_cof[0] = parcor_scaled_values[get_bits(gb, 7)];
> +
> +                // read coefficient 1
> +                quant_cof[1] = -parcor_scaled_values[get_bits(gb, 7)];
> +
> +                // read coefficients 2 to opt_order
> +                for (k = 2; k < opt_order; k++)
> +                    quant_cof[k] = get_bits(gb, 7);
> +            } else {
> +                int k_max;
> +                add_base = 1;
> +
> +                // read coefficient 0 to 19
> +                k_max = FFMIN(opt_order, 20);
> +                for (k = 0; k < k_max; k++) {
> +                    int rice_param = parcor_rice_table[sconf->coef_table][k][1];
> +                    int offset     = parcor_rice_table[sconf->coef_table][k][0];
> +                    quant_cof[k] = decode_rice(gb, rice_param) + offset;
> +                }
> +
> +                // read coefficients 20 to 126
> +                k_max = FFMIN(opt_order, 127);
> +                for (; k < k_max; k++)
> +                    quant_cof[k] = decode_rice(gb, 2) + (k & 1);
> +
> +                // read coefficients 127 to opt_order
> +                for (; k < opt_order; k++)
> +                    quant_cof[k] = decode_rice(gb, 1);
> +
> +                quant_cof[0] =  parcor_scaled_values[quant_cof[0] + 64];
> +                quant_cof[1] = -parcor_scaled_values[quant_cof[1] + 64];
> +            }
> +
> +        for (k = 2; k < opt_order; k++)
> +            quant_cof[k] = (quant_cof[k] << 14) + (add_base << 13);
> +        }
> +    }
> +
> +    // TODO: LTP mode
> +
> +    start = 0;
> +
> +    // read first value and residuals in case of a random access block
> +    if (ra_block) {
> +        if (opt_order)
> +            raw_samples[0] = decode_rice(gb, avctx->bits_per_raw_sample - 4);
> +        if (opt_order > 1)
> +            raw_samples[1] = decode_rice(gb, s[0] + 3);
> +        if (opt_order > 2)
> +            raw_samples[2] = decode_rice(gb, s[0] + 1);
> +
> +        start = FFMIN(opt_order, 3);
> +    } else {

> +        for (k = 0; k < opt_order; k++)
> +            parcor_to_lpc(k, quant_cof, lpc_cof);

i think this can be moved closer to where the parcor_to_lpc()
of the ra_block case is


[...]
> +/** Computes the bytes left to decode for the current frame
> + */
> +static void zero_remaining(unsigned int b, unsigned int b_max,
> +                           const unsigned int *div_blocks, int32_t *buf)

the doxy comment is slightly incomplete


[...]
> +/** Decodes blocks dependently.
> + */
> +static int decode_blocks(ALSDecContext *ctx, unsigned int ra_frame,
> +                         unsigned int c, const unsigned int *div_blocks,
> +                         unsigned int *js_blocks)
> +{
> +    ALSSpecificConfig *sconf = &ctx->sconf;
> +    unsigned int offset = 0;
> +    int32_t *raw_samples_R;
> +    int32_t *raw_samples_L;
> +    unsigned int b;
> +
> +    // decode all blocks
> +    for (b = 0; b < ctx->num_blocks; b++) {
> +        unsigned int s;
> +        raw_samples_L = ctx->raw_samples[c    ] + offset;
> +        raw_samples_R = ctx->raw_samples[c + 1] + offset;
> +        if (read_block_data(ctx, ra_frame, raw_samples_L, div_blocks[b],
> +                            &js_blocks[0], raw_samples_R) ||
> +            read_block_data(ctx, ra_frame, raw_samples_R, div_blocks[b],
> +                            &js_blocks[1], raw_samples_L)) {
> +            // damaged block, write zero for the rest of the frame
> +            zero_remaining(b, ctx->num_blocks, div_blocks, raw_samples_L);
> +            zero_remaining(b, ctx->num_blocks, div_blocks, raw_samples_R);
> +            return -1;
> +        }

This code looks strange, the first call would use raw_samples_R
that does not seem initialized at that point

have all the js cases been tested?


[...]
> +/** Decodes an ALS frame.
> + */
> +static int decode_frame(AVCodecContext *avctx,
> +                        void *data, int *data_size,
> +                        AVPacket *avpkt)
> +{
> +    ALSDecContext *ctx       = avctx->priv_data;
> +    ALSSpecificConfig *sconf = &ctx->sconf;
> +    const uint8_t *buffer    = avpkt->data;
> +    int buffer_size          = avpkt->size;

> +    int invalid_frame        = 0;

redudnant init


> +    unsigned int c, sample, ra_frame, bytes_read, shift;
> +
> +    init_get_bits(&ctx->gb, buffer, buffer_size * 8);
> +    ra_frame = sconf->ra_distance && !(ctx->frame_id % sconf->ra_distance);

has the ra_distance==0 case been tested?


> +
> +    // the last frame to decode might have a different length
> +    if (ctx->num_frames && ctx->num_frames - 1 == ctx->frame_id)
> +        ctx->cur_frame_length = ctx->last_frame_length;
> +
> +    // decode the frame data
> +    if ((invalid_frame = read_frame_data(ctx, ra_frame)))

all errors should be negative values, thus this should be a <0 check


> +        av_log(ctx->avctx, AV_LOG_WARNING,
> +               "Reading frame data failed. Skipping RA unit.\n");
> +
> +    // increment the frame counter
> +    ctx->frame_id++;
> +
> +    // transform decoded frame into output format
> +    #define INTERLEAVE_OUTPUT(bps)                                 \
> +    {                                                              \
> +        int##bps##_t *dest = (int##bps##_t*) data;                 \
> +        shift = bps - ctx->avctx->bits_per_raw_sample;             \
> +        for (sample = 0; sample < ctx->cur_frame_length; sample++) \
> +            for (c = 0; c < avctx->channels; c++)                  \
> +                *dest++ = ctx->raw_samples[c][sample] << shift;    \
> +    }
> +
> +    if (ctx->avctx->bits_per_raw_sample <= 16) {
> +        INTERLEAVE_OUTPUT(16)
> +    } else {
> +        INTERLEAVE_OUTPUT(32)
> +    }
> +
> +    *data_size = ctx->cur_frame_length * avctx->channels
> +                 * (av_get_bits_per_sample_format(avctx->sample_fmt) >> 3);

data_size is not checked before writing into the buffer


[...]
> +/** Initializes the ALS decoder.
> + */
> +static av_cold int decode_init(AVCodecContext *avctx)
> +{
> +    unsigned int c;
> +    unsigned int channel_size;
> +    int64_t data_size;
> +    ALSDecContext *ctx = avctx->priv_data;
> +    ALSSpecificConfig *sconf = &ctx->sconf;
> +    ctx->avctx = avctx;
> +
> +    if (!avctx->extradata) {
> +        av_log(avctx, AV_LOG_ERROR, "Missing required ALS extradata.\n");
> +        return -1;
> +    }
> +
> +    if (read_specific_config(ctx)) {
> +        av_log(avctx, AV_LOG_ERROR, "Reading ALSSpecificConfig failed.\n");
> +        decode_end(avctx);
> +        return -1;
> +    }
> +
> +    if (check_specific_config(ctx)) {
> +        decode_end(avctx);
> +        return -1;
> +    }
> +
> +    if (sconf->floating) {
> +        avctx->sample_fmt          = SAMPLE_FMT_FLT;
> +        avctx->bits_per_raw_sample = 32;
> +    } else {
> +        avctx->sample_fmt          = sconf->resolution > 1
> +                                     ? SAMPLE_FMT_S32 : SAMPLE_FMT_S16;
> +        avctx->bits_per_raw_sample = (sconf->resolution + 1) * 8;
> +    }
> +
> +    avctx->frame_size = sconf->frame_length;
> +    channel_size      = sconf->frame_length + sconf->max_order;
> +

> +    // check for size of decoded data
> +    data_size = sconf->frame_length * avctx->channels *
> +                (av_get_bits_per_sample_format(avctx->sample_fmt) >> 3);
> +
> +    if (data_size > INT_MAX) {
> +        av_log(avctx, AV_LOG_ERROR, "Decoded data exceeds buffer size.\n");
> +        decode_end(avctx);
> +        return -1;
> +    }

whatever this check should do it doesnt work, it wont ever be true as the
multiplications are using int not int64

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

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- 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/20090902/a8350155/attachment.pgp>



More information about the ffmpeg-devel mailing list