[Spice-devel] [usbredir PATCH v3] usbredir memory leak

[Hans de Goede]

Hi,

On 19-10-15 14:25, Fabiano Fidêncio wrote:
> Adding Hans ...

Thanks, I missed the original patch, Victor, can you please
Cc me when sending this patches, and perhaps resend v3 with
me in the Cc ?

> On Mon, Oct 19, 2015 at 12:07 PM, Victor Toso <victortoso at redhat.com> wrote:
>> This is already better then v2 due the fact that we are only dropping
>> isochronous packets. Still need to find a way to drop the correct
>> amount of packages so usbredir always start sending data from start of
>> payload (not in the middle of a frame, for instance)

I'm afraid that dropping the right amount of packages is pretty
much impossible without adding support for every usb isoc protocol
out there. I would simply go for making sure that when you drop,
you drop a bunch of packets at once, rather then dropping every
other packets. This means that when dropping and the stream is
a video stream you will cause the frame currently being send +
how every many you drop + the frame which is in progress when
you stop dropping to get dropped. If you add smartness wrt
the amount of packets to drop, the only thing which will change
in that equation is the "+ the frame which is in progress when
you stop dropping" so without this you effectively end up
dropping 1 frame more (by causing it to be corrupted, at which
point the receiving side should drop it). I do not think that
that is a big deal.

What would be interesting is to change the callback from
a boolean to something returning how much data is buffered
by the channel code, then you can put the threshold when
to start / stop buffering inside the usbredirhost code and
make it depend on the isoc packet size + number of packets /
second. This way you can make it so that you always drop say
200ms of data (which will be a couple of frames), rather then
say always drop 2MB of data, which will be 1.5 frames at a
high resolution and many more at a low resolution.

Regards,

Hans