[Spice-devel] Adjust JPEG Quality according to Bandwidth

[Christophe Fergeau]

Hey,

Sorry for the very late answer,

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 :)
> 
> 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.


Generally, your patch makes sense. I'm a bit worried by the quality loss
when switching to very high compression factors. Dropping some frames
might be better, but I suspect this is complicated (?)

> 
> Greetings, Flavio
> 
> ---
> 
> 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;
> +}
> +

I guess "main_channel_client_is_slow_client could be changed to use that
helper. Once you have checked for mbps > 10 and know it's false, you
don't need to check again for mbps <= 10 in the following "else if"

Christophe

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150331/6c02d38e/attachment.sig>