[Spice-devel] [PATCH 11/11] worker: don't use weird RedCompressedBuf nbytes shifting

[Fabiano Fidêncio]

On Wed, Nov 11, 2015 at 1:20 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>
> ---
>  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 12ef60a..599cce7 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 f37da2b..10b95e8 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -3759,7 +3759,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;
>
> @@ -3770,31 +3770,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
> @@ -3808,7 +3808,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)
> @@ -4462,8 +4462,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)) {
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

As Jonathon said, this code is still a bit weird.
I'd prefer to keep it as it is right now and drop the patch instead of
adding the whole union dance.