[Spice-devel] [PATCH v2] server: factor out bitmap_fmt_is_rgb

Alon Levy alevy at redhat.com
Mon Jul 16 04:30:27 PDT 2012


On Mon, Jul 16, 2012 at 01:22:37PM +0200, Marc-André Lureau wrote:
> On Mon, Jul 16, 2012 at 10:40 AM, Alon Levy <alevy at redhat.com> wrote:
> > ---
> >  server/red_common.h    |   10 ++++++++++
> >  server/red_parse_qxl.c |    6 +-----
> >  server/red_worker.c    |   15 +++++++--------
> >  3 files changed, 18 insertions(+), 13 deletions(-)
> >
> > diff --git a/server/red_common.h b/server/red_common.h
> > index cb7bf71..7b14f9a 100644
> > --- a/server/red_common.h
> > +++ b/server/red_common.h
> > @@ -35,4 +35,14 @@ enum {
> >      STREAM_VIDEO_FILTER
> >  };
> >
> > +static inline int bitmap_fmt_is_rgb(uint8_t fmt)
> > +{
> > +    static const int BITMAP_FMT_IS_RGB[] = {0, 0, 0, 0, 0, 0, 1, 1, 1, 1};
> 
> It would be more readable to use the SPICE_BITMAP_FMT_xxx  as indexes,
> although VC doesn't like it iirc.

If you think it's not going to work it's good enough reason for me not
to check.

> 
> > +    if (fmt >= sizeof(BITMAP_FMT_IS_RGB)/sizeof(BITMAP_FMT_IS_RGB[0])) {
> > +        return 0;
> 
> You could also use SPICE_BITMAP_FMT_ENUM_END, or SPICE_N_ELEMENTS.

Ok.

> 
> Shouldn't it warn in this case?

Adding a warning.

> 
> > +    }
> > +    return BITMAP_FMT_IS_RGB[fmt];
> > +}
> > +
> >  #endif
> > diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
> > index bae9135..41759cf 100644
> > --- a/server/red_parse_qxl.c
> > +++ b/server/red_parse_qxl.c
> > @@ -328,10 +328,6 @@ static SpiceChunks *red_get_image_data_chunked(RedMemSlotInfo *slots, int group_
> >      return data;
> >  }
> >
> > -// This is based on SPICE_BITMAP_FMT_*, copied from server/red_worker.c
> > -// to avoid a possible unoptimization from making it non static.
> > -static const int BITMAP_FMT_IS_RGB[] = {0, 0, 0, 0, 0, 0, 1, 1, 1, 1};
> > -
> >  static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
> >                                   QXLPHYSICAL addr, uint32_t flags)
> >  {
> > @@ -366,7 +362,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
> >      switch (red->descriptor.type) {
> >      case SPICE_IMAGE_TYPE_BITMAP:
> >          red->u.bitmap.format = qxl->bitmap.format;
> > -        if (!BITMAP_FMT_IS_RGB[qxl->bitmap.format] && !qxl->bitmap.palette) {
> > +        if (!bitmap_fmt_is_rgb(qxl->bitmap.format) && !qxl->bitmap.palette) {
> >              spice_warning("guest error: missing palette on bitmap format=%d\n",
> >                            red->u.bitmap.format);
> >              return NULL;
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index e88e2ed..2400c3a 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -446,7 +446,6 @@ struct RedCompressBuf {
> >  };
> >
> >  static const int BITMAP_FMT_IS_PLT[] = {0, 1, 1, 1, 1, 1, 0, 0, 0, 0};
> > -static const int BITMAP_FMT_IS_RGB[] = {0, 0, 0, 0, 0, 0, 1, 1, 1, 1};
> >  static const int BITMAP_FMP_BYTES_PER_PIXEL[] = {0, 0, 0, 0, 0, 1, 2, 3, 4, 4};
> >
> >  pthread_mutex_t cache_lock = PTHREAD_MUTEX_INITIALIZER;
> > @@ -3112,7 +3111,7 @@ static inline void red_update_copy_graduality(RedWorker* worker, Drawable *drawa
> >
> >      bitmap = &drawable->red_drawable->u.copy.src_bitmap->u.bitmap;
> >
> > -    if (!BITMAP_FMT_IS_RGB[bitmap->format] || _stride_is_extra(bitmap) ||
> > +    if (!bitmap_fmt_is_rgb(bitmap->format) || _stride_is_extra(bitmap) ||
> >          (bitmap->data->flags & SPICE_CHUNKS_FLAGS_UNSTABLE)) {
> >          drawable->copy_bitmap_graduality = BITMAP_GRADUAL_NOT_AVAIL;
> >      } else  {
> > @@ -5850,7 +5849,7 @@ static BitmapGradualType _get_bitmap_graduality_level(RedWorker *worker, SpiceBi
> >  static inline int _stride_is_extra(SpiceBitmap *bitmap)
> >  {
> >      spice_assert(bitmap);
> > -    if (BITMAP_FMT_IS_RGB[bitmap->format]) {
> > +    if (bitmap_fmt_is_rgb(bitmap->format)) {
> >          return ((bitmap->x * BITMAP_FMP_BYTES_PER_PIXEL[bitmap->format]) < bitmap->stride);
> >      } else {
> >          switch (bitmap->format) {
> > @@ -5904,7 +5903,7 @@ static inline int red_glz_compress_image(DisplayChannelClient *dcc,
> >  #ifdef COMPRESS_STAT
> >      stat_time_t start_time = stat_now();
> >  #endif
> > -    spice_assert(BITMAP_FMT_IS_RGB[src->format]);
> > +    spice_assert(bitmap_fmt_is_rgb(src->format));
> >      GlzData *glz_data = &dcc->glz_data;
> >      ZlibData *zlib_data;
> >      LzImageType type = MAP_BITMAP_FMT_TO_LZ_IMAGE_TYPE[src->format];
> > @@ -6047,7 +6046,7 @@ static inline int red_lz_compress_image(DisplayChannelClient *dcc,
> >          longjmp(lz_data->data.jmp_env, 1);
> >      }
> >
> > -    if (BITMAP_FMT_IS_RGB[src->format]) {
> > +    if (bitmap_fmt_is_rgb(src->format)) {
> >          dest->descriptor.type = SPICE_IMAGE_TYPE_LZ_RGB;
> >          dest->u.lz_rgb.data_size = size;
> >
> > @@ -6346,7 +6345,7 @@ static inline int red_compress_image(DisplayChannelClient *dcc,
> >                      quic_compress = FALSE;
> >                  } else {
> >                      if (drawable->copy_bitmap_graduality == BITMAP_GRADUAL_INVALID) {
> > -                        quic_compress = BITMAP_FMT_IS_RGB[src->format] &&
> > +                        quic_compress = bitmap_fmt_is_rgb(src->format) &&
> >                              (_get_bitmap_graduality_level(display_channel->common.worker, src,
> >                                                            drawable->group_id) ==
> >                               BITMAP_GRADUAL_HIGH);
> > @@ -6381,7 +6380,7 @@ static inline int red_compress_image(DisplayChannelClient *dcc,
> >          int ret;
> >          if ((image_compression == SPICE_IMAGE_COMPRESS_AUTO_GLZ) ||
> >              (image_compression == SPICE_IMAGE_COMPRESS_GLZ)) {
> > -            glz = BITMAP_FMT_IS_RGB[src->format] && (
> > +            glz = bitmap_fmt_is_rgb(src->format) && (
> >                      (src->x * src->y) < glz_enc_dictionary_get_size(
> >                          dcc->glz_dict->dict));
> >          } else if ((image_compression == SPICE_IMAGE_COMPRESS_AUTO_LZ) ||
> > @@ -8454,7 +8453,7 @@ static void red_marshall_image(RedChannelClient *rcc, SpiceMarshaller *m, ImageI
> >
> >      if ((comp_mode == SPICE_IMAGE_COMPRESS_AUTO_LZ) ||
> >          (comp_mode == SPICE_IMAGE_COMPRESS_AUTO_GLZ)) {
> > -        if (BITMAP_FMT_IS_RGB[item->image_format]) {
> > +        if (bitmap_fmt_is_rgb(item->image_format)) {
> >              if (!_stride_is_extra(&bitmap)) {
> >                  BitmapGradualType grad_level;
> >                  grad_level = _get_bitmap_graduality_level(display_channel->common.worker,
> > --
> > 1.7.10.1
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> 
> 
> -- 
> Marc-André Lureau


More information about the Spice-devel mailing list