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

[Thilo Borgmann]

>>>
>>> also dont forget to test the code with a fuzzer to make sure it doesnt
>>> crash and please also go over it to make sure there are no other buffer
>>> overflows, you know the code better then i do so i might not spot all.
>>
> 
>> I played around with the trasher a lot and got some pitfalls removed. I
>> end up in demuxer segfaults for my trashed files.
> 
> please report segfaults on roundup unless they are already reported

My gdb skills are worse than poor, so a stripped ffmpeg pointed me to a
ff_ac3_ function... the unstripped ffmpeg_g told me it was my fault.

I've found and debugged the segfault caused by an infinite recursion.
Now all my trash files fail without crashing ffmpeg.

> 
> 
> [...]
>> @@ -%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;
>> +            }
>> +        }
>> +
>> +        current++;
>> +        entries++;
>> +    }
>> +
>> +    if (entries == channels) {
>> +        current--;
>> +        current->stop_flag = 1;
>> +        current->master_channel = c;
>> +        return -1;
>> +    }
> 
> there are 2 return -1 in this function but only one sets these things like
> stop_flag, why is it needed in one but not the other ?

Indeed these are unnecessary now.

> 
> 
> [...]
>> @@ -%ld,%ld +%ld,%ld @@
> 
> whatever generated this patch is not too well working

Known issue in MacPorts's svn >= 1.6.5 :(
As long as I commit these myself I can ignore it until they can solve it...


Updated patch attached.

-Thilo
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: als_mcc.rev2.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091221/f31b52ab/attachment.asc>