> > On Mon, 12 Dec 2016, Frediano Ziglio wrote: > > > The partial expression "MSEC_PER_SEC * size * 8" can overflow if > > size is 536870 or more. This as the operation is done using > > 32 bit unsigned integers. Being the size potentially double of > > a compressed frame size the limit can be easily reached. > > As get_average_frame_size already return a 64 bit use 64 bit > > even for the size to avoid the integer overflow. > > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > --- > > server/gstreamer-encoder.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c > > index e28ab00..988d193 100644 > > --- a/server/gstreamer-encoder.c > > +++ b/server/gstreamer-encoder.c > > @@ -511,7 +511,7 @@ static uint32_t get_min_playback_delay(SpiceGstEncoder > > *encoder) > > * an I frame) and an average frame. This also takes into account the > > * frames dropped by the encoder bit rate control. > > */ > > - uint32_t size = get_maximum_frame_size(encoder) + > > get_average_frame_size(encoder); > > + uint64_t size = get_maximum_frame_size(encoder) + > > get_average_frame_size(encoder); > > uint32_t send_time = MSEC_PER_SEC * size * 8 / encoder->bit_rate; > > > > /* Also factor in the network latency with a margin for jitter. */ > > > > 536870 bytes seems really large for a single frame but being prepared > for that case makes sense anyway. > > Acked-by: Francois Gouget <fgouget@xxxxxxxxxxxxxxx> > For mjpeg assuming a 2560x1600 -> 11mb to get about 262kb (536870 / 2) means a 44:1 compression, not impossible. As somebody is also asking for bigger resolution this would make more possible the overflow. The division, which is the most expensive I think is already doing 64 bit / 64 bit. At the and is not so expensive. By the way, thanks for the review. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel