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.

>                            (((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.

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.

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to