[Spice-devel] [usbredir PATCH v6 1/2] usbredirhost: new callback to drop isoc packets

[Victor Toso]

For streaming devices it might be necessary from application to drop
data for different reasons. This patch provides a new callback that it
is called before queueing the most recent isoc packet.

usbredirhost uses the device information to set two levels of threshold.
In case application's buffer size is higher then the higher threshold we
will drop enough isoc packets to be able to provide a complete video
frame again.

The calculation aims at 10fps as worst case. The higher threshold is set
to 180ms and the lower threshold to 30ms which would give us at least
150ms of continuous data.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1264156
---
 usbredirhost/usbredirhost.c | 62 +++++++++++++++++++++++++++++++++++++++++++--
 usbredirhost/usbredirhost.h | 14 ++++++++++
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
index ad30722..adf9c5f 100644
--- a/usbredirhost/usbredirhost.c
+++ b/usbredirhost/usbredirhost.c
@@ -23,6 +23,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
+#include <stdbool.h>
 #include <string.h>
 #include <errno.h>
 #include <unistd.h>
@@ -109,6 +110,7 @@ struct usbredirhost {
     usbredirparser_read read_func;
     usbredirparser_write write_func;
     usbredirhost_flush_writes flush_writes_func;
+    usbredirhost_buffered_output_size buffered_output_size_func;
     void *func_priv;
     int verbose;
     libusb_context *ctx;
@@ -130,6 +132,11 @@ struct usbredirhost {
     struct usbredirtransfer transfers_head;
     struct usbredirfilter_rule *filter_rules;
     int filter_rules_count;
+    struct {
+        uint64_t higher;
+        uint64_t lower;
+        bool dropping;
+    } iso_threshold;
 };
 
 struct usbredirhost_dev_ids {
@@ -1003,6 +1010,30 @@ static void usbredirhost_send_stream_status(struct usbredirhost *host,
     }
 }
 
+static int usbredirhost_can_write_iso_package(struct usbredirhost *host)
+{
+    uint64_t size;
+
+    if (!host->buffered_output_size_func)
+        return true;
+
+    size = host->buffered_output_size_func(host->func_priv);
+    if (size >= host->iso_threshold.higher) {
+        if (!host->iso_threshold.dropping)
+            DEBUG("START dropping isoc packets %lu buffer > %lu hi threshold",
+                  size, host->iso_threshold.higher);
+        host->iso_threshold.dropping = true;
+    } else if (size < host->iso_threshold.lower) {
+        if (host->iso_threshold.dropping)
+            DEBUG("STOP dropping isoc packets %lu buffer < %lu low threshold",
+                  size, host->iso_threshold.lower);
+
+        host->iso_threshold.dropping = false;
+    }
+
+    return !host->iso_threshold.dropping;
+}
+
 static void usbredirhost_send_stream_data(struct usbredirhost *host,
     uint64_t id, uint8_t ep, uint8_t status, uint8_t *data, int len)
 {
@@ -1028,8 +1059,10 @@ static void usbredirhost_send_stream_data(struct usbredirhost *host,
             .status   = status,
             .length   = len,
         };
-        usbredirparser_send_iso_packet(host->parser, id, &iso_packet,
-                                       data, len);
+
+        if (usbredirhost_can_write_iso_package(host))
+            usbredirparser_send_iso_packet(host->parser, id, &iso_packet,
+                                           data, len);
         break;
     }
     case usb_redir_type_bulk: {
@@ -1120,6 +1153,16 @@ static void usbredirhost_stop_stream(struct usbredirhost *host,
     FLUSH(host);
 }
 
+static void usbredirhost_set_iso_threshold(struct usbredirhost *host,
+    uint8_t pkts_per_transfer, uint8_t transfer_count, uint16_t max_packetsize)
+{
+    uint64_t reference = pkts_per_transfer * transfer_count * max_packetsize;
+    host->iso_threshold.lower = reference / 2;
+    host->iso_threshold.higher = reference * 3;
+    DEBUG("higher threshold is %lu bytes | lower threshold is %lu bytes",
+           host->iso_threshold.higher, host->iso_threshold.lower);
+}
+
 /* Called from both parser read and packet complete callbacks */
 static void usbredirhost_alloc_stream_unlocked(struct usbredirhost *host,
     uint64_t id, uint8_t ep, uint8_t type, uint8_t pkts_per_transfer,
@@ -1178,6 +1221,10 @@ static void usbredirhost_alloc_stream_unlocked(struct usbredirhost *host,
                 host->endpoint[EP2I(ep)].transfer[i], ISO_TIMEOUT);
             libusb_set_iso_packet_lengths(
                 host->endpoint[EP2I(ep)].transfer[i]->transfer, pkt_size);
+
+            usbredirhost_set_iso_threshold(
+                host, pkts_per_transfer,  transfer_count,
+                host->endpoint[EP2I(ep)].max_packetsize);
             break;
         case usb_redir_type_bulk:
             libusb_fill_bulk_transfer(
@@ -1358,6 +1405,17 @@ static void usbredirhost_log_data(struct usbredirhost *host, const char *desc,
 
 /**************************************************************************/
 
+void usbredirhost_set_buffered_output_size_cb(struct usbredirhost *host,
+    usbredirhost_buffered_output_size buffered_output_size_func)
+{
+    if (!host) {
+        ERROR("invalid usbredirhost");
+        return;
+    }
+
+    host->buffered_output_size_func = buffered_output_size_func;
+}
+
 /* Return value:
     0 All ok
     1 Packet borked, continue with next packet / urb
diff --git a/usbredirhost/usbredirhost.h b/usbredirhost/usbredirhost.h
index c0042c9..042e084 100644
--- a/usbredirhost/usbredirhost.h
+++ b/usbredirhost/usbredirhost.h
@@ -33,6 +33,8 @@ struct usbredirhost;
 
 typedef void (*usbredirhost_flush_writes)(void *priv);
 
+typedef uint64_t (*usbredirhost_buffered_output_size)(void *priv);
+
 /* This function creates an usbredirhost instance, including its embedded
    libusbredirparser instance and sends the initial usb_redir_hello packet to
    the usb-guest.
@@ -114,6 +116,18 @@ void usbredirhost_close(struct usbredirhost *host);
 int usbredirhost_set_device(struct usbredirhost *host,
                             libusb_device_handle *usb_dev_handle);
 
+/* Call this function to set a callback in usbredirhost.
+   The usbredirhost_buffered_output_size callback should return the
+   application's buffer size (in bytes) that are handling the isochronous data.
+  
+   usbredirhost will set two levels of threshold based in the information
+   provided by the usb device. In case the application's buffer is increasing
+   too much then usbredirhost uses the threshold limits to drop isochronous
+   packages but still send full frames whenever is possible.
+*/
+void usbredirhost_set_buffered_output_size_cb(struct usbredirhost *host,
+    usbredirhost_buffered_output_size buffered_output_size_func);
+
 /* Call this whenever there is data ready for the usbredirhost to read from
    the usb-guest
    returns 0 on success, or an error code from the below enum on error.
-- 
2.5.0