[Spice-devel] RFC: remove Adler checksum and zlib header/trailer from zlib compressed images

[Yonit Halperin]

On 02/06/2012 09:02 AM, Yaniv Kaul wrote:
> On 01/29/2012 11:29 AM, Alon Levy wrote:
>> On Sun, Jan 29, 2012 at 09:28:34AM +0200, Yaniv Kaul wrote:
>>> These small changes to server and (gtk) client seem to work, and
>>> remove both the zlib header/trailer from the deflate stream as well
>>> as the Adler checksum, which removes several bytes from the stream
>>> as well as speedup (unnoticeable) of the compression/decompression
>>> due to the removal of the checksum calculation.
>>>
>>> Problem is - not sure how to test it - don't know how to get an
>>> image once with and once without the patch - and ensure it's the
>>> same image.
>>> The fact that the first image is never zlib compressed complicates
>>> things. I could not reliably get a 2nd image sent to the client
>>> which looks the same as one in a different connection.
>>>
>>> Nevertheless it does no harm - and images pass well - verified by
>>> connecting with a modified Spice GTK client to a modified Spice
>>> server, as well as looking at the network capture via wireshark.
>>>
>>> It's of course missing the display channel specific capability
>>> negotiation for this change.
>
> I'm afraid I could not really find where to perform the display channel
> specific capability negotiation. :(
> Anyone?
>
Hi,

red_dispatcher.c/red_dispatcher_set_display_peer receives the client's caps.
They should be sent to red_worker.c/handle_dev_display_connect (need to 
change RedWorkerMessageDisplayConnect payload to contain them).
caps are tested via red_channel_client_test_remote_cap.
The negotiation should be in red_worker.c/handle_new_display_channel.

The display channel server caps should be set in 
red_worker.c/display_channel_create via red_channel.set_cap.

Cheers,
Yonit.
> TIA,
> Y.
>
>>> Comments are welcome.
>> Other then that adding a link to the docs page of
>> inlateInit2/deflateInit2 in the commit message would be nice for those
>> seeking to find the explanation for the magic numbers, no comment,
>> thanks for doing this!
>>
>> If you want a repeatable test you can write one or use one of the
>> existing artificial tests in server/tests.
>>
>>> Client change:
>>> diff --git a/gtk/decode-zlib.c b/gtk/decode-zlib.c
>>> index a692020..997f350 100644
>>> --- a/gtk/decode-zlib.c
>>> +++ b/gtk/decode-zlib.c
>>> @@ -63,7 +63,7 @@ SpiceZlibDecoder *zlib_decoder_new(void)
>>> d->_z_strm.opaque = Z_NULL;
>>> d->_z_strm.next_in = Z_NULL;
>>> d->_z_strm.avail_in = 0;
>>> - z_ret = inflateInit(&d->_z_strm);
>>> + z_ret = inflateInit2(&d->_z_strm, -15);
>>> if (z_ret != Z_OK) {
>>> g_warning("zlib decoder init failed, error %d", z_ret);
>>> goto fail;
>>>
>>>
>>> server change:
>>> diff --git a/server/zlib_encoder.c b/server/zlib_encoder.c
>>> index c51466b..66a039a 100644
>>> --- a/server/zlib_encoder.c
>>> +++ b/server/zlib_encoder.c
>>> @@ -47,7 +47,7 @@ ZlibEncoder*
>>> zlib_encoder_create(ZlibEncoderUsrContext *usr, int level)
>>> enc->strm.zfree = Z_NULL;
>>> enc->strm.opaque = Z_NULL;
>>>
>>> - z_ret = deflateInit(&enc->strm, level);
>>> + z_ret = deflateInit2(&enc->strm, level, Z_DEFLATED, -15, 8,
>>> Z_DEFAULT_STRATEGY);
>>> enc->last_level = level;
>>> if (z_ret != Z_OK) {
>>> red_printf("zlib error");
>>> _______________________________________________
>>> Spice-devel mailing list
>>> Spice-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel