> > Any more thoughts/reviews on this series? > > Christophe > I asked to have a test and you said is not worth. I asked to split the series in different ones and you said is not worth. For me a single "is not worth" is a self nack. So... why are you asking for though? Frediano > 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. > > > > > 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). > > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel