Hi, On 06/24/2016 06:57 PM, Victor Toso wrote:
yes, it should be better to send data of isochronous devices uncompressed (although it is not guaranteed that isoc devices data is already compressed). as far as i looked with Uri there are two ways to do so: 1. by looking for the type in each of the endpoints descriptors via libusb 2. to check if usbredir_buffered_output_size_callback is being called and if it is, it means the device is isoc (kind of hackish)Hi, On Mon, Jun 13, 2016 at 07:54:34PM +0300, Snir Sheriber wrote:Compressed message type is CompressedData which contains compression type (1 byte) followed by the uncompressed data size (4 bytes-exists only if data was compressed) followed by the compressed data If SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4 capability is available && data_size > COMPRESS_THRESHOLD && !AF_LOCAL data will be sent compressed otherwise data will be sent uncompressed (as well if compression has failed) Update required spice-protocol to 0.12.12Now that we did the spice-gtk release, updating the spice-common and bumping the spice-protocol requirement should be okay. I tested this before but I plan to double check it today, if no problems and no one complains, I plan to push this next by Monday. Minor comments below:Signed-off-by: Snir Sheriber <ssheribe@xxxxxxxxxx> --- configure.ac | 2 +- src/channel-usbredir.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 3fe8055..f2acb23 100644 --- a/configure.ac +++ b/configure.ac @@ -69,7 +69,7 @@ AC_CHECK_LIBM AC_SUBST(LIBM)AC_CONFIG_SUBDIRS([spice-common])-PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.12.11]) +PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.12.12])COMMON_CFLAGS='-I ${top_srcdir}/spice-common/ ${SPICE_PROTOCOL_CFLAGS}'AC_SUBST(COMMON_CFLAGS) diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c index 2c5feae..bb69e26 100644 --- a/src/channel-usbredir.c +++ b/src/channel-usbredir.c @@ -24,6 +24,9 @@ #ifdef USE_USBREDIR #include <glib/gi18n.h> #include <usbredirhost.h> +#ifdef USE_LZ4 +#include <lz4.h> +#endif #ifdef USE_POLKIT #include "usb-acl-helper.h" #endif @@ -32,6 +35,7 @@ #include "usbutil.h" #endif+#include "common/log.h"#include "spice-client.h" #include "spice-common.h"@@ -51,6 +55,7 @@ #ifdef USE_USBREDIR +#define COMPRESS_THRESHOLD 1000#define SPICE_USBREDIR_CHANNEL_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_USBREDIR_CHANNEL, SpiceUsbredirChannelPrivate))@@ -233,6 +238,7 @@ static void channel_set_handlers(SpiceChannelClass *klass){ static const spice_msg_handler handlers[] = { [ SPICE_MSG_SPICEVMC_DATA ] = usbredir_handle_msg, + [ SPICE_MSG_SPICEVMC_COMPRESSED_DATA ] = usbredir_handle_msg, };spice_channel_set_handlers(klass, handlers, G_N_ELEMENTS(handlers));@@ -269,6 +275,9 @@ void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel, #if USBREDIR_VERSION >= 0x000701 usbredirhost_set_buffered_output_size_cb(priv->host, usbredir_buffered_output_size_callback); #endif +#ifdef USE_LZ4 + spice_channel_set_capability(channel, SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4); +#endif }static gboolean spice_usbredir_channel_open_device(@@ -632,11 +641,71 @@ static void usbredir_free_write_cb_data(uint8_t *data, void *user_data) usbredirhost_free_write_buffer(priv->host, data); }+#ifdef USE_LZ4+static int try_write_compress_LZ4(SpiceUsbredirChannel *channel, uint8_t *data, int count) { + SpiceChannelPrivate *c; + SpiceMsgOut *msg_out_compressed; + int bound, compressed_data_count; + uint8_t *compressed_buf; + SpiceMsgCompressedData compressed_data_msg = { + .type = SPICE_DATA_COMPRESSION_TYPE_LZ4, + .uncompressed_size = count + }; + + c = SPICE_CHANNEL(channel)->priv; + if (g_socket_get_family(c->sock) == G_SOCKET_FAMILY_UNIX) { + /* AF_LOCAL socket - data will not be compressed */ + return FALSE; + } + if (count <= COMPRESS_THRESHOLD) { + /* Not enough data to compress */ + return FALSE; + } + if (!spice_channel_test_capability(SPICE_CHANNEL(channel), + SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4)) { + /* No server compression capability - data will not be compressed */ + return FALSE; + } + bound = LZ4_compressBound(count); + if (bound == 0) { + /* Invalid bound - data will not be compressed */ + return FALSE; + }What about devices of isochronous type? Here, the packages for my webcam were < COMPRESS_THRESHOLD but I guess we could save some time/process in general by avoiding compressing in case the device is isoc.
what do you think should be used here?
+ + compressed_buf = (uint8_t*)spice_malloc(bound); + compressed_data_count = LZ4_compress_default((char*)data, + (char*)compressed_buf, + count, + bound); + if (compressed_data_count > 0 && compressed_data_count < count) { + compressed_data_msg.compressed_data = compressed_buf; + msg_out_compressed = spice_msg_out_new(SPICE_CHANNEL(channel), + SPICE_MSGC_SPICEVMC_COMPRESSED_DATA); + msg_out_compressed->marshallers->msg_SpiceMsgCompressedData(msg_out_compressed->marshaller, + &compressed_data_msg); + spice_marshaller_add_ref_full(msg_out_compressed->marshaller, + compressed_data_msg.compressed_data, + compressed_data_count, + usbredir_free_write_cb_data, + channel); + spice_msg_out_send(msg_out_compressed); + return TRUE; + } + /* if not - free & fallback to sending the message uncompressed */ + free(compressed_buf); + return FALSE; +} +#endif + static int usbredir_write_callback(void *user_data, uint8_t *data, int count) { SpiceUsbredirChannel *channel = user_data; SpiceMsgOut *msg_out;+#ifdef USE_LZ4+ if (try_write_compress_LZ4(channel, data, count)) + return count; +#endif msg_out = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_SPICEVMC_DATA); spice_marshaller_add_ref_full(msg_out->marshaller, data, count, @@ -724,11 +793,49 @@ static void spice_usbredir_channel_up(SpiceChannel *c) usbredirhost_write_guest_data(priv->host); }+static int try_handle_compressed_msg(SpiceMsgCompressedData *compressed_data_msg,+ uint8_t **buf, + int *size) { + int decompressed_size = 0; + char *decompressed = NULL; + + if (compressed_data_msg->uncompressed_size == 0) { + spice_warning("Invalid uncompressed_size"); + return FALSE; + } + + switch (compressed_data_msg->type) { +#ifdef USE_LZ4 + case SPICE_DATA_COMPRESSION_TYPE_LZ4: + decompressed = g_malloc(compressed_data_msg->uncompressed_size); + decompressed_size = LZ4_decompress_safe ((char*)compressed_data_msg->compressed_data, + decompressed, + compressed_data_msg->compressed_size, + compressed_data_msg->uncompressed_size); + break; +#endif + default: + spice_warning("Unknown Compression Type"); + return FALSE; + } + if (decompressed_size != compressed_data_msg->uncompressed_size) { + spice_warning("Decompress Error decompressed_size=%d expected=%d", + decompressed_size, compressed_data_msg->uncompressed_size);with -Werror-format, build fails here with: error: format ‘%d’ expects argument of type ‘int’, but argument 7 has type ‘uint32_t {aka unsigned int}’ expected=%d", Cheers, toso
Thanks! shouldn't be missed :/
+ g_free(decompressed); + return FALSE; + } + + *size = decompressed_size; + *buf = (uint8_t*)decompressed; + return TRUE; + +} + static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in) { SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c); SpiceUsbredirChannelPrivate *priv = channel->priv; - int r, size; + int r = 0, size; uint8_t *buf;g_return_if_fail(priv->host != NULL);@@ -736,13 +843,23 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in) /* No recursion allowed! */ g_return_if_fail(priv->read_buf == NULL);- buf = spice_msg_in_raw(in, &size);- priv->read_buf = buf; - priv->read_buf_size = size; + if (spice_msg_in_type(in) == SPICE_MSG_SPICEVMC_COMPRESSED_DATA) { + SpiceMsgCompressedData *compressed_data_msg = spice_msg_in_parsed(in); + if (try_handle_compressed_msg(compressed_data_msg, &buf, &size)) { + priv->read_buf_size = size; + priv->read_buf = buf; + } else { + r = usbredirhost_read_parse_error; + } + } else { /* Regular SPICE_MSG_SPICEVMC_DATA msg */ + buf = spice_msg_in_raw(in, &size); + priv->read_buf_size = size; + priv->read_buf = buf; + }spice_usbredir_channel_lock(channel);- - r = usbredirhost_read_guest_data(priv->host); + if (r == 0) + r = usbredirhost_read_guest_data(priv->host); if (r != 0) { SpiceUsbDevice *spice_device = priv->spice_device; device_error_data err_data; -- 2.5.5 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel