Re: [PATCH 6/7] worker: don't use weird RedCompressedBuf nbytes shifting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Nov 13, 2015 at 1:26 PM, Pavel Grunt <pgrunt@xxxxxxxxxx> wrote:
> On Fri, 2015-11-13 at 06:20 -0500, Frediano Ziglio wrote:
>> ---
>>  server/display-channel.h |  9 ++++++++-
>>  server/red_worker.c      | 41 ++++++++++++++++++++++-------------------
>>  2 files changed, 30 insertions(+), 20 deletions(-)
>>
>> diff --git a/server/display-channel.h b/server/display-channel.h
>> index 12ef60a..7aa73cf 100644
>> --- a/server/display-channel.h
>> +++ b/server/display-channel.h
>> @@ -73,7 +73,14 @@
>>  #define RED_COMPRESS_BUF_SIZE (1024 * 64)
>>  typedef struct RedCompressBuf RedCompressBuf;
>>  struct RedCompressBuf {
>> -    uint32_t buf[RED_COMPRESS_BUF_SIZE / 4];
>> +    /* This buffer provide space for compression algorithms.
>> +     * Some algorithms access the buffer as an array of 32 bit words
>> +     * so is defined to make sure is always aligned that way.
>> +     */
>> +    union {
>> +        uint8_t  bytes[RED_COMPRESS_BUF_SIZE];
>> +        uint32_t words[RED_COMPRESS_BUF_SIZE / 4];
>> +    } buf;
>>      RedCompressBuf *next;
>>      RedCompressBuf *send_next;
>>  };
>> diff --git a/server/red_worker.c b/server/red_worker.c
>> index af85591..aad9b4a 100644
>> --- a/server/red_worker.c
>> +++ b/server/red_worker.c
>> @@ -3343,7 +3343,7 @@ static void marshaller_add_compressed(SpiceMarshaller
>> *m,
>>          spice_assert(comp_buf);
>>          now = MIN(sizeof(comp_buf->buf), max);
>>          max -= now;
>> -        spice_marshaller_add_ref(m, (uint8_t*)comp_buf->buf, now);
>> +        spice_marshaller_add_ref(m, comp_buf->buf.bytes, now);
>>          comp_buf = comp_buf->send_next;
>>      } while (max);
>>  }
>> @@ -3758,7 +3758,10 @@ 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)
>> +/* Allocate more space for compressed buffer.
>> + * The pointer returned in io_ptr is garanteed to be aligned to 4 bytes.
>> + */
>> +static int encoder_usr_more_space(EncoderData *enc_data, uint8_t **io_ptr)
>>  {
>>      RedCompressBuf *buf;
>>
>> @@ -3768,46 +3771,46 @@ static inline int encoder_usr_more_space(EncoderData
>> *enc_data, uint32_t **io_pt
>>      enc_data->bufs_tail->send_next = buf;
>>      enc_data->bufs_tail = buf;
>>      buf->send_next = NULL;
>> -    *io_ptr = buf->buf;
>> -    return sizeof(buf->buf) >> 2;
>> +    *io_ptr = buf->buf.bytes;
>> +    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
>>  static int lz4_usr_more_space(Lz4EncoderUsrContext *usr, uint8_t **io_ptr)
>>  {
>>      EncoderData *usr_data = &(((Lz4Data *)usr)->data);
>> -    return (encoder_usr_more_space(usr_data, (uint32_t **)io_ptr) << 2);
>> +    return encoder_usr_more_space(usr_data, io_ptr);
>>  }
>>  #endif
>>
>>  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)
>> @@ -3882,7 +3885,7 @@ static int zlib_usr_more_input(ZlibEncoderUsrContext
>> *usr, uint8_t** input)
>>          return 0;
>>      }
>>
>> -    *input = (uint8_t*)usr_data->u.compressed_data.next->buf;
>> +    *input = usr_data->u.compressed_data.next->buf.bytes;
>>      buf_size = MIN(sizeof(usr_data->u.compressed_data.next->buf),
>>                     usr_data->u.compressed_data.size_left);
>>
>> @@ -4045,7 +4048,7 @@ static inline int
>> red_glz_compress_image(DisplayChannelClient *dcc,
>>
>>      glz_size = glz_encode(dcc->glz, type, src->x, src->y,
>>                            (src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN), NULL,
>> 0,
>> -                          src->stride, (uint8_t*)glz_data->data.bufs_head-
>> >buf,
>> +                          src->stride, glz_data->data.bufs_head->buf.bytes,
>>                            sizeof(glz_data->data.bufs_head->buf),
>>                            glz_drawable_instance,
>>                            &glz_drawable_instance->glz_instance);
>> @@ -4075,7 +4078,7 @@ static inline int
>> red_glz_compress_image(DisplayChannelClient *dcc,
>>      zlib_data->data.u.compressed_data.size_left = glz_size;
>>
>>      zlib_size = zlib_encode(worker->zlib, display_channel->zlib_level,
>> -                            glz_size, (uint8_t*)zlib_data->data.bufs_head-
>> >buf,
>> +                            glz_size, zlib_data->data.bufs_head->buf.bytes,
>>                              sizeof(zlib_data->data.bufs_head->buf));
>>
>>      // the compressed buffer is bigger than the original data
>> @@ -4150,7 +4153,7 @@ static inline int
>> red_lz_compress_image(DisplayChannelClient *dcc,
>>      size = lz_encode(lz, type, src->x, src->y,
>>                       !!(src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN),
>>                       NULL, 0, src->stride,
>> -                     (uint8_t*)lz_data->data.bufs_head->buf,
>> +                     lz_data->data.bufs_head->buf.bytes,
>>                       sizeof(lz_data->data.bufs_head->buf));
>>
>>      // the compressed buffer is bigger than the original data
>> @@ -4263,7 +4266,7 @@ static int red_jpeg_compress_image(DisplayChannelClient
>> *dcc, SpiceImage *dest,
>>      }
>>      jpeg_size = jpeg_encode(jpeg, display_channel->jpeg_quality,
>> jpeg_in_type,
>>                              src->x, src->y, NULL,
>> -                            0, stride, (uint8_t*)jpeg_data->data.bufs_head-
>> >buf,
>> +                            0, stride, jpeg_data->data.bufs_head->buf.bytes,
>>                              sizeof(jpeg_data->data.bufs_head->buf));
>>
>>      // the compressed buffer is bigger than the original data
>> @@ -4289,7 +4292,7 @@ static int red_jpeg_compress_image(DisplayChannelClient
>> *dcc, SpiceImage *dest,
>>
>>      comp_head_filled = jpeg_size % sizeof(lz_data->data.bufs_head->buf);
>>      comp_head_left = sizeof(lz_data->data.bufs_head->buf) - comp_head_filled;
>> -    lz_out_start_byte = ((uint8_t *)lz_data->data.bufs_head->buf) +
>> comp_head_filled;
>> +    lz_out_start_byte = lz_data->data.bufs_head->buf.bytes +
>> comp_head_filled;
>>
>>      lz_data->data.dcc = dcc;
>>
>> @@ -4372,7 +4375,7 @@ static int red_lz4_compress_image(DisplayChannelClient
>> *dcc, SpiceImage *dest,
>>      lz4_data->data.u.lines_data.reverse = 0;
>>      lz4_data->usr.more_lines = lz4_usr_more_lines;
>>
>> -    lz4_size = lz4_encode(lz4, src->y, src->stride, (uint8_t*)lz4_data-
>> >data.bufs_head->buf,
>> +    lz4_size = lz4_encode(lz4, src->y, src->stride, lz4_data->data.bufs_head-
>> >buf.bytes,
>>                            sizeof(lz4_data->data.bufs_head->buf),
>>                            src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN, src-
>> >format);
>>
>> @@ -4461,8 +4464,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);
>> +                       quic_data->data.bufs_head->buf.words,
>> +                       G_N_ELEMENTS(quic_data->data.bufs_head->buf.words));
>>
>>      // the compressed buffer is bigger than the original data
>>      if ((size << 2) > (src->y * src->stride)) {
>
> Ack, thanks for the comments
>
> Pavel
>

Same here, ACK!
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]