[FFmpeg-soc] [soc]: r5430 - in als: als_data.h alsdec.c

Justin Ruggles justin.ruggles at gmail.com
Sun Nov 8 01:54:49 CET 2009


thilo.borgmann wrote:

> Author: thilo.borgmann
> Date: Sat Nov  7 23:58:55 2009
> New Revision: 5430
> 
> Log:
> Add Multi-Channel Correlation decoding.
> Introduce ALSChannelData.
> 
> Modified:
>    als/als_data.h
>    als/alsdec.c
> 
> Modified: als/als_data.h
> ==============================================================================
> --- als/als_data.h	Sat Nov  7 23:51:02 2009	(r5429)
> +++ als/als_data.h	Sat Nov  7 23:58:55 2009	(r5430)
> @@ -99,4 +99,14 @@ static const uint8_t ltp_gain_values [] 
>      0, 8, 16, 24, 32, 40, 48, 56, 64, 70, 76, 82, 88, 92, 96, 100
>  };
>  
> +
> +/** Inter-channel weighting factors for multi-channel correlation.
> + *  To be indexed by the Rice coded indices.
> + */
> +static const int mcc_weightings[] = {
> +    204,  192,  179,  166,  153,  140,  128,  115,
> +    102,   89,   76,   64,   51,   38,   25,   12,
> +      0,  -12,  -25,  -38,  -51,  -64,  -76,  -89,
> +   -102, -115, -128, -140, -153, -166, -179, -192
> +};


those will fit in int16_t

>  #endif /* AVCODEC_ALS_DATA_H */
> 
> Modified: als/alsdec.c
> ==============================================================================
> --- als/alsdec.c	Sat Nov  7 23:51:02 2009	(r5429)
> +++ als/alsdec.c	Sat Nov  7 23:58:55 2009	(r5430)
> @@ -70,6 +70,16 @@ typedef struct {
>  
>  
>  typedef struct {
> +    int stop_flag;
> +    int master_channel;
> +    int time_diff_flag;
> +    int time_diff_sign;
> +    int time_diff_index;
> +    int weighting[6];
> +} ALSChannelData;
> +
> +
> +typedef struct {
>      AVCodecContext *avctx;
>      ALSSpecificConfig sconf;
>      GetBitContext gb;
> @@ -80,8 +90,12 @@ typedef struct {
>      unsigned int js_switch;         ///< if true, joint-stereo decoding is enforced
>      unsigned int num_blocks;        ///< number of blocks used in the current frame
>      int ltp_lag_length;             ///< number of bits used for ltp lag value
> -    int32_t *quant_cof;             ///< quantized parcor coefficients
> -    int32_t *lpc_cof;               ///< coefficients of the direct form prediction filter
> +    int32_t **quant_cof;            ///< quantized parcor coefficients for a channel
> +    int32_t *quant_cof_buffer;      ///< contains all quantized parcor coefficients
> +    int32_t **lpc_cof;              ///< coefficients of the direct form prediction filter for a channel
> +    int32_t *lpc_cof_buffer;        ///< contains all coefficients of the direct form prediction filter
> +    ALSChannelData **chan_data;     ///< channel data for multi-channel correlation
> +    ALSChannelData *chan_data_buffer; ///< contains channel data for all channels
>      int32_t *prev_raw_samples;      ///< contains unshifted raw samples from the previous block
>      int32_t **raw_samples;          ///< decoded raw samples for each channel
>      int32_t *raw_buffer;            ///< contains all decoded raw samples including carryover samples
> @@ -155,7 +169,7 @@ static av_cold int read_specific_config(
>  {
>      GetBitContext gb;
>      uint64_t ht_size;
> -    int i, config_offset, crc_enabled;
> +    int i, config_offset, crc_enabled, n;
>      MPEG4AudioConfig m4ac;
>      ALSSpecificConfig *sconf = &ctx->sconf;
>      AVCodecContext *avctx    = ctx->avctx;
> @@ -212,11 +226,26 @@ static av_cold int read_specific_config(
>      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))) {
> +    n = sconf->mc_coding ? avctx->channels : 1;


'n' is not a very descriptive variable name.  I'm not sure what else
would be better... num_lpc_buffers maybe?

> +    i = n * sconf->max_order;
> +
> +    ctx->quant_cof        = av_malloc(sizeof(*ctx->quant_cof) * avctx->channels);
> +    ctx->lpc_cof          = av_malloc(sizeof(*ctx->lpc_cof)   * avctx->channels);
> +    ctx->quant_cof_buffer = av_malloc(sizeof(*ctx->quant_cof_buffer) * i);
> +    ctx->lpc_cof_buffer   = av_malloc(sizeof(*ctx->lpc_cof_buffer)   * i);


using 'i' for an iterator in one place and something else in another is
confusing.  I think it is clearer to just use
  sizeof(...) * n * sconf->max_order
in this case.

> +
> +    if (!ctx->quant_cof        || !ctx->lpc_cof       ||
> +        !ctx->quant_cof_buffer || !ctx->lpc_cof_buffer) {
>          av_log(avctx, AV_LOG_ERROR, "Allocating buffer memory failed.\n");
>          return AVERROR(ENOMEM);
>      }
> +
> +    // assign quantized parcor coefficient buffers
> +    for (i = 0; i < n; i++) {
> +        ctx->quant_cof[i] = ctx->quant_cof_buffer + i * sconf->max_order;
> +        ctx->lpc_cof[i]   = ctx->lpc_cof_buffer   + i * sconf->max_order;
> +    }
> +
>      // calculate total number of frames to decode if possible
>      if (samples != 0xFFFFFFFF) {
>          ctx->num_frames        = (samples - 1) / sconf->frame_length + 1;
> @@ -312,7 +341,6 @@ static int check_specific_config(ALSDecC
>  
>      MISSING_ERR(sconf->floating,             "Floating point decoding",     -1);
>      MISSING_ERR(sconf->bgmc,                 "BGMC entropy decoding",       -1);
> -    MISSING_ERR(sconf->mc_coding,            "Multi-channel correlation",   -1);
>      MISSING_ERR(sconf->rlslms,               "Adaptive RLS-LMS prediction", -1);
>      MISSING_ERR(sconf->chan_sort,            "Channel sorting",              0);
>  
> @@ -801,8 +829,8 @@ static int decode_blocks_ind(ALSDecConte
>      memset(&bd, 0, sizeof(ALSBlockData));
>  
>      bd.ra_block         = ra_frame;
> -    bd.quant_cof        = ctx->quant_cof;
> -    bd.lpc_cof          = ctx->lpc_cof;
> +    bd.quant_cof        = ctx->quant_cof[0];
> +    bd.lpc_cof          = ctx->lpc_cof[0];
>      bd.prev_raw_samples = ctx->prev_raw_samples;
>      bd.raw_samples      = ctx->raw_samples[c];
>  
> @@ -841,14 +869,14 @@ static int decode_blocks(ALSDecContext *
>      memset(bd, 0, 2 * sizeof(ALSBlockData));
>  
>      bd[0].ra_block         = ra_frame;
> -    bd[0].quant_cof        = ctx->quant_cof;
> -    bd[0].lpc_cof          = ctx->lpc_cof;
> +    bd[0].quant_cof        = ctx->quant_cof[0];
> +    bd[0].lpc_cof          = ctx->lpc_cof[0];
>      bd[0].prev_raw_samples = ctx->prev_raw_samples;
>      bd[0].js_blocks        = *js_blocks;
>  
>      bd[1].ra_block         = ra_frame;
> -    bd[1].quant_cof        = ctx->quant_cof;
> -    bd[1].lpc_cof          = ctx->lpc_cof;
> +    bd[1].quant_cof        = ctx->quant_cof[0];
> +    bd[1].lpc_cof          = ctx->lpc_cof[0];
>      bd[1].prev_raw_samples = ctx->prev_raw_samples;
>      bd[1].js_blocks        = *(js_blocks + 1);
>  
> @@ -903,6 +931,108 @@ static int decode_blocks(ALSDecContext *
>  }
>  
>  
> +/** Reads the channel data
> +  */
> +static void read_channel_data(ALSDecContext *ctx, ALSChannelData *cd, int c)  // replace "current" by "cd"


why two comments to document one function?

> +{
> +    GetBitContext *gb       = &ctx->gb;
> +    ALSChannelData *current = cd;
> +
> +    while (!(current->stop_flag = get_bits1(gb))) {
> +        current->master_channel = get_bits_long(gb, av_ceil_log2(ctx->avctx->channels));
> +
> +        if (current->master_channel != c) {
> +            current->time_diff_flag = get_bits1(gb);
> +            current->weighting[0]   = mcc_weightings[decode_rice(gb, 1) + 16];
> +            current->weighting[1]   = mcc_weightings[decode_rice(gb, 2) + 14];
> +            current->weighting[2]   = mcc_weightings[decode_rice(gb, 1) + 16];
> +
> +            if (current->time_diff_flag) {
> +                current->weighting[3] = mcc_weightings[decode_rice(gb, 1) + 16];
> +                current->weighting[4] = mcc_weightings[decode_rice(gb, 1) + 16];
> +                current->weighting[5] = mcc_weightings[decode_rice(gb, 1) + 16];


Reading values from the stream to access an array without bounds
checking or clipping.

> +
> +                current->time_diff_sign  = get_bits1(gb);
> +                current->time_diff_index = get_bits(gb, ctx->ltp_lag_length - 3) + 3;
> +                if (current->time_diff_sign)
> +                    current->time_diff_index = -current->time_diff_index;
> +            }
> +        }
> +
> +        current++;
> +    }
> +
> +    align_get_bits(gb);
> +}
> +
> +
> +static void revert_channel_correlation(ALSDecContext *ctx, ALSBlockData *bd,
> +                                       ALSChannelData **cd, int *reverted,
> +                                       unsigned int offset, int c)


This reverting is pretty complex, much worse than the other reverting
for joint stereo and bit shift.  If it is possible, it might be worth
just storing the original data, then restoring it instead of manually
reversing the process.

[...]

>  /** Reads the frame data.
>   */
>  static int read_frame_data(ALSDecContext *ctx, unsigned int ra_frame)
> @@ -911,7 +1041,7 @@ static int read_frame_data(ALSDecContext
>      AVCodecContext *avctx    = ctx->avctx;
>      GetBitContext *gb = &ctx->gb;
>      unsigned int div_blocks[32];                ///< block sizes.
> -    unsigned int c;
> +    unsigned int b, c;
>      unsigned int js_blocks[2];
>  
>      uint32_t bs_info = 0;
> @@ -963,13 +1093,53 @@ static int read_frame_data(ALSDecContext
>                  sizeof(*ctx->raw_samples[c]) * sconf->max_order);
>          }
>      } else { // multi-channel coding
> +        ALSBlockData   bd;
> +        int            reverted_channels[avctx->channels];
> +        unsigned int   offset = 0;
> +
> +        memset(&bd,           0, sizeof(ALSBlockData));
> +        memset(&reverted_channels, 0, sizeof(int) * avctx->channels);


sizeof(reverted_channels)

> +
> +        bd.ra_block         = ra_frame;
> +        bd.prev_raw_samples = ctx->prev_raw_samples;
> +
>          get_block_sizes(ctx, div_blocks, &bs_info);
>  
> -        // TODO: multi channel coding might use a temporary buffer instead as
> -        //       the actual channel is not known when read_block-data is called
> -        if (decode_blocks_ind(ctx, ra_frame, 0, div_blocks, js_blocks))
> -            return -1;
> -        // TODO: read_channel_data
> +        for (b = 0; b < ctx->num_blocks; b++) {
> +            bd.shift_lsbs   = 0;
> +            bd.block_length = div_blocks[b];
> +
> +            for (c = 0; c < avctx->channels; c++) {
> +                bd.lpc_cof     = ctx->lpc_cof[c];
> +                bd.quant_cof   = ctx->quant_cof[c];
> +                bd.raw_samples = ctx->raw_samples[c] + offset;
> +                bd.raw_other   = NULL;
> +
> +                read_block(ctx, &bd);
> +                read_channel_data(ctx, ctx->chan_data[c], c);
> +            }
> +
> +            for (c = 0; c < avctx->channels; c++)
> +                revert_channel_correlation(ctx, &bd, ctx->chan_data,
> +                                           reverted_channels, offset, c);
> +
> +            for (c = 0; c < avctx->channels; c++) {
> +                bd.lpc_cof     = ctx->lpc_cof[c];
> +                bd.quant_cof   = ctx->quant_cof[c];
> +                bd.raw_samples = ctx->raw_samples[c] + offset;
> +                decode_block(ctx, &bd);
> +            }
> +
> +            memset(&reverted_channels, 0, avctx->channels * sizeof(int));


sizeof(reverted_channels)


Great job getting this implemented!  That completes the implementation
of everything in the proposed ALS Simple Profile except for channel
sorting, correct?

-Justin



More information about the FFmpeg-soc mailing list