[Spice-devel] [spice-common] Some steps toward quic_tmpl.c and quic_rgb_tmpl.c 'unification'

[Frediano Ziglio]

> 
> On Wed, Jul 05, 2017 at 10:27:18AM -0400, Frediano Ziglio wrote:
> > > 
> > > Hey,
> > > 
> > > This is a v2 of the initial 2 patches, but looking a bit closer at the
> > > quic
> > > code led me down a rabbit hole ;) We currently have 2 different quic
> > > implementations using the preprocessor as a template engine. These 2
> > > implementations are quic_tmpl.c which handles 1 byte and 4 bytes images,
> > > and the quic_rgb_tmpl.c which handles 24bpp/32bpp/16bpp images.
> > > 
> > 
> > Looks like this more than a series is a set of chained series dealing with
> > 1- compiler problems
> > 2- typos
> > 3- removal obsolete stuff
> > 4- code reuse/unification
> > 
> > I think 1) and 2) could be integrated straight forward. 3) requires
> > mostly a team agreement that I don't think is hard to have, Quic is used
> > only by Spice, already superseded by other encoding so I don't see any
> > point keeping old dead code branches dealing with different settings.
> 
> Here is an attempt at classifying things, but they are mostly in order
> in the series I sent already. I'd rather not resend separate series
> when they have been unchanged since the first sending.
> 
> ~~~~
> 
>   quic: Use #define rather than static const int
>   quic: Use upper-case for constant names
> 
> These 2 patches are 1) in your list, and fix a compilation issue, so it would
> be nice to get them in sooner rather than later ;)
> 
> ~~~~
> 
>   quic: Fix "corelate" typo
> 
> Typo fix, 2)
> 
> ~~~~
> 
>   quic: Remove configurable RLE_PRED
>   quic: Remove configurable PRED
>   quic: Get rid of QUIC_RGB #define
>   quic: Get rid of RLE_STAT #define
>   quic: Get rid of RLE #define
> 
> Removal of obsolete stuff (3). We cannot change some of these anyways (I
> tested PRED) as they would change the on-wire protocol. client and
> server have to agree on the value of PRED to use, otherwise connection
> won't work.
> 
> ~~~~
> 
>   quic: Factor common code
>   quic: Introduce CommonState *state variable in templates
>   quic: Use i == 0 rather than !i
>   quic: s/decorrelate_drow/correlate_row
>   quic: Add macros to make quic_tmpl.c much closer to quic_rgb_tmpl.c
>   quic: Remove unused argument in uncompress_row{0,}
>   quic: Use channel->correlate_row in macros
>   quic: Remove undocumented 'no-inline' hack
>   quic: Add test case for compression/decompression
> 
> These remaining ones are 4) could indeed break things, but they are
> mostly mechanical, so hopefully not too hard to review (even without an
> extensive test case). I can resend them separately later if they feel
> too risky at the moment.
> 

Isn't the s/decorrelate_drow/correlate_row patch part of 2).
I remember you mentioned also the fact that Windows driver contains
Quic code. I checked and is really similar. Should we try to reuse
spice-common code directly?

> Christophe
> 

Frediano