> > On Wed, 2015-11-11 at 06:47 -0500, Frediano Ziglio wrote: > > > > > > Hmm, this code was fairly weird. It's still a bit weird that the > > > 'more_space' > > > vfunc for the quic encoder has different semantics than all of the rest > > > of > > > them > > > (returning the number of uint32_t elements allocated instead of the > > > number > > > of > > > bytes allocated). But the code is slightly less confusing now. ACK > > > > > > > > > > Did you see > > http://lists.freedesktop.org/archives/spice-devel/2015-November/023358.html? > > > Ah, I missed this one. Sorry. > > > > > On the same subject... is it ok to post again same patch if there are > > comments > > or is it confusing? > > > Yeah, it is a little confusing to repost the same patch when there are > pending > comments. It would be slightly easier to follow if the reposted patch > contained > an annotation that referenced the earlier discussion, perhaps, but that means > more work for the person posting patch series (which is you, currently). > Alternatively, I can just try to pay closer attention :) > > > > > I personally agree encoder_usr_more_space should return bytes and not > > uint32_t > > elements however I prefer to have uint32_t in the structure. > > Yeah, now that I look at it a bit more closely, I think that the union > approach > is not so bad. > I'll try to came up with a proposed patch tomorrow Frediano > > > > > > > > > On Tue, 2015-11-10 at 14:16 +0000, Frediano Ziglio wrote: > > > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> > > > > > > > > --- > > > > server/display-channel.h | 3 +-- > > > > server/red_worker.c | 18 +++++++++--------- > > > > 2 files changed, 10 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/server/display-channel.h b/server/display-channel.h > > > > index 7c62a62..f1f4e3a 100644 > > > > --- a/server/display-channel.h > > > > +++ b/server/display-channel.h > > > > @@ -70,10 +70,9 @@ > > > > #define NUM_STREAMS 50 > > > > #define NUM_SURFACES 10000 > > > > > > > > -#define RED_COMPRESS_BUF_SIZE (1024 * 64) > > > > typedef struct RedCompressBuf RedCompressBuf; > > > > struct RedCompressBuf { > > > > - uint32_t buf[RED_COMPRESS_BUF_SIZE / 4]; > > > > + uint8_t buf[64 * 1024]; > > > > RedCompressBuf *next; > > > > RedCompressBuf *send_next; > > > > }; > > > > diff --git a/server/red_worker.c b/server/red_worker.c > > > > index 1b8951f..5daca87 100644 > > > > --- a/server/red_worker.c > > > > +++ b/server/red_worker.c > > > > @@ -3886,7 +3886,7 @@ static void glz_usr_free(GlzEncoderUsrContext > > > > *usr, > > > > void > > > > *ptr) > > > > free(ptr); > > > > } > > > > > > > > -static inline int encoder_usr_more_space(EncoderData *enc_data, > > > > uint32_t > > > > **io_ptr) > > > > +static int encoder_usr_more_space(EncoderData *enc_data, uint8_t > > > > **io_ptr) > > > > { > > > > RedCompressBuf *buf; > > > > > > > > @@ -3897,31 +3897,31 @@ static inline int > > > > encoder_usr_more_space(EncoderData > > > > *enc_data, uint32_t **io_pt > > > > enc_data->bufs_tail = buf; > > > > buf->send_next = NULL; > > > > *io_ptr = buf->buf; > > > > - return sizeof(buf->buf) >> 2; > > > > + return sizeof(buf->buf); > > > > } > > > > > > > > static int quic_usr_more_space(QuicUsrContext *usr, uint32_t **io_ptr, > > > > int > > > > rows_completed) > > > > { > > > > EncoderData *usr_data = &(((QuicData *)usr)->data); > > > > - return encoder_usr_more_space(usr_data, io_ptr); > > > > + return encoder_usr_more_space(usr_data, (uint8_t **)io_ptr) / > > > > sizeof(uint32_t); > > > > } > > > > > > > > static int lz_usr_more_space(LzUsrContext *usr, uint8_t **io_ptr) > > > > { > > > > EncoderData *usr_data = &(((LzData *)usr)->data); > > > > - return (encoder_usr_more_space(usr_data, (uint32_t **)io_ptr) << > > > > 2); > > > > + return encoder_usr_more_space(usr_data, io_ptr); > > > > } > > > > > > > > static int glz_usr_more_space(GlzEncoderUsrContext *usr, uint8_t > > > > **io_ptr) > > > > { > > > > EncoderData *usr_data = &(((GlzData *)usr)->data); > > > > - return (encoder_usr_more_space(usr_data, (uint32_t **)io_ptr) << > > > > 2); > > > > + return encoder_usr_more_space(usr_data, io_ptr); > > > > } > > > > > > > > static int jpeg_usr_more_space(JpegEncoderUsrContext *usr, uint8_t > > > > **io_ptr) > > > > { > > > > EncoderData *usr_data = &(((JpegData *)usr)->data); > > > > - return (encoder_usr_more_space(usr_data, (uint32_t **)io_ptr) << > > > > 2); > > > > + return encoder_usr_more_space(usr_data, io_ptr); > > > > } > > > > > > > > #ifdef USE_LZ4 > > > > @@ -3935,7 +3935,7 @@ static int > > > > lz4_usr_more_space(Lz4EncoderUsrContext > > > > *usr, > > > > uint8_t **io_ptr) > > > > static int zlib_usr_more_space(ZlibEncoderUsrContext *usr, uint8_t > > > > **io_ptr) > > > > { > > > > EncoderData *usr_data = &(((ZlibData *)usr)->data); > > > > - return (encoder_usr_more_space(usr_data, (uint32_t **)io_ptr) << > > > > 2); > > > > + return encoder_usr_more_space(usr_data, io_ptr); > > > > } > > > > > > > > static inline int encoder_usr_more_lines(EncoderData *enc_data, > > > > uint8_t > > > > **lines) > > > > @@ -4589,8 +4589,8 @@ static inline int > > > > red_quic_compress_image(DisplayChannelClient *dcc, SpiceImage > > > > stride = -src->stride; > > > > } > > > > size = quic_encode(quic, type, src->x, src->y, NULL, 0, stride, > > > > - quic_data->data.bufs_head->buf, > > > > - sizeof(quic_data->data.bufs_head->buf) >> 2); > > > > + (uint32_t*)quic_data->data.bufs_head->buf, > > > > + sizeof(quic_data->data.bufs_head->buf) / > > > > sizeof(uint32_t)); > > > > > > > > // the compressed buffer is bigger than the original data > > > > if ((size << 2) > (src->y * src->stride)) { > > > > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel