[Spice-devel] [PATCH 10/10] server: move bitmap related to red_bitmap_utils
Fabiano Fidêncio
fidencio at redhat.com
Mon Nov 2 09:17:43 PST 2015
On Mon, Nov 2, 2015 at 10:56 AM, Frediano Ziglio <fziglio at redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>
> ---
> server/Makefile.am | 2 +
> server/red_bitmap_utils.c | 99 +++++++++++++++++++++++++++++
> server/red_bitmap_utils.h | 91 ++++++++++++++++++++++++++
> server/red_common.h | 13 ----
> server/red_parse_qxl.c | 1 +
> server/red_worker.c | 158 ++++------------------------------------------
> server/tree.h | 9 +--
> 7 files changed, 206 insertions(+), 167 deletions(-)
> create mode 100644 server/red_bitmap_utils.c
> create mode 100644 server/red_bitmap_utils.h
>
> diff --git a/server/Makefile.am b/server/Makefile.am
> index d2a7343..5d28e9e 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -133,6 +133,8 @@ libspice_server_la_SOURCES = \
> pixmap-cache.c \
> tree.h \
> tree.c \
> + red_bitmap_utils.h \
> + red_bitmap_utils.c \
> utils.h \
> $(NULL)
>
> diff --git a/server/red_bitmap_utils.c b/server/red_bitmap_utils.c
> new file mode 100644
> index 0000000..d293dae
> --- /dev/null
> +++ b/server/red_bitmap_utils.c
> @@ -0,0 +1,99 @@
> +#include "red_bitmap_utils.h"
> +
> +#define RED_BITMAP_UTILS_RGB16
> +#include "red_bitmap_utils_tmpl.c"
> +#define RED_BITMAP_UTILS_RGB24
> +#include "red_bitmap_utils_tmpl.c"
> +#define RED_BITMAP_UTILS_RGB32
> +#include "red_bitmap_utils_tmpl.c"
> +
> +#define GRADUAL_HIGH_RGB24_TH -0.03
> +#define GRADUAL_HIGH_RGB16_TH 0
> +
> +// setting a more permissive threshold for stream identification in order
> +// not to miss streams that were artificially scaled on the guest (e.g., full screen view
> +// in window media player 12). see red_stream_add_frame
> +#define GRADUAL_MEDIUM_SCORE_TH 0.002
> +
> +// assumes that stride doesn't overflow
> +BitmapGradualType bitmap_get_graduality_level(SpiceBitmap *bitmap)
> +{
> + double score = 0.0;
> + int num_samples = 0;
> + int num_lines;
> + double chunk_score = 0.0;
> + int chunk_num_samples = 0;
> + uint32_t x, i;
> + SpiceChunk *chunk;
> +
> + chunk = bitmap->data->chunk;
> + for (i = 0; i < bitmap->data->num_chunks; i++) {
> + num_lines = chunk[i].len / bitmap->stride;
> + x = bitmap->x;
> + switch (bitmap->format) {
> + case SPICE_BITMAP_FMT_16BIT:
> + compute_lines_gradual_score_rgb16((rgb16_pixel_t *)chunk[i].data, x, num_lines,
> + &chunk_score, &chunk_num_samples);
> + break;
> + case SPICE_BITMAP_FMT_24BIT:
> + compute_lines_gradual_score_rgb24((rgb24_pixel_t *)chunk[i].data, x, num_lines,
> + &chunk_score, &chunk_num_samples);
> + break;
> + case SPICE_BITMAP_FMT_32BIT:
> + case SPICE_BITMAP_FMT_RGBA:
> + compute_lines_gradual_score_rgb32((rgb32_pixel_t *)chunk[i].data, x, num_lines,
> + &chunk_score, &chunk_num_samples);
> + break;
> + default:
> + spice_error("invalid bitmap format (not RGB) %u", bitmap->format);
> + }
> + score += chunk_score;
> + num_samples += chunk_num_samples;
> + }
> +
> + spice_assert(num_samples);
> + score /= num_samples;
> +
> + if (bitmap->format == SPICE_BITMAP_FMT_16BIT) {
> + if (score < GRADUAL_HIGH_RGB16_TH) {
> + return BITMAP_GRADUAL_HIGH;
> + }
> + } else {
> + if (score < GRADUAL_HIGH_RGB24_TH) {
> + return BITMAP_GRADUAL_HIGH;
> + }
> + }
> +
> + if (score < GRADUAL_MEDIUM_SCORE_TH) {
> + return BITMAP_GRADUAL_MEDIUM;
> + } else {
> + return BITMAP_GRADUAL_LOW;
> + }
> +}
> +
> +int bitmap_has_extra_stride(SpiceBitmap *bitmap)
> +{
> + spice_assert(bitmap);
> + if (bitmap_fmt_is_rgb(bitmap->format)) {
> + return ((bitmap->x * bitmap_fmt_get_bytes_per_pixel(bitmap->format)) < bitmap->stride);
> + } else {
> + switch (bitmap->format) {
> + case SPICE_BITMAP_FMT_8BIT:
> + return (bitmap->x < bitmap->stride);
> + case SPICE_BITMAP_FMT_4BIT_BE:
> + case SPICE_BITMAP_FMT_4BIT_LE: {
> + int bytes_width = SPICE_ALIGN(bitmap->x, 2) >> 1;
> + return bytes_width < bitmap->stride;
> + }
> + case SPICE_BITMAP_FMT_1BIT_BE:
> + case SPICE_BITMAP_FMT_1BIT_LE: {
> + int bytes_width = SPICE_ALIGN(bitmap->x, 8) >> 3;
> + return bytes_width < bitmap->stride;
> + }
> + default:
> + spice_error("invalid image type %u", bitmap->format);
> + return 0;
> + }
> + }
> + return 0;
> +}
> diff --git a/server/red_bitmap_utils.h b/server/red_bitmap_utils.h
> new file mode 100644
> index 0000000..38cb88a
> --- /dev/null
> +++ b/server/red_bitmap_utils.h
> @@ -0,0 +1,91 @@
> +/* -*- 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/>.
> +*/
> +#ifndef RED_BITMAP_UTILS_H_
> +# define RED_BITMAP_UTILS_H_
> +
> +#include <glib.h>
> +#include <stdint.h>
> +#include "common/draw.h"
> +#include "common/log.h"
> +
> +typedef enum {
> + BITMAP_GRADUAL_INVALID,
> + BITMAP_GRADUAL_NOT_AVAIL,
> + BITMAP_GRADUAL_LOW,
> + BITMAP_GRADUAL_MEDIUM,
> + BITMAP_GRADUAL_HIGH,
> +} BitmapGradualType;
> +
> +typedef struct {
> + uint8_t b;
> + uint8_t g;
> + uint8_t r;
> + uint8_t pad;
> +} rgb32_pixel_t;
> +
> +G_STATIC_ASSERT(sizeof(rgb32_pixel_t) == 4);
> +
> +typedef struct {
> + uint8_t b;
> + uint8_t g;
> + uint8_t r;
> +} rgb24_pixel_t;
> +
> +G_STATIC_ASSERT(sizeof(rgb24_pixel_t) == 3);
> +
> +typedef uint16_t rgb16_pixel_t;
> +
> +
> +static inline int bitmap_fmt_get_bytes_per_pixel(uint8_t fmt)
> +{
> + static const int bytes_per_pixel[] = {0, 0, 0, 0, 0, 1, 2, 3, 4, 4, 1};
> +
> + spice_return_val_if_fail(fmt < SPICE_N_ELEMENTS(bytes_per_pixel), 0);
> +
> + return bytes_per_pixel[fmt];
> +}
> +
> +
> +static inline int bitmap_fmt_is_plt(uint8_t fmt)
> +{
> + static const int fmt_is_plt[] = {0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0};
> +
> + spice_return_val_if_fail(fmt < SPICE_N_ELEMENTS(fmt_is_plt), 0);
> +
> + return fmt_is_plt[fmt];
> +}
> +
> +static inline int bitmap_fmt_is_rgb(uint8_t fmt)
> +{
> + static const int fmt_is_rgb[] = {0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1};
> +
> + spice_return_val_if_fail(fmt < SPICE_N_ELEMENTS(fmt_is_rgb), 0);
> +
> + return fmt_is_rgb[fmt];
> +}
> +
> +static inline int bitmap_fmt_has_graduality(uint8_t fmt)
> +{
> + return bitmap_fmt_is_rgb(fmt) && fmt != SPICE_BITMAP_FMT_8BIT_A;
> +}
> +
> +
> +BitmapGradualType bitmap_get_graduality_level (SpiceBitmap *bitmap);
> +int bitmap_has_extra_stride (SpiceBitmap *bitmap);
> +
> +#endif /* RED_BITMAP_UTILS_H_ */
> diff --git a/server/red_common.h b/server/red_common.h
> index 47e591d..04d4c02 100644
> --- a/server/red_common.h
> +++ b/server/red_common.h
> @@ -44,17 +44,4 @@ static const LzImageType MAP_BITMAP_FMT_TO_LZ_IMAGE_TYPE[] = {
> LZ_IMAGE_TYPE_A8
> };
>
> -static inline int bitmap_fmt_is_rgb(uint8_t fmt)
> -{
> - static const int BITMAP_FMT_IS_RGB[SPICE_BITMAP_FMT_ENUM_END] =
> - {0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1};
> -
> - if (fmt >= SPICE_BITMAP_FMT_ENUM_END) {
> - spice_warning("fmt >= SPICE_BITMAP_FMT_ENUM_END; %d >= %d",
> - fmt, SPICE_BITMAP_FMT_ENUM_END);
> - return 0;
> - }
> - return BITMAP_FMT_IS_RGB[fmt];
> -}
> -
> #endif
> diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
> index dd52602..dc449db 100644
> --- a/server/red_parse_qxl.c
> +++ b/server/red_parse_qxl.c
> @@ -23,6 +23,7 @@
> #include <inttypes.h>
> #include <glib.h>
> #include "common/lz_common.h"
> +#include "red_bitmap_utils.h"
> #include "red_common.h"
> #include "red_memslots.h"
> #include "red_parse_qxl.h"
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 0049cca..60cae01 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -311,13 +311,6 @@ typedef struct StreamClipItem {
> SpiceClipRects *rects;
> } StreamClipItem;
>
> -static const int BITMAP_FMT_IS_PLT[] = {0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0};
> -static const int BITMAP_FMP_BYTES_PER_PIXEL[] = {0, 0, 0, 0, 0, 1, 2, 3, 4, 4, 1};
> -
> -#define BITMAP_FMT_HAS_GRADUALITY(f) \
> - (bitmap_fmt_is_rgb(f) && \
> - ((f) != SPICE_BITMAP_FMT_8BIT_A))
> -
> typedef struct {
> QuicUsrContext usr;
> EncoderData data;
> @@ -614,10 +607,6 @@ static int red_display_free_some_independent_glz_drawables(DisplayChannelClient
> static void red_display_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawable);
> static ImageItem *red_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
> SpiceRect *area, PipeItem *pos, int can_lossy);
> -static BitmapGradualType _get_bitmap_graduality_level(RedWorker *worker, SpiceBitmap *bitmap,
> - uint32_t group_id);
> -static inline int _stride_is_extra(SpiceBitmap *bitmap);
> -
> static void display_channel_client_release_item_before_push(DisplayChannelClient *dcc,
> PipeItem *item);
> static void display_channel_client_release_item_after_push(DisplayChannelClient *dcc,
> @@ -2651,12 +2640,11 @@ static inline void red_update_copy_graduality(RedWorker* worker, Drawable *drawa
>
> bitmap = &drawable->red_drawable->u.copy.src_bitmap->u.bitmap;
>
> - if (!BITMAP_FMT_HAS_GRADUALITY(bitmap->format) || _stride_is_extra(bitmap) ||
> + if (!bitmap_fmt_has_graduality(bitmap->format) || bitmap_has_extra_stride(bitmap) ||
> (bitmap->data->flags & SPICE_CHUNKS_FLAGS_UNSTABLE)) {
> drawable->copy_bitmap_graduality = BITMAP_GRADUAL_NOT_AVAIL;
> } else {
> - drawable->copy_bitmap_graduality =
> - _get_bitmap_graduality_level(worker, bitmap,drawable->group_id);
> + drawable->copy_bitmap_graduality = bitmap_get_graduality_level(bitmap);
> }
> }
>
> @@ -4903,124 +4891,6 @@ static inline void red_init_zlib(RedWorker *worker)
> }
> }
>
> -typedef struct {
> - uint8_t b;
> - uint8_t g;
> - uint8_t r;
> - uint8_t pad;
> -} rgb32_pixel_t;
> -
> -G_STATIC_ASSERT(sizeof(rgb32_pixel_t) == 4);
> -
> -typedef struct {
> - uint8_t b;
> - uint8_t g;
> - uint8_t r;
> -} rgb24_pixel_t;
> -
> -G_STATIC_ASSERT(sizeof(rgb24_pixel_t) == 3);
> -
> -typedef uint16_t rgb16_pixel_t;
> -
> -#define RED_BITMAP_UTILS_RGB16
> -#include "red_bitmap_utils_tmpl.c"
> -#define RED_BITMAP_UTILS_RGB24
> -#include "red_bitmap_utils_tmpl.c"
> -#define RED_BITMAP_UTILS_RGB32
> -#include "red_bitmap_utils_tmpl.c"
> -
> -#define GRADUAL_HIGH_RGB24_TH -0.03
> -#define GRADUAL_HIGH_RGB16_TH 0
> -
> -// setting a more permissive threshold for stream identification in order
> -// not to miss streams that were artificially scaled on the guest (e.g., full screen view
> -// in window media player 12). see red_stream_add_frame
> -#define GRADUAL_MEDIUM_SCORE_TH 0.002
> -
> -// assumes that stride doesn't overflow
> -static BitmapGradualType _get_bitmap_graduality_level(RedWorker *worker, SpiceBitmap *bitmap,
> - uint32_t group_id)
> -{
> - double score = 0.0;
> - int num_samples = 0;
> - int num_lines;
> - double chunk_score = 0.0;
> - int chunk_num_samples = 0;
> - uint32_t x, i;
> - SpiceChunk *chunk;
> -
> - chunk = bitmap->data->chunk;
> - for (i = 0; i < bitmap->data->num_chunks; i++) {
> - num_lines = chunk[i].len / bitmap->stride;
> - x = bitmap->x;
> - switch (bitmap->format) {
> - case SPICE_BITMAP_FMT_16BIT:
> - compute_lines_gradual_score_rgb16((rgb16_pixel_t *)chunk[i].data, x, num_lines,
> - &chunk_score, &chunk_num_samples);
> - break;
> - case SPICE_BITMAP_FMT_24BIT:
> - compute_lines_gradual_score_rgb24((rgb24_pixel_t *)chunk[i].data, x, num_lines,
> - &chunk_score, &chunk_num_samples);
> - break;
> - case SPICE_BITMAP_FMT_32BIT:
> - case SPICE_BITMAP_FMT_RGBA:
> - compute_lines_gradual_score_rgb32((rgb32_pixel_t *)chunk[i].data, x, num_lines,
> - &chunk_score, &chunk_num_samples);
> - break;
> - default:
> - spice_error("invalid bitmap format (not RGB) %u", bitmap->format);
> - }
> - score += chunk_score;
> - num_samples += chunk_num_samples;
> - }
> -
> - spice_assert(num_samples);
> - score /= num_samples;
> -
> - if (bitmap->format == SPICE_BITMAP_FMT_16BIT) {
> - if (score < GRADUAL_HIGH_RGB16_TH) {
> - return BITMAP_GRADUAL_HIGH;
> - }
> - } else {
> - if (score < GRADUAL_HIGH_RGB24_TH) {
> - return BITMAP_GRADUAL_HIGH;
> - }
> - }
> -
> - if (score < GRADUAL_MEDIUM_SCORE_TH) {
> - return BITMAP_GRADUAL_MEDIUM;
> - } else {
> - return BITMAP_GRADUAL_LOW;
> - }
> -}
> -
> -static inline int _stride_is_extra(SpiceBitmap *bitmap)
> -{
> - spice_assert(bitmap);
> - if (bitmap_fmt_is_rgb(bitmap->format)) {
> - return ((bitmap->x * BITMAP_FMP_BYTES_PER_PIXEL[bitmap->format]) < bitmap->stride);
> - } else {
> - switch (bitmap->format) {
> - case SPICE_BITMAP_FMT_8BIT:
> - return (bitmap->x < bitmap->stride);
> - case SPICE_BITMAP_FMT_4BIT_BE:
> - case SPICE_BITMAP_FMT_4BIT_LE: {
> - int bytes_width = SPICE_ALIGN(bitmap->x, 2) >> 1;
> - return bytes_width < bitmap->stride;
> - }
> - case SPICE_BITMAP_FMT_1BIT_BE:
> - case SPICE_BITMAP_FMT_1BIT_LE: {
> - int bytes_width = SPICE_ALIGN(bitmap->x, 8) >> 3;
> - return bytes_width < bitmap->stride;
> - }
> - default:
> - spice_error("invalid image type %u", bitmap->format);
> - return 0;
> - }
> - }
> - return 0;
> -}
> -
> typedef struct compress_send_data_t {
> void* comp_buf;
> uint32_t comp_buf_size;
> @@ -5518,7 +5388,7 @@ static inline int red_compress_image(DisplayChannelClient *dcc,
> ((src->y * src->stride) < MIN_SIZE_TO_COMPRESS)) { // TODO: change the size cond
> return FALSE;
> } else if (image_compression == SPICE_IMAGE_COMPRESSION_QUIC) {
> - if (BITMAP_FMT_IS_PLT[src->format]) {
> + if (bitmap_fmt_is_plt(src->format)) {
> return FALSE;
> } else {
> quic_compress = TRUE;
> @@ -5528,11 +5398,11 @@ static inline int red_compress_image(DisplayChannelClient *dcc,
> lz doesn't handle (1) bitmaps with strides that are larger than the width
> of the image in bytes (2) unstable bitmaps
> */
> - if (_stride_is_extra(src) || (src->data->flags & SPICE_CHUNKS_FLAGS_UNSTABLE)) {
> + if (bitmap_has_extra_stride(src) || (src->data->flags & SPICE_CHUNKS_FLAGS_UNSTABLE)) {
> if ((image_compression == SPICE_IMAGE_COMPRESSION_LZ) ||
> (image_compression == SPICE_IMAGE_COMPRESSION_GLZ) ||
> (image_compression == SPICE_IMAGE_COMPRESSION_LZ4) ||
> - BITMAP_FMT_IS_PLT[src->format]) {
> + bitmap_fmt_is_plt(src->format)) {
> return FALSE;
> } else {
> quic_compress = TRUE;
> @@ -5544,10 +5414,8 @@ static inline int red_compress_image(DisplayChannelClient *dcc,
> quic_compress = FALSE;
> } else {
> if (drawable->copy_bitmap_graduality == BITMAP_GRADUAL_INVALID) {
> - quic_compress = BITMAP_FMT_HAS_GRADUALITY(src->format) &&
> - (_get_bitmap_graduality_level(display_channel->common.worker, src,
> - drawable->group_id) ==
> - BITMAP_GRADUAL_HIGH);
> + quic_compress = bitmap_fmt_has_graduality(src->format) &&
> + bitmap_get_graduality_level(src) == BITMAP_GRADUAL_HIGH;
> } else {
> quic_compress = (drawable->copy_bitmap_graduality == BITMAP_GRADUAL_HIGH);
> }
> @@ -5567,7 +5435,7 @@ static inline int red_compress_image(DisplayChannelClient *dcc,
> ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> (image_compression == SPICE_IMAGE_COMPRESSION_AUTO_GLZ))) {
> // if we use lz for alpha, the stride can't be extra
> - if (src->format != SPICE_BITMAP_FMT_RGBA || !_stride_is_extra(src)) {
> + if (src->format != SPICE_BITMAP_FMT_RGBA || !bitmap_has_extra_stride(src)) {
> return red_jpeg_compress_image(dcc, dest,
> src, o_comp_data, drawable->group_id);
> }
> @@ -5579,7 +5447,7 @@ static inline int red_compress_image(DisplayChannelClient *dcc,
> int ret;
> if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_GLZ) ||
> (image_compression == SPICE_IMAGE_COMPRESSION_GLZ)) {
> - glz = BITMAP_FMT_HAS_GRADUALITY(src->format) && (
> + glz = bitmap_fmt_has_graduality(src->format) && (
> (src->x * src->y) < glz_enc_dictionary_get_size(
> dcc->glz_dict->dict));
> } else if ((image_compression == SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> @@ -7835,14 +7703,12 @@ static void red_marshall_image(RedChannelClient *rcc, SpiceMarshaller *m, ImageI
> comp_mode = display_channel->common.worker->image_compression;
>
> if (((comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_LZ) ||
> - (comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_GLZ)) && !_stride_is_extra(&bitmap)) {
> + (comp_mode == SPICE_IMAGE_COMPRESSION_AUTO_GLZ)) && !bitmap_has_extra_stride(&bitmap)) {
>
> - if (BITMAP_FMT_HAS_GRADUALITY(item->image_format)) {
> + if (bitmap_fmt_has_graduality(item->image_format)) {
> BitmapGradualType grad_level;
>
> - grad_level = _get_bitmap_graduality_level(display_channel->common.worker,
> - &bitmap,
> - worker->mem_slots.internal_groupslot_id);
> + grad_level = bitmap_get_graduality_level(&bitmap);
> if (grad_level == BITMAP_GRADUAL_HIGH) {
> // if we use lz for alpha, the stride can't be extra
> lossy_comp = display_channel->enable_jpeg && item->can_lossy;
> diff --git a/server/tree.h b/server/tree.h
> index 8cd7b05..e3b957f 100644
> --- a/server/tree.h
> +++ b/server/tree.h
> @@ -21,6 +21,7 @@
> #include <stdint.h>
> #include "common/region.h"
> #include "common/ring.h"
> +#include "red_bitmap_utils.h"
>
> enum {
> TREE_ITEM_TYPE_NONE,
> @@ -64,14 +65,6 @@ struct DrawItem {
>
> #define IS_DRAW_ITEM(item) ((item)->type == TREE_ITEM_TYPE_DRAWABLE)
>
> -typedef enum {
> - BITMAP_GRADUAL_INVALID,
> - BITMAP_GRADUAL_NOT_AVAIL,
> - BITMAP_GRADUAL_LOW,
> - BITMAP_GRADUAL_MEDIUM,
> - BITMAP_GRADUAL_HIGH,
> -} BitmapGradualType;
> -
> typedef struct DependItem {
> Drawable *drawable;
> RingItem ring_item;
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
ACK!
More information about the Spice-devel
mailing list