[Spice-devel] [PATCH 11/22] server: rename files

Jonathon Jongsma jjongsma at redhat.com
Wed Dec 2 12:57:16 PST 2015


Needs a new commit subject. 

On Wed, 2015-12-02 at 16:19 +0000, Frediano Ziglio wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> ---
>  NEWS                        |   2 +-
>  server/Makefile.am          |   9 +--
>  server/dcc-encoders.h       |   1 -
>  server/dispatcher.h         |   3 +-
>  server/display-channel.h    |   2 +-
>  server/glz-encoder-dict.c   |   1 +
>  server/glz-encoder-dict.h   |   4 +-
>  server/glz-encoder-priv.h   |  14 ++++
>  server/glz-encoder.h        |  22 +++++-
>  server/glz_encoder_config.h |  59 --------------
>  server/image-cache.h        |   2 +-
>  server/memslot.c            |   1 -
>  server/memslot.h            |   2 +-
>  server/red_common.h         |  17 +++-
>  server/red_dispatcher.h     |   2 -
>  server/red_worker.h         |   3 -
>  server/reds.c               |   1 -
>  server/reds.h               |   2 +
>  server/sound.c              |   2 +-
>  server/spice-bitmap-utils.c | 162 ++++++++++++++++++++++++++++++++++++++
>  server/spice-bitmap-utils.h |  15 ++--
>  server/spice_bitmap_utils.c | 188 -------------------------------------------
> -
>  server/spice_bitmap_utils.h |   8 --
>  server/sw-canvas.c          |   7 +-
>  server/sw-canvas.h          |   5 +-
>  server/utils.h              |   6 +-
>  26 files changed, 244 insertions(+), 296 deletions(-)
>  delete mode 100644 server/glz_encoder_config.h
>  delete mode 100644 server/spice_bitmap_utils.c
>  delete mode 100644 server/spice_bitmap_utils.h
> 
> diff --git a/NEWS b/NEWS
> index a33f6cf..c05f62a 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -101,7 +101,7 @@ Major changes in 0.11.3:
>   SPICE_MAIN_CAP_AGENT_CONNECTED_TOKENS
>   SPICE_MAIN_CAP_SEAMLESS_MIGRATE
>  * Misc:
> - * char_device.c: Introducing shared flow control code for char devices
> + * char-device.c: Introducing shared flow control code for char devices
>   * Enable build without client, cegui and slirp.
>  
>  Major changes in 0.11.0:
> diff --git a/server/Makefile.am b/server/Makefile.am
> index e964fb1..1e40610 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -71,7 +71,6 @@ libspice_server_la_SOURCES =			\
>  	demarshallers.h				\
>  	glz-encoder.c				\
>  	glz-encoder.h				\
> -	glz_encoder_config.h			\


It seems that the contents of glz_encoder_config.h are being moved into glz
-encoder.h and glz-encoder-dict.h. I'd prefer this change to be in its own
commit.


>  	glz-encoder-dict.c		\
>  	glz-encoder-dict.h		\
>  	glz-encoder-priv.h	\
> @@ -114,18 +113,16 @@ libspice_server_la_SOURCES =			\
>  	reds-private.h				\
>  	reds_stream.c				\
>  	reds_stream.h				\
> -	sw-canvas.c			\
> -	sw-canvas.h			\
>  	sound.c				\
>  	sound.h				\
> +	spice-experimental.h			\
> +	spice.h					\
>  	stat.h					\
>  	spicevmc.c				\
>  	spice_timer_queue.c			\
>  	spice_timer_queue.h			\
>  	zlib-encoder.c				\
>  	zlib-encoder.h				\
> -	spice_bitmap_utils.h		\
> -	spice_bitmap_utils.c		\

I mentioned in patch 10/22 that the rename of spice-bitmap-utils would fit
better in patch 10 than in this one. However, it appears that we're removing
spice_bitmap_utils.[ch] from the makefile, but we're not adding the renamed
spice-bitmap-utils.[ch]


>  	image-cache.h			\
>  	image-cache.c			\
>  	pixmap-cache.h				\
> @@ -144,6 +141,8 @@ libspice_server_la_SOURCES =			\
>  	display-limits.h			\
>  	dcc-encoders.c					\
>  	dcc-encoders.h					\
> +	sw-canvas.c					\
> +	sw-canvas.h					\
>  	$(NULL)
>  
>  if HAVE_SMARTCARD
> diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
> index a0a5c64..dc00ed1 100644
> --- a/server/dcc-encoders.h
> +++ b/server/dcc-encoders.h
> @@ -24,7 +24,6 @@
>  #include "red_channel.h"
>  #include "red_parse_qxl.h"
>  #include "image-cache.h"
> -#include "glz-encoder-dict.h"
>  #include "glz-encoder.h"
>  #include "jpeg-encoder.h"
>  #ifdef USE_LZ4
> diff --git a/server/dispatcher.h b/server/dispatcher.h
> index 353744a..0d3175f 100644
> --- a/server/dispatcher.h
> +++ b/server/dispatcher.h
> @@ -18,8 +18,7 @@
>  #ifndef DISPATCHER_H
>  #define DISPATCHER_H
>  
> -#include <spice.h>
> -#include "utils.h"
> +#include "red_common.h"
>  
>  typedef struct Dispatcher Dispatcher;
>  
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 50f6c5e..69b53bc 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -39,7 +39,7 @@
>  #include "main-channel.h"
>  #include "migration-protocol.h"
>  #include "main-dispatcher.h"
> -#include "spice_bitmap_utils.h"
> +#include "spice-bitmap-utils.h"

Move this to patch 10

>  #include "image-cache.h"
>  #include "utils.h"
>  #include "tree.h"
> diff --git a/server/glz-encoder-dict.c b/server/glz-encoder-dict.c
> index 1fd6753..af74736 100644
> --- a/server/glz-encoder-dict.c
> +++ b/server/glz-encoder-dict.c
> @@ -22,6 +22,7 @@
>  #include <string.h>
>  #include <stdio.h>
>  
> +#include "glz-encoder.h"
>  #include "glz-encoder-dict.h"
>  #include "glz-encoder-priv.h"
>  
> diff --git a/server/glz-encoder-dict.h b/server/glz-encoder-dict.h
> index 960f165..79b3781 100644
> --- a/server/glz-encoder-dict.h
> +++ b/server/glz-encoder-dict.h
> @@ -19,7 +19,6 @@
>  #define GLZ_ENCODER_DICT_H_
>  
>  #include <stdint.h>
> -#include "glz_encoder_config.h"
>  
>  /*
>      Interface for maintaining lz dictionary that is shared among several
> encoders.
> @@ -30,6 +29,9 @@
>  typedef void GlzEncDictContext;
>  typedef void GlzEncDictImageContext;
>  
> +typedef void GlzUsrImageContext;
> +typedef struct GlzEncoderUsrContext GlzEncoderUsrContext;
> +
>  /* NOTE: DISPLAY_MIGRATE_DATA_VERSION should change in case
> GlzEncDictRestoreData changes*/
>  typedef struct GlzEncDictRestoreData {
>      uint32_t size;
> diff --git a/server/glz-encoder-priv.h b/server/glz-encoder-priv.h
> index a408966..36f3878 100644
> --- a/server/glz-encoder-priv.h
> +++ b/server/glz-encoder-priv.h
> @@ -18,6 +18,8 @@
>  #ifndef GLZ_ENCODER_PRIV_H_
>  #define GLZ_ENCODER_PRIV_H_
>  
> +#include "red_common.h"
> +
>  /* Interface for using the dictionary for encoding.
>     Data structures are exposed for the encoder for efficiency
>     purposes. */
> @@ -183,4 +185,16 @@ void glz_dictionary_post_encode(uint32_t encoder_id,
> GlzEncoderUsrContext *usr,
>          (dict)->window.encoders_heads[enc_id]].pixels_so_far <= \
>          ref_seg->pixels_so_far)))
>  
> +#ifdef DEBUG
> +
> +#define GLZ_ASSERT(usr, x)                                              \
> +    if (!(x)) (usr)->error(usr, "%s: ASSERT %s failed\n", __FUNCTION__, #x);
> +
> +#else
> +
> +#define GLZ_ASSERT(usr, x)
> +
> +#endif
> +
> +
>  #endif // GLZ_ENCODER_PRIV_H_
> diff --git a/server/glz-encoder.h b/server/glz-encoder.h
> index 93164ed..fb4fce3 100644
> --- a/server/glz-encoder.h
> +++ b/server/glz-encoder.h
> @@ -20,10 +20,28 @@
>  
>  /* Manging the lz encoding using a dictionary that is shared among encoders
> */
>  
> -#include <stdint.h>
> +#include "red_common.h"
>  #include "common/lz_common.h"
>  #include "glz-encoder-dict.h"
> -#include "glz_encoder_config.h"
> +
> +struct GlzEncoderUsrContext {
> +    SPICE_GNUC_PRINTF(2, 3) void (*error)(GlzEncoderUsrContext *usr, const
> char *fmt, ...);
> +    SPICE_GNUC_PRINTF(2, 3) void (*warn)(GlzEncoderUsrContext *usr, const
> char *fmt, ...);
> +    SPICE_GNUC_PRINTF(2, 3) void (*info)(GlzEncoderUsrContext *usr, const
> char *fmt, ...);
> +    void    *(*malloc)(GlzEncoderUsrContext *usr, int size);
> +    void (*free)(GlzEncoderUsrContext *usr, void *ptr);
> +
> +    // get the next chunk of the image which is entered to the dictionary. If
> the image is down to
> +    // top, return it from the last line to the first one (stride should
> always be positive)
> +    int (*more_lines)(GlzEncoderUsrContext *usr, uint8_t **lines);
> +
> +    // get the next chunk of the compressed buffer.return number of bytes in
> the chunk.
> +    int (*more_space)(GlzEncoderUsrContext *usr, uint8_t **io_ptr);
> +
> +    // called when an image is removed from the dictionary, due to the window
> size limit
> +    void (*free_image)(GlzEncoderUsrContext *usr, GlzUsrImageContext *image);
> +
> +};
>  
>  typedef void GlzEncoderContext;
>  
> diff --git a/server/glz_encoder_config.h b/server/glz_encoder_config.h
> deleted file mode 100644
> index 6472668..0000000
> --- a/server/glz_encoder_config.h
> +++ /dev/null
> @@ -1,59 +0,0 @@
> -/*
> -   Copyright (C) 2009 Red Hat, Inc.
> -
> -   This library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   This library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with this library; if not, see <http://www.gnu.org/licenses/
> >.
> -*/
> -
> -#ifndef _H_GLZ_ENCODER_CONFIG
> -#define _H_GLZ_ENCODER_CONFIG
> -
> -#include <spice/macros.h>
> -#include "common/lz_common.h"
> -
> -typedef void GlzUsrImageContext;
> -typedef struct GlzEncoderUsrContext GlzEncoderUsrContext;
> -
> -struct GlzEncoderUsrContext {
> -    SPICE_GNUC_PRINTF(2, 3) void (*error)(GlzEncoderUsrContext *usr, const
> char *fmt, ...);
> -    SPICE_GNUC_PRINTF(2, 3) void (*warn)(GlzEncoderUsrContext *usr, const
> char *fmt, ...);
> -    SPICE_GNUC_PRINTF(2, 3) void (*info)(GlzEncoderUsrContext *usr, const
> char *fmt, ...);
> -    void    *(*malloc)(GlzEncoderUsrContext *usr, int size);
> -    void (*free)(GlzEncoderUsrContext *usr, void *ptr);
> -
> -    // get the next chunk of the image which is entered to the dictionary. If
> the image is down to
> -    // top, return it from the last line to the first one (stride should
> always be positive)
> -    int (*more_lines)(GlzEncoderUsrContext *usr, uint8_t **lines);
> -
> -    // get the next chunk of the compressed buffer.return number of bytes in
> the chunk.
> -    int (*more_space)(GlzEncoderUsrContext *usr, uint8_t **io_ptr);
> -
> -    // called when an image is removed from the dictionary, due to the window
> size limit
> -    void (*free_image)(GlzEncoderUsrContext *usr, GlzUsrImageContext *image);
> -
> -};
> -
> -
> -#ifdef DEBUG
> -
> -#define GLZ_ASSERT(usr, x) \
> -    if (!(x)) (usr)->error(usr, "%s: ASSERT %s failed\n", __FUNCTION__, #x);
> -
> -#else
> -
> -#define GLZ_ASSERT(usr, x)
> -
> -#endif
> -
> -
> -#endif
> diff --git a/server/image-cache.h b/server/image-cache.h
> index d66c7ff..014a45e 100644
> --- a/server/image-cache.h
> +++ b/server/image-cache.h
> @@ -24,7 +24,7 @@
>  #include "common/canvas_base.h"
>  #include "common/ring.h"
>  
> -/* FIXME: move back to display_channel.h (once structs are private) */
> +/* FIXME: move back to display-channel.h (once structs are private) */

This belongs in patch 10.

>  typedef struct Drawable Drawable;
>  typedef struct DisplayChannelClient DisplayChannelClient;
>  
> diff --git a/server/memslot.c b/server/memslot.c
> index e7ee04c..b7d56e0 100644
> --- a/server/memslot.c
> +++ b/server/memslot.c
> @@ -21,7 +21,6 @@
>  
>  #include <inttypes.h>
>  
> -#include "red_common.h"
>  #include "memslot.h"
>  
>  static unsigned long __get_clean_virt(RedMemSlotInfo *info, QXLPHYSICAL addr)
> diff --git a/server/memslot.h b/server/memslot.h
> index 1fba4b8..cc95662 100644
> --- a/server/memslot.h
> +++ b/server/memslot.h
> @@ -69,4 +69,4 @@ void memslot_info_add_slot(RedMemSlotInfo *info, uint32_t
> slot_group_id, uint32_
>  void memslot_info_del_slot(RedMemSlotInfo *info, uint32_t slot_group_id,
> uint32_t slot_id);
>  void memslot_info_reset(RedMemSlotInfo *info);
>  
> -#endif
> +#endif /* MEMSLOT_H_ */

this belongs in patch 10


> diff --git a/server/red_common.h b/server/red_common.h
> index 7f1677e..2d3977b 100644
> --- a/server/red_common.h
> +++ b/server/red_common.h
> @@ -18,15 +18,26 @@
>  #ifndef _H_RED_COMMON
>  #define _H_RED_COMMON
>  
> -#include <spice/macros.h>
> +#include <glib.h>
> +
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stdint.h>
>  #include <string.h>
> +#include <unistd.h>
>  
> +#include <spice/macros.h>
> +#include "common/log.h"
> +#include "common/lz_common.h"
> +#include "common/marshaller.h"
>  #include "common/mem.h"
> -#include "common/spice_common.h"
>  #include "common/messages.h"
> -#include "common/lz_common.h"
> +#include "common/ring.h"
> +#include "common/spice_common.h"
> +#include "common/draw.h"
>  
>  #include "spice.h"
> +#include "utils.h"
>  
>  #define SPICE_GNUC_VISIBLE __attribute__ ((visibility ("default")))
>  
> diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
> index 9f3474f..13c0cee 100644
> --- a/server/red_dispatcher.h
> +++ b/server/red_dispatcher.h
> @@ -18,8 +18,6 @@
>  #ifndef _H_RED_DISPATCHER
>  #define _H_RED_DISPATCHER
>  
> -#include <unistd.h>
> -#include <errno.h>
>  #include "red_channel.h"
>  
>  typedef struct RedDispatcher RedDispatcher;
> diff --git a/server/red_worker.h b/server/red_worker.h
> index a76c805..1e2c550 100644
> --- a/server/red_worker.h
> +++ b/server/red_worker.h
> @@ -18,9 +18,6 @@
>  #ifndef _H_REDWORKER
>  #define _H_REDWORKER
>  
> -#include <unistd.h>
> -#include <errno.h>
> -#include "utils.h"
>  #include "red_common.h"
>  #include "red_dispatcher.h"
>  #include "red_parse_qxl.h"
> diff --git a/server/reds.c b/server/reds.c
> index 5891034..509c346 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -58,7 +58,6 @@
>  #include "agent-msg-filter.h"
>  #include "inputs-channel.h"
>  #include "main-channel.h"
> -#include "red_common.h"
>  #include "red_dispatcher.h"
>  #include "main-dispatcher.h"
>  #include "sound.h"
> diff --git a/server/reds.h b/server/reds.h
> index 0584694..eca049b 100644
> --- a/server/reds.h
> +++ b/server/reds.h
> @@ -30,6 +30,8 @@
>  #include "red_channel.h"
>  #include "migration-protocol.h"
>  
> +#define SPICE_GNUC_VISIBLE __attribute__ ((visibility ("default")))
> +

Why is this added to reds.h? It is already defined in red_common.h


>  struct QXLState {
>      QXLInterface          *qif;
>      struct RedDispatcher  *dispatcher;
> diff --git a/server/sound.c b/server/sound.c
> index a91d56e..4b1ec7a 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -28,6 +28,7 @@
>  
>  #include "common/marshaller.h"
>  #include "common/generated_server_marshallers.h"
> +#include "common/snd_codec.h"
>  
>  #include "spice.h"
>  #include "red_common.h"
> @@ -35,7 +36,6 @@
>  #include "reds.h"
>  #include "red_dispatcher.h"
>  #include "sound.h"
> -#include "common/snd_codec.h"
>  #include "demarshallers.h"
>  
>  #ifndef IOV_MAX
> diff --git a/server/spice-bitmap-utils.c b/server/spice-bitmap-utils.c
> index 03d7694..8d6e7c6 100644
> --- a/server/spice-bitmap-utils.c
> +++ b/server/spice-bitmap-utils.c
> @@ -117,3 +117,165 @@ int bitmap_has_extra_stride(SpiceBitmap *bitmap)
>      }
>      return 0;
>  }
> +
> +int spice_bitmap_from_surface_type(uint32_t surface_format)
> +{
> +    switch (surface_format) {
> +    case SPICE_SURFACE_FMT_16_555:
> +        return SPICE_BITMAP_FMT_16BIT;
> +    case SPICE_SURFACE_FMT_32_xRGB:
> +        return SPICE_BITMAP_FMT_32BIT;
> +    case SPICE_SURFACE_FMT_32_ARGB:
> +        return SPICE_BITMAP_FMT_RGBA;
> +    case SPICE_SURFACE_FMT_8_A:
> +        return SPICE_BITMAP_FMT_8BIT_A;
> +    default:
> +        spice_critical("Unsupported surface format");
> +    }
> +    return 0;
> +}
> +
> +#define RAM_PATH "/tmp/tmpfs"
> +
> +static void dump_palette(FILE *f, SpicePalette* plt)
> +{
> +    int i;
> +    for (i = 0; i < plt->num_ents; i++) {
> +        fwrite(plt->ents + i, sizeof(uint32_t), 1, f);
> +    }
> +}
> +
> +static void dump_line(FILE *f, uint8_t* line, uint16_t n_pixel_bits, int
> width, int row_size)
> +{
> +    int i;
> +    int copy_bytes_size = SPICE_ALIGN(n_pixel_bits * width, 8) / 8;
> +
> +    fwrite(line, 1, copy_bytes_size, f);
> +    if (row_size > copy_bytes_size) {
> +        // each line should be 4 bytes aligned
> +        for (i = copy_bytes_size; i < row_size; i++) {
> +            fprintf(f, "%c", 0);
> +        }
> +    }
> +}
> +void dump_bitmap(SpiceBitmap *bitmap)
> +{
> +    static uint32_t file_id = 0;
> +
> +    char file_str[200];
> +    int rgb = TRUE;
> +    uint16_t n_pixel_bits;
> +    SpicePalette *plt = NULL;
> +    uint32_t id;
> +    int row_size;
> +    uint32_t file_size;
> +    int alpha = 0;
> +    uint32_t header_size = 14 + 40;
> +    uint32_t bitmap_data_offset;
> +    uint32_t tmp_u32;
> +    int32_t tmp_32;
> +    uint16_t tmp_u16;
> +    FILE *f;
> +    int i, j;
> +
> +    switch (bitmap->format) {
> +    case SPICE_BITMAP_FMT_1BIT_BE:
> +    case SPICE_BITMAP_FMT_1BIT_LE:
> +        rgb = FALSE;
> +        n_pixel_bits = 1;
> +        break;
> +    case SPICE_BITMAP_FMT_4BIT_BE:
> +    case SPICE_BITMAP_FMT_4BIT_LE:
> +        rgb = FALSE;
> +        n_pixel_bits = 4;
> +        break;
> +    case SPICE_BITMAP_FMT_8BIT:
> +        rgb = FALSE;
> +        n_pixel_bits = 8;
> +        break;
> +    case SPICE_BITMAP_FMT_16BIT:
> +        n_pixel_bits = 16;
> +        break;
> +    case SPICE_BITMAP_FMT_24BIT:
> +        n_pixel_bits = 24;
> +        break;
> +    case SPICE_BITMAP_FMT_32BIT:
> +        n_pixel_bits = 32;
> +        break;
> +    case SPICE_BITMAP_FMT_RGBA:
> +        n_pixel_bits = 32;
> +        alpha = 1;
> +        break;
> +    default:
> +        spice_error("invalid bitmap format  %u", bitmap->format);
> +        return;
> +    }
> +
> +    if (!rgb) {
> +        if (!bitmap->palette) {
> +            return; // dont dump masks.
> +        }
> +        plt = bitmap->palette;
> +    }
> +    row_size = (((bitmap->x * n_pixel_bits) + 31) / 32) * 4;
> +    bitmap_data_offset = header_size;
> +
> +    if (plt) {
> +        bitmap_data_offset += plt->num_ents * 4;
> +    }
> +    file_size = bitmap_data_offset + (bitmap->y * row_size);
> +
> +    id = ++file_id;
> +    sprintf(file_str, "%s/%u.bmp", RAM_PATH, id);
> +
> +    f = fopen(file_str, "wb");
> +    if (!f) {
> +        spice_error("Error creating bmp");
> +        return;
> +    }
> +
> +    /* writing the bmp v3 header */
> +    fprintf(f, "BM");
> +    fwrite(&file_size, sizeof(file_size), 1, f);
> +    tmp_u16 = alpha ? 1 : 0;
> +    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f); // reserved for application
> +    tmp_u16 = 0;
> +    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f);
> +    fwrite(&bitmap_data_offset, sizeof(bitmap_data_offset), 1, f);
> +    tmp_u32 = header_size - 14;
> +    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // sub header size
> +    tmp_32 = bitmap->x;
> +    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> +    tmp_32 = bitmap->y;
> +    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> +
> +    tmp_u16 = 1;
> +    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f); // color plane
> +    fwrite(&n_pixel_bits, sizeof(n_pixel_bits), 1, f); // pixel depth
> +
> +    tmp_u32 = 0;
> +    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // compression method
> +
> +    tmp_u32 = 0; //file_size - bitmap_data_offset;
> +    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // image size
> +    tmp_32 = 0;
> +    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> +    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> +    tmp_u32 = (!plt) ? 0 : plt->num_ents; // plt entries
> +    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
> +    tmp_u32 = 0;
> +    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
> +
> +    if (plt) {
> +        dump_palette(f, plt);
> +    }
> +    /* writing the data */
> +    for (i = 0; i < bitmap->data->num_chunks; i++) {
> +        SpiceChunk *chunk = &bitmap->data->chunk[i];
> +        int num_lines = chunk->len / bitmap->stride;
> +        for (j = 0; j < num_lines; j++) {
> +            dump_line(f, chunk->data + (j * bitmap->stride), n_pixel_bits,
> bitmap->x, row_size);
> +        }
> +    }
> +    fclose(f);
> +}
> diff --git a/server/spice-bitmap-utils.h b/server/spice-bitmap-utils.h
> index 38cb88a..beaa96f 100644
> --- a/server/spice-bitmap-utils.h
> +++ b/server/spice-bitmap-utils.h
> @@ -15,13 +15,10 @@
>     You should have received a copy of the GNU Lesser General Public
>     License along with this library; if not, see <http://www.gnu.org/licenses/
> >.
>  */
> -#ifndef RED_BITMAP_UTILS_H_
> -# define RED_BITMAP_UTILS_H_
> +#ifndef H_SPICE_BITMAP_UTILS
> +#define H_SPICE_BITMAP_UTILS
>  
> -#include <glib.h>
> -#include <stdint.h>
> -#include "common/draw.h"
> -#include "common/log.h"
> +#include "red_common.h"
>  
>  typedef enum {
>      BITMAP_GRADUAL_INVALID,
> @@ -88,4 +85,8 @@ static inline int bitmap_fmt_has_graduality(uint8_t fmt)
>  BitmapGradualType bitmap_get_graduality_level     (SpiceBitmap *bitmap);
>  int               bitmap_has_extra_stride         (SpiceBitmap *bitmap);
>  
> -#endif /* RED_BITMAP_UTILS_H_ */
> +void dump_bitmap(SpiceBitmap *bitmap);
> +
> +int spice_bitmap_from_surface_type(uint32_t surface_format);
> +
> +#endif
> diff --git a/server/spice_bitmap_utils.c b/server/spice_bitmap_utils.c
> deleted file mode 100644
> index ae3fc8b..0000000
> --- a/server/spice_bitmap_utils.c
> +++ /dev/null
> @@ -1,188 +0,0 @@
> -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> -/*
> -   Copyright (C) 2009-2015 Red Hat, Inc.
> -
> -   This library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   This library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with this library; if not, see <http://www.gnu.org/licenses/
> >.
> -*/
> -#ifdef HAVE_CONFIG_H
> -#include <config.h>
> -#endif
> -#include <stdio.h>
> -
> -#include "common/log.h"
> -#include "common/draw.h"
> -
> -#include "spice_bitmap_utils.h"
> -
> -int spice_bitmap_from_surface_type(uint32_t surface_format)
> -{
> -    switch (surface_format) {
> -    case SPICE_SURFACE_FMT_16_555:
> -        return SPICE_BITMAP_FMT_16BIT;
> -    case SPICE_SURFACE_FMT_32_xRGB:
> -        return SPICE_BITMAP_FMT_32BIT;
> -    case SPICE_SURFACE_FMT_32_ARGB:
> -        return SPICE_BITMAP_FMT_RGBA;
> -    case SPICE_SURFACE_FMT_8_A:
> -        return SPICE_BITMAP_FMT_8BIT_A;
> -    default:
> -        spice_critical("Unsupported surface format");
> -    }
> -    return 0;
> -}

OK, now I'm confused. it seems that we're moving all of this code from
spice_bitmap_utils.c to spice-bitmap-utils.c and getting rid of the former. So
until now, both spice_bitmap_utils.c and spice-bitmap-utils.c existed. that's
confusing. That also probably explains why we didn't add an entry for spice
-bitmap-utils.[ch] to the makefile as I mentioned above... I guess this should
all be moved to patch 10.


> -
> -#define RAM_PATH "/tmp/tmpfs"
> -
> -static void dump_palette(FILE *f, SpicePalette* plt)
> -{
> -    int i;
> -    for (i = 0; i < plt->num_ents; i++) {
> -        fwrite(plt->ents + i, sizeof(uint32_t), 1, f);
> -    }
> -}
> -
> -static void dump_line(FILE *f, uint8_t* line, uint16_t n_pixel_bits, int
> width, int row_size)
> -{
> -    int i;
> -    int copy_bytes_size = SPICE_ALIGN(n_pixel_bits * width, 8) / 8;
> -
> -    fwrite(line, 1, copy_bytes_size, f);
> -    if (row_size > copy_bytes_size) {
> -        // each line should be 4 bytes aligned
> -        for (i = copy_bytes_size; i < row_size; i++) {
> -            fprintf(f, "%c", 0);
> -        }
> -    }
> -}
> -void dump_bitmap(SpiceBitmap *bitmap)
> -{
> -    static uint32_t file_id = 0;
> -
> -    char file_str[200];
> -    int rgb = TRUE;
> -    uint16_t n_pixel_bits;
> -    SpicePalette *plt = NULL;
> -    uint32_t id;
> -    int row_size;
> -    uint32_t file_size;
> -    int alpha = 0;
> -    uint32_t header_size = 14 + 40;
> -    uint32_t bitmap_data_offset;
> -    uint32_t tmp_u32;
> -    int32_t tmp_32;
> -    uint16_t tmp_u16;
> -    FILE *f;
> -    int i, j;
> -
> -    switch (bitmap->format) {
> -    case SPICE_BITMAP_FMT_1BIT_BE:
> -    case SPICE_BITMAP_FMT_1BIT_LE:
> -        rgb = FALSE;
> -        n_pixel_bits = 1;
> -        break;
> -    case SPICE_BITMAP_FMT_4BIT_BE:
> -    case SPICE_BITMAP_FMT_4BIT_LE:
> -        rgb = FALSE;
> -        n_pixel_bits = 4;
> -        break;
> -    case SPICE_BITMAP_FMT_8BIT:
> -        rgb = FALSE;
> -        n_pixel_bits = 8;
> -        break;
> -    case SPICE_BITMAP_FMT_16BIT:
> -        n_pixel_bits = 16;
> -        break;
> -    case SPICE_BITMAP_FMT_24BIT:
> -        n_pixel_bits = 24;
> -        break;
> -    case SPICE_BITMAP_FMT_32BIT:
> -        n_pixel_bits = 32;
> -        break;
> -    case SPICE_BITMAP_FMT_RGBA:
> -        n_pixel_bits = 32;
> -        alpha = 1;
> -        break;
> -    default:
> -        spice_error("invalid bitmap format  %u", bitmap->format);
> -        return;
> -    }
> -
> -    if (!rgb) {
> -        if (!bitmap->palette) {
> -            return; // dont dump masks.
> -        }
> -        plt = bitmap->palette;
> -    }
> -    row_size = (((bitmap->x * n_pixel_bits) + 31) / 32) * 4;
> -    bitmap_data_offset = header_size;
> -
> -    if (plt) {
> -        bitmap_data_offset += plt->num_ents * 4;
> -    }
> -    file_size = bitmap_data_offset + (bitmap->y * row_size);
> -
> -    id = ++file_id;
> -    sprintf(file_str, "%s/%u.bmp", RAM_PATH, id);
> -
> -    f = fopen(file_str, "wb");
> -    if (!f) {
> -        spice_error("Error creating bmp");
> -        return;
> -    }
> -
> -    /* writing the bmp v3 header */
> -    fprintf(f, "BM");
> -    fwrite(&file_size, sizeof(file_size), 1, f);
> -    tmp_u16 = alpha ? 1 : 0;
> -    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f); // reserved for application
> -    tmp_u16 = 0;
> -    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f);
> -    fwrite(&bitmap_data_offset, sizeof(bitmap_data_offset), 1, f);
> -    tmp_u32 = header_size - 14;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // sub header size
> -    tmp_32 = bitmap->x;
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> -    tmp_32 = bitmap->y;
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> -
> -    tmp_u16 = 1;
> -    fwrite(&tmp_u16, sizeof(tmp_u16), 1, f); // color plane
> -    fwrite(&n_pixel_bits, sizeof(n_pixel_bits), 1, f); // pixel depth
> -
> -    tmp_u32 = 0;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // compression method
> -
> -    tmp_u32 = 0; //file_size - bitmap_data_offset;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f); // image size
> -    tmp_32 = 0;
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> -    fwrite(&tmp_32, sizeof(tmp_32), 1, f);
> -    tmp_u32 = (!plt) ? 0 : plt->num_ents; // plt entries
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
> -    tmp_u32 = 0;
> -    fwrite(&tmp_u32, sizeof(tmp_u32), 1, f);
> -
> -    if (plt) {
> -        dump_palette(f, plt);
> -    }
> -    /* writing the data */
> -    for (i = 0; i < bitmap->data->num_chunks; i++) {
> -        SpiceChunk *chunk = &bitmap->data->chunk[i];
> -        int num_lines = chunk->len / bitmap->stride;
> -        for (j = 0; j < num_lines; j++) {
> -            dump_line(f, chunk->data + (j * bitmap->stride), n_pixel_bits,
> bitmap->x, row_size);
> -        }
> -    }
> -    fclose(f);
> -}
> diff --git a/server/spice_bitmap_utils.h b/server/spice_bitmap_utils.h
> deleted file mode 100644
> index 69860e5..0000000
> --- a/server/spice_bitmap_utils.h
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -#ifndef H_SPICE_BITMAP_UTILS
> -#define H_SPICE_BITMAP_UTILS
> -
> -void dump_bitmap(SpiceBitmap *bitmap);
> -
> -int spice_bitmap_from_surface_type(uint32_t surface_format);
> -
> -#endif
> diff --git a/server/sw-canvas.c b/server/sw-canvas.c
> index 0ef050e..cba44ea 100644
> --- a/server/sw-canvas.c
> +++ b/server/sw-canvas.c
> @@ -1,3 +1,4 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
>  /*
>     Copyright (C) 2011 Red Hat, Inc.
>  
> @@ -17,10 +18,10 @@
>  #ifdef HAVE_CONFIG_H
>  #include <config.h>
>  #endif
> -
> -#include "common/spice_common.h"
> -
>  #include "sw-canvas.h"
> +
> +#define SPICE_CANVAS_INTERNAL
>  #define SW_CANVAS_IMAGE_CACHE
>  #include "common/sw_canvas.c"
>  #undef SW_CANVAS_IMAGE_CACHE
> +#undef SPICE_CANVAS_INTERNAL

I don't understand this change. As far as I can tell, the SPICE_CANVAS_INTERNAL
symbol is never used anywhere... I'd just drop it.


> diff --git a/server/sw-canvas.h b/server/sw-canvas.h
> index 5ffba3d..152fa0e 100644
> --- a/server/sw-canvas.h
> +++ b/server/sw-canvas.h
> @@ -1,3 +1,4 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
>  /*
>     Copyright (C) 2011 Red Hat, Inc.
>  
> @@ -17,8 +18,10 @@
>  #ifndef SW_CANVAS_H_
>  #define SW_CANVAS_H_
>  
> +#define SPICE_CANVAS_INTERNAL
>  #define SW_CANVAS_IMAGE_CACHE
>  #include "common/sw_canvas.h"
>  #undef SW_CANVAS_IMAGE_CACHE
> +#undef SPICE_CANVAS_INTERNAL

Same as previous comment.

>  
> -#endif
> +#endif /* SW_CANVAS_H_ */
> diff --git a/server/utils.h b/server/utils.h
> index aba85f9..20ebe00 100644
> --- a/server/utils.h
> +++ b/server/utils.h
> @@ -18,11 +18,9 @@
>  #ifndef UTILS_H_
>  # define UTILS_H_
>  
> -#include <glib.h>
> -#include <time.h>
> +#include "red_common.h"
>  
> -#include "common/ring.h"
> -#include "common/log.h"
> +#define SPICE_GNUC_VISIBLE __attribute__ ((visibility ("default")))

This is the second file that this SPICE_GNUC_VISIBLE symbol has been added to in
this patch. Why?


>  
>  static inline void set_bit(int index, uint32_t *addr)
>  {




General comment:

Aside from some things that I think should be moved back to patch 10, there are
basically two things being done in this patch:

- adding some more general includes (such as stdint.h) to red_common.h and
replacing these includes in other files with includes for red_common.h

- re-arranging some of the code in glz-encoder* and getting rid of
 glz_encoder_config.h

I think these could be split into 2 commits.

Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>


More information about the Spice-devel mailing list