[Spice-devel] [PATCH 11/19] server: move enum and struct away from red_common

[Fabiano Fidêncio]

On Thu, Nov 26, 2015 at 10:29 AM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
> On Wed, Nov 25, 2015 at 4:27 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
>> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>>
>> ---
>>  server/dcc.c            | 18 ++++++++++++++++--
>>  server/red_common.h     | 14 --------------
>>  server/red_dispatcher.c | 15 +++++++++++++--
>>  server/reds.c           |  2 +-
>>  server/stream.h         |  7 +++++++
>>  5 files changed, 37 insertions(+), 19 deletions(-)
>>
>> diff --git a/server/dcc.c b/server/dcc.c
>> index 6c089da..ffe5b34 100644
>> --- a/server/dcc.c
>> +++ b/server/dcc.c
>> @@ -616,6 +616,20 @@ static GlzDrawableInstanceItem *add_glz_drawable_instance(RedGlzDrawable *glz_dr
>>      return ret;
>>  }
>>
>> +static const LzImageType bitmap_fmt_to_lz_image_type[] = {
>> +    LZ_IMAGE_TYPE_INVALID,
>> +    LZ_IMAGE_TYPE_PLT1_LE,
>> +    LZ_IMAGE_TYPE_PLT1_BE,
>> +    LZ_IMAGE_TYPE_PLT4_LE,
>> +    LZ_IMAGE_TYPE_PLT4_BE,
>> +    LZ_IMAGE_TYPE_PLT8,
>> +    LZ_IMAGE_TYPE_RGB16,
>> +    LZ_IMAGE_TYPE_RGB24,
>> +    LZ_IMAGE_TYPE_RGB32,
>> +    LZ_IMAGE_TYPE_RGBA,
>> +    LZ_IMAGE_TYPE_A8
>> +};
>> +
>>  #define MIN_GLZ_SIZE_FOR_ZLIB 100
>>
>>  int dcc_compress_image_glz(DisplayChannelClient *dcc,
>> @@ -629,7 +643,7 @@ int dcc_compress_image_glz(DisplayChannelClient *dcc,
>>      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];
>> +    LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
>>      RedGlzDrawable *glz_drawable;
>>      GlzDrawableInstanceItem *glz_drawable_instance;
>>      int glz_size;
>> @@ -710,7 +724,7 @@ int dcc_compress_image_lz(DisplayChannelClient *dcc,
>>  {
>>      LzData *lz_data = &dcc->lz_data;
>>      LzContext *lz = dcc->lz;
>> -    LzImageType type = MAP_BITMAP_FMT_TO_LZ_IMAGE_TYPE[src->format];
>> +    LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
>>      int size;            // size of the compressed data
>>
>>  #ifdef COMPRESS_STAT
>> diff --git a/server/red_common.h b/server/red_common.h
>> index 04d4c02..7f1677e 100644
>> --- a/server/red_common.h
>> +++ b/server/red_common.h
>> @@ -30,18 +30,4 @@
>>
>>  #define SPICE_GNUC_VISIBLE __attribute__ ((visibility ("default")))
>>
>> -static const LzImageType MAP_BITMAP_FMT_TO_LZ_IMAGE_TYPE[] = {
>> -    LZ_IMAGE_TYPE_INVALID,
>> -    LZ_IMAGE_TYPE_PLT1_LE,
>> -    LZ_IMAGE_TYPE_PLT1_BE,
>> -    LZ_IMAGE_TYPE_PLT4_LE,
>> -    LZ_IMAGE_TYPE_PLT4_BE,
>> -    LZ_IMAGE_TYPE_PLT8,
>> -    LZ_IMAGE_TYPE_RGB16,
>> -    LZ_IMAGE_TYPE_RGB24,
>> -    LZ_IMAGE_TYPE_RGB32,
>> -    LZ_IMAGE_TYPE_RGBA,
>> -    LZ_IMAGE_TYPE_A8
>> -};
>> -
>>  #endif
>> diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
>> index a7825f5..952add9 100644
>> --- a/server/red_dispatcher.c
>> +++ b/server/red_dispatcher.c
>> @@ -36,6 +36,7 @@
>>  #include "reds.h"
>>  #include "dispatcher.h"
>>  #include "red_parse_qxl.h"
>> +#include "stream.h"
>>
>>  #include "red_dispatcher.h"
>>
>> @@ -702,9 +703,19 @@ static void qxl_worker_loadvm_commands(QXLWorker *qxl_worker,
>>      red_dispatcher_loadvm_commands((RedDispatcher*)qxl_worker, ext, count);
>>  }
>>
>> -static inline int calc_compression_level(void)
>> +void red_dispatcher_set_mm_time(uint32_t mm_time)
>>  {
>> -    spice_assert(streaming_video != SPICE_STREAM_VIDEO_INVALID);
>> +    RedDispatcher *now = dispatchers;
>> +    while (now) {
>> +        now->qxl->st->qif->set_mm_time(now->qxl, mm_time);
>> +        now = now->next;
>> +    }
>> +}
>> +
>> +static int calc_compression_level(void)
>> +{
>> +    spice_return_val_if_fail(streaming_video != SPICE_STREAM_VIDEO_INVALID, -1);
>> +
>>      if ((streaming_video != SPICE_STREAM_VIDEO_OFF) ||
>>          (image_compression != SPICE_IMAGE_COMPRESSION_QUIC)) {
>>          return 0;
>
> IMO, this hunk should not be part of this patch.
>
>> diff --git a/server/reds.c b/server/reds.c
>> index 8b3c3cb..bdea7e1 100644
>> --- a/server/reds.c
>> +++ b/server/reds.c
>> @@ -55,10 +55,10 @@
>>
>>  #include "spice.h"
>>  #include "reds.h"
>> +#include "stream.h"
>
> Neither this one ...
>
>>  #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 "snd_worker.h"
>> diff --git a/server/stream.h b/server/stream.h
>> index 7c589e4..30876f1 100644
>> --- a/server/stream.h
>> +++ b/server/stream.h
>> @@ -57,6 +57,13 @@ enum {
>>      STREAM_FRAME_CONTAINER,
>>  };
>>
>> +enum {
>> +    STREAM_VIDEO_INVALID,
>> +    STREAM_VIDEO_OFF,
>> +    STREAM_VIDEO_ALL,
>> +    STREAM_VIDEO_FILTER
>> +};
>> +
>
>
> Neither this one ...
> To be honest, I am not sure why it was created here but
> SPICE_STREAM_VIDEO_INVALID is still in use and wasn't removed.
>
>>  #define STREAM_STATS
>>  #ifdef STREAM_STATS
>>  typedef struct StreamStats {
>> --
>> 2.4.3
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
> Reviewed-by: Fabiano Fidêncio <fidencio at redhat.com>


And here is a proposed patch to replace this one:
http://paste.fedoraproject.org/294684/85303631

> --
> Fabiano Fidêncio
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel