[Spice-devel] Adjust JPEG Quality according to Bandwidth

[Victor Toso]

Hey,

Thanks for your time and interest in spice,

On Sat, Mar 21, 2015 at 11:38:33AM +0100, Flavio G. wrote:
> Hi,
>
> my name is Flavio (deano in IRC). Because I want to get more practice in C
> Programming and Programming generally and because I like spice very much I
> started reading source code of spice server and already written a very
> little patch. Maybe you could tell me if I am on the right track or if it
> is garbage what I do :)

IMHO, you are in the right track regarding the jpeg compression. The
main issue I think is that the bandwidth is only calculated once, right?
I guess it should be dynamic.

I believe someone more knowledgeable about the server can give you better
feedback.

> So: I am using wireless LAN and the access point is relativly far away. So
> I just get ca. 8-9 Megabit per seconds. When using LibreOffice for example
> on the VM it runs smoothly. But when opening a website for example with
> Pictures, Flash animations and maybe ads the connection is more or less
> unusable (at 100MBit Ethernet performance is absolute adequate). I saw that
> JPEG compression is "hard coded" to 85.
> My idea is to classify the available bandwidth (enum Bandwidth) and based
> on the classification adjust the JPEG compression level.
> The values for the JPEG level are orientated on the examples from
> http://regex.info/blog/lightroom-goodies/jpeg-quality
> I don't know if a connection 4 Megabit is very realistic. But the Jump from
> 85 to 54 saves some bandwidth for me in WLAN and I can Browse better.
> Adjusting the compression level to 7 lets play youtube videos very good but
> they are fairly ugly.
>
> Greetings, Flavio

There is also a bandwidth monitoring RFE open if you are interesting in
that, feel free to comment o the bug report.

https://bugs.freedesktop.org/show_bug.cgi?id=87324

> ---
> 
> server/main_channel.c | 16 ++++++++++++++++
> server/main_channel.h | 10 ++++++++++
> server/red_worker.c   | 22 ++++++++++++++++++++--
> 3 files changed, 46 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/server/main_channel.h b/server/main_channel.h
> index c8e9ade..f956778 100644
> --- a/server/main_channel.h
> +++ b/server/main_channel.h
> @@ -40,6 +40,14 @@ typedef struct MainChannel {
>      int num_clients_mig_wait;
>  } MainChannel;
> 
> +typedef enum Bandwidth {
> + LAN,
> + WAN,
> + EIGHT_MBPS,
> + FOUR_MBPS,
> + VERY_LOW
> +} Bandwidth;
> 
>  MainChannel *main_channel_init(void);
>  RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan,
> uint32_t link_id);
> @@ -76,6 +84,8 @@ int
> main_channel_client_is_low_bandwidth(MainChannelClient *mcc);
>  uint64_t main_channel_client_get_bitrate_per_sec(MainChannelClient *mcc);
>  uint64_t main_channel_client_get_roundtrip_ms(MainChannelClient *mcc);
> 
> +Bandwidth main_channel_client_get_bandwidth_type(MainChannelClient *mcc);
> +
>  int main_channel_is_connected(MainChannel *main_chan);
>  RedChannelClient* main_channel_client_get_base(MainChannelClient* mcc);
> 
> 
> 
> diff --git a/server/main_channel.c b/server/main_channel.c
> index 54718ba..c656820 100644
> --- a/server/main_channel.c
> +++ b/server/main_channel.c
> @@ -1171,6 +1171,22 @@ int
> main_channel_client_is_low_bandwidth(MainChannelClient *mcc)
>      return mcc->bitrate_per_sec < 10 * 1024 * 1024;
>  }
> 
> +Bandwidth main_channel_client_get_bandwidth_type(MainChannelClient *mcc)
> +{
> +    int mbps = mcc->bitrate_per_sec / (1024*1024);
> +
> +    if(mbps > 10)
> + return LAN;
> +    else if(mbps <= 10 && mbps > 8)
> + return WAN;
> +    else if(mbps <= 8 && mbps > 6)
> +        return EIGHT_MBPS;
> +    else if(mbps <= 6 && mbps > 3)
> +        return FOUR_MBPS;
> +
> +        return VERY_LOW;
> +}
> +
>  uint64_t main_channel_client_get_bitrate_per_sec(MainChannelClient *mcc)
>  {
>      return mcc->bitrate_per_sec;
> 
> 
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 5deb30b..7448232 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -680,6 +680,7 @@ typedef struct CommonChannelClient {
>      uint32_t id;
>      struct RedWorker *worker;
>      int is_low_bandwidth;
> +    Bandwidth bandwidth_type;
>  } CommonChannelClient;
> 
>  /* Each drawable can refer to at most 3 images: src, brush and mask */
> @@ -10318,6 +10319,7 @@ static int
> common_channel_config_socket(RedChannelClient *rcc)
> 
>      // TODO - this should be dynamic, not one time at channel creation
>      ccc->is_low_bandwidth = main_channel_client_is_low_bandwidth(mcc);
> +    ccc->bandwidth_type = main_channel_client_get_bandwidth_type(mcc);
>      delay_val = ccc->is_low_bandwidth ? 0 : 1;
>      /* FIXME: Using Nagle's Algorithm can lead to apparent delays,
> depending
>       * on the delayed ack timeout on the other side.
> @@ -10803,9 +10805,25 @@ static void handle_new_display_channel(RedWorker
> *worker, RedClient *client, Red
>      } else {
>          display_channel->enable_jpeg = (worker->jpeg_state ==
> SPICE_WAN_COMPRESSION_ALWAYS);
>      }
> -
>      // todo: tune quality according to bandwidth
> -    display_channel->jpeg_quality = 85;
> +
> +    switch(dcc->common.bandwidth_type) {
> +        case LAN:
> +            display_channel->jpeg_quality = 85;
> +            break;
> +        case WAN:
> +            display_channel->jpeg_quality = 54;
> +            break;
> +        case EIGHT_MBPS:
> +            display_channel->jpeg_quality = 39;
> +            break;
> +        case FOUR_MBPS:
> +            display_channel->jpeg_quality = 16;
> +            break;
> +        default:
> +            // very bad performance and very bad quality
> +            display_channel->jpeg_quality = 7;
> +    }
> 
>      if (worker->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
>          display_channel->enable_zlib_glz_wrap =
> dcc->common.is_low_bandwidth;

> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel