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

_______________________________________________
FFmpeg-soc mailing list
FFmpeg-soc@mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc

Reply via email to