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

[Thilo Borgmann]

Am 21.12.09 13:24, schrieb 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.

ping