On 18.04.2015 21:46, Michael Niedermayer wrote: > On Sat, Apr 18, 2015 at 09:13:30PM +0200, Andreas Cadhalpun wrote: >> On 18.04.2015 20:42, Michael Niedermayer wrote: >>> On Sat, Apr 18, 2015 at 08:13:30PM +0200, Andreas Cadhalpun wrote: >>>> @@ -1290,8 +1290,16 @@ static int revert_channel_correlation(ALSDecContext >>>> *ctx, ALSBlockData *bd, >>>> >>>> if (ch[dep].time_diff_sign) { >>>> t = -t; >>>> + if (t > 0 && begin < t) { >>> >>> time_diff_index is always positive, so t is always negative here >> >> I didn't verify this, but I added the 'begin < t' check only for symmetry >> with the end case.
I verified it now: current->time_diff_index = get_bits(gb, ctx->ltp_lag_length - 3) + 3; ... ctx->ltp_lag_length = 8 + (avctx->sample_rate >= 96000) + (avctx->sample_rate >= 192000); So ltp_lag_length is at most 10, i.e. at most 7 bits are read for the time_diff_index, so it is always positive. >>> so this cant be true unless the context got corrupted or iam missing >>> something >> >> If you're sure t is always negative here, this check can be dropped. > > maybe add a av_assert0() to protect againt future code changes I don't think an assert would be good here. If you want to protect against future code changes, I would rather leave the error return. But I'd also be fine with dropping this check altogether. What do you prefer? Best regards, Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel