On Tue, Jul 03, 2018 at 04:59:12PM -0500, Jonathon Jongsma wrote: > On Thu, 2018-06-28 at 14:23 +0100, Frediano Ziglio wrote: > > QUIC is implemented using 2 C template files. > > One for single channel and one for multiple (RGB) channels. > > Unify the 2 templates to have a single source. > > > > Frediano Ziglio (6): > > quic: Call encode from golomb_coding > > quic: Continue template unification > > quic: Other template unification > > quic: More template unification, unify macros > > quic: Make the template identical > > quic: Remove duplicate file > > > > common/Makefile.am | 1 - > > common/quic.c | 31 +- > > common/quic_family_tmpl.c | 6 +- > > common/quic_rgb_tmpl.c | 689 ------------------------------------ > > -- > > common/quic_tmpl.c | 403 ++++++++++++++-------- > > 5 files changed, 264 insertions(+), 866 deletions(-) > > delete mode 100644 common/quic_rgb_tmpl.c > > > > > So, after reading through this series, I feel that the individual patch > splits were a bit arbitrary. Each patch contains a bunch of individual > sort-of-related changes. But I can't really see much justification for > where the splits were made, to be honest. (After writing this, teuf > told me he was already looking at these patches and had started > splitting them). Yes, it can be refined a little bit (in particular, no commit logs), but I've split that into https://gitlab.com/teuf/spice-common/commits/quic My main comment would be regarding the addition of CHANNEL_ and CHANNEL_ARG_, it seems to me it could be nicer if this became a ENCODER_AND_CHANNEL_ and expanded to either Encoder *encoder or Encoder *encoder, Channel *channl. I think this would make it look a bit more like an actual arg (ie avoid (Encoder *encoder, CHANNEL_ARG_ int foo), with CHANNEL_ARG_ being either empty or 'Channel *channel,'. We'd have (ENCODER_AND_CHANNEL_ARG_, int foo);. Or even wrap that in a macro call like ENCODER_FUNC(func_name)(int foo) Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel