On 13/03/2017 19:24, Uoti Urpala wrote: > On Sun, 2017-03-12 at 17:27 +0100, Luca Barbato wrote: >> The field is expressed in 6 bits so up to 63 bits amplitudes could be >> encoded. >> >> Reported and debugged by Uoti Urpala. > > But this patch doesn't actually match what was said on IRC... > >> - unsigned amplitude, book_idx; >> + uint64_t amplitude; > > I suggested changing the type of "amplitude" to double; it seems > pointless to make the variable uint64_t and then cast it to double in > the only use of the value other than comparison against 0.
We actually read uint64_t so I preferred cast later, shouldn't make any difference I guess. >> (((1 << vf->amplitude_bits) - 1) * sqrt(p + q))) >> - vf->amplitude_offset) * .11512925f); > > You're missing the change to 1ULL - the shift will overflow a 32-bit > int if amplitude_bits is >= 32. Thank you for reminding me! > While fixing that, you might as well remove the "f" suffix from the > constant - while it likely does no detectable harm, explicitly forcing > the constant to single precision seems entirely pointless here. Sure. Thank you! lu _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
