[vlc-devel] [PATCH] demux/asf: fix 17579: prevent signed integer overflow

[Filip Roséen]

Hi Remi,

On 2016-11-01 15:00, Rémi Denis-Courmont wrote:

> Le tiistaina 1. marraskuuta 2016, 2.04.06 EET Filip Roséen a écrit :
> > The previous implementation could overflow the mtime_t when
> > multiplying p_sys->p_fp->i_preroll by a thousand when converting from
> > the asf time unit (milliseconds) to VLCs (microseconds).
> > 
> > Given that you can always divide a value without running into issues
> > in terms of under/overflow, these changes prevent any overflow error
> > while still preserving the same logic.
> > 
> > In short the implementation takes advantage of the below two
> > conditions being equivalent:
> > 
> >    1: A > ( ( B * C ) + D )
> >    2: ( ( A - D ) / C ) > B
> 
> That´s true with reals, not with integers. For instance,
> with A=2, B=0, C=2, D=1:
> 
> (1) <=> 2 > ((0 * 2) + 1 <=> 2 > 1 <=> true
> (2) <=> ((2 - 1) / 2) > 0 <=> (1 / 2) > 0 <=> 0 > 0 <=> false
> 
> (I don´t know if it affects the correctness of the diff.)

That's a valid point, and I should have written a constraint on the
values of `A`, `B`, `C`.

As far as I can tell the patch should have the same behavior as the
previous implementation, though without suffering from a potential
integer overflow. Basically all the implementation does is converting
from one time unit to another, and changing from multiplying on the
right-hand side to dividing on the left-hand side should not make a
difference; both methods suffer from loss of precision in one way or
another.

Thanks,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20161101/5afe7c13/attachment.html>