On Sat, Apr 18, 2015 at 10:28:53PM +0200, Andreas Cadhalpun wrote: > 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?
whatever you prefer, the important part is to add checks where they are needed [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel