[FFmpeg-devel] [PATCH] Multi-Channel Correlation in ALS

[Michael Niedermayer]

On Mon, Jan 04, 2010 at 03:25:06PM +0100, Thilo Borgmann wrote:
> 
> >Index: libavcodec/alsdec.c
> >===================================================================
> >--- libavcodec/alsdec.c	(Revision 21012)
> >+++ libavcodec/alsdec.c	(Arbeitskopie)
> >...
> >+                                           num_buffers);
> >+        ctx->chan_data         = av_malloc(sizeof(ALSChannelData) *
> >+                                           num_buffers);
> >+        ctx->reverted_channels =
> >av_malloc(sizeof(*ctx->reverted_channels) *
> >+                                           num_buffers);
> >+
> >+        if (!ctx->chan_data_buffer || !ctx->chan_data) {
> >+            av_log(avctx, AV_LOG_ERROR, "Allocating buffer memory
> >failed.\n");
> >+            decode_end(avctx);
> >+            return AVERROR(ENOMEM);
> >+        }
> 
> Missed to include the check for the new reverted_channels field. Patch
> updated to revision 4.
> 
> -Thilo

i said its ok but it seems i found a few more minor things

[...]
> @@ -%ld,%ld +%ld,%ld @@
>  }
>  
>  
> +/** Reads the channel data.
> +  */
> +static int read_channel_data(ALSDecContext *ctx, ALSChannelData *cd, int c)
> +{
> +    GetBitContext *gb       = &ctx->gb;
> +    ALSChannelData *current = cd;
> +    unsigned int channels   = ctx->avctx->channels;
> +    int entries             = 0;
> +
> +    while (entries < channels && !(current->stop_flag = get_bits1(gb))) {
> +        current->master_channel = get_bits_long(gb, av_ceil_log2(channels));
> +
> +        if (current->master_channel >= channels) {
> +            av_log(ctx->avctx, AV_LOG_ERROR, "Invalid master channel!\n");
> +            return -1;
> +        }
> +
> +        if (current->master_channel != c) {
> +            current->time_diff_flag = get_bits1(gb);
> +            current->weighting[0]   = mcc_weightings[av_clip(decode_rice(gb, 1) + 16, 0, 32)];
> +            current->weighting[1]   = mcc_weightings[av_clip(decode_rice(gb, 2) + 14, 0, 32)];
> +            current->weighting[2]   = mcc_weightings[av_clip(decode_rice(gb, 1) + 16, 0, 32)];
> +
> +            if (current->time_diff_flag) {
> +                current->weighting[3] = mcc_weightings[av_clip(decode_rice(gb, 1) + 16, 0, 32)];
> +                current->weighting[4] = mcc_weightings[av_clip(decode_rice(gb, 1) + 16, 0, 32)];
> +                current->weighting[5] = mcc_weightings[av_clip(decode_rice(gb, 1) + 16, 0, 32)];
> +
> +                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;

unneeded, you can flip the sign in the code that uses it

[...]

> +    if (reverted[c])
> +        return 0;
> +
> +    reverted[c] = 1;
> +
> +    while (dep < channels && !ch[dep].stop_flag) {
> +        if (!reverted[ch[dep].master_channel])
> +            revert_channel_correlation(ctx, bd, cd, reverted, offset,
> +                                       ch[dep].master_channel);
> +
> +        dep++;
> +    }

one of the 2 reverted checks looks redundant


[...]
> @@ -%ld,%ld +%ld,%ld @@
>      av_freep(&ctx->ltp_gain_buffer);
>      av_freep(&ctx->quant_cof);
>      av_freep(&ctx->lpc_cof);
> +    av_freep(&ctx->quant_cof_buffer);
> +    av_freep(&ctx->lpc_cof_buffer);
>      av_freep(&ctx->prev_raw_samples);
>      av_freep(&ctx->raw_samples);
>      av_freep(&ctx->raw_buffer);
>  

> +    if (ctx->sconf.mc_coding) {
> +        av_freep(&ctx->chan_data);
> +        av_freep(&ctx->chan_data_buffer);
> +        av_freep(&ctx->reverted_channels);
> +    }

the if() appears to be superflous

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- 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/20100104/6cd9d42c/attachment.pgp>