On 16/03/15 5:00 AM, Niels Möller wrote: > James Almer <[email protected]> writes: > >> Valgrind is complaining about this code ("Conditional jump or move >> depends on uninitialised >> value" error), as seen here >> https://fate.libav.org/x86_64-linux-gcc-valgrind/20150316044429 >> >> Zero initializing the param_state[16] struct from >> ff_dca_xll_decode_audio() with { { 0 } } >> "fixes" it, but it's possible it may instead be hiding the real bug in the >> code. > > If I read the code correctly, it looks like params->pancABIT0 is read > from the stream for the first segment (seg == 0) only, and used for > decoding params->nSamplePart0 samples. And that the latter value ought > to be always zero when seg != 0. > > The logic is a bit complex, and since it many months since I wrote that > code, I don't quite remember how it is supposed to work... But I suspect > the problem is that the value, which is a loop invariant, is read and > tested up-front, even in the case that the loop "using" it runs for zero > iterations. > > Can you test if the below patch solves the problem? It reads > params->pancABIT0 only when it's going to be used. > > Regards, > /Niels > > diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c > index 0c32d6e..5a558b8 100644 > --- a/libavcodec/dca_xll.c > +++ b/libavcodec/dca_xll.c > @@ -514,8 +514,8 @@ int ff_dca_xll_decode_audio(DCAContext *s, AVFrame *frame) > } > for (i = 0; i < chset->channels; i++) { > int param_index = params->seg_type ? 0 : i; > - int bits = params->pancABIT0[param_index]; > int part0 = params->nSamplPart0[param_index]; > + int bits = part0 ? params->pancABIT0[param_index] : 0; > int *sample_buf = s->xll_sample_buf + > (in_channel + i) * s->xll_smpl_in_seg;
Yes, it fixes it on my end. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
