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

Frediano Ziglio fziglio at redhat.com
Wed Jul 5 14:27:18 UTC 2017


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

> The main difference is that the former implementation handles a single
> channel,
> while the rgb implementation handles each color component separately. Besides
> that,
> the implementation is the same, but this is not obvious at all by just
> comparing the
> 2 files. This patch series makes the 2 files closer, but not easily mergeable
> yet ;)
> 
> The biggest pending difference is that quic_tmpl.c carries around a 'channel'
> with the data it needs, while quic_rgb_tmpl.c iterates over Encoder::channels
> when needed.
> 
> This imo does not justify duplicating that much code, but I don't want to
> spend
> too much time on quic either, so I'll stop there for now :)
> 
> I've only tested the RGB32 case, as I'm not sure how to trigger the 1 byte or
> 4
> byte code paths (I've tried win7 and fedora 25 guests so far).
> 
> Christophe

The problem of point 4) is that is worth but if we are not sure we are
not breaking stuff is not that great. We should have a test that test only
image compression/decompression. I was trying to write a similar test
for Windows agent for png using a mix of shell/ImageMagick/C code. Lost
the commands I was using :-( but mainly in imagemagick there are some test
images, I was taking these images with different formats (grey/palette/rgb/
rgba and so on) and trying to compress -> uncompress checking that input
and output was the same (as PNG is lossless).

Frediano


More information about the Spice-devel mailing list