Re: [PATCH 01/10] server: move display_channel_client_create() to dcc_new()

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

 



On Mon, Nov 2, 2015 at 11:02 AM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
>>
>> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
>>
>> Move function from server/red_worker.c to new server/display-channel.c.
>> ---
>>  server/Makefile.am       |  1 +
>>  server/display-channel.c | 38 +++++++++++++++++++++++++++
>>  server/display-channel.h | 50 ++++++++++++++++++++++++++++++++---
>>  server/red_worker.c      | 68
>>  +++---------------------------------------------
>>  4 files changed, 89 insertions(+), 68 deletions(-)
>>  create mode 100644 server/display-channel.c
>>
>> diff --git a/server/Makefile.am b/server/Makefile.am
>> index 87288cc..428417b 100644
>> --- a/server/Makefile.am
>> +++ b/server/Makefile.am
>> @@ -105,6 +105,7 @@ libspice_server_la_SOURCES =                      \
>>       red_parse_qxl.h                         \
>>       red_worker.c                            \
>>       red_worker.h                            \
>> +     display-channel.c                       \
>>       display-channel.h                       \
>>       cursor-channel.c                        \
>>       cursor-channel.h                        \
>> diff --git a/server/display-channel.c b/server/display-channel.c
>> new file mode 100644
>> index 0000000..00be665
>> --- /dev/null
>> +++ b/server/display-channel.c
>> @@ -0,0 +1,38 @@
>> +/*
>> +   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/>.
>> +*/
>> +#include "display-channel.h"
>> +
>> +DisplayChannelClient *dcc_new(DisplayChannel *display,
>> +                              RedClient *client, RedsStream *stream,
>> +                              int mig_target,
>> +                              uint32_t *common_caps, int num_common_caps,
>> +                              uint32_t *caps, int num_caps)
>> +{
>> +    DisplayChannelClient *dcc;
>> +
>> +    dcc = (DisplayChannelClient*)common_channel_new_client(
>> +        (CommonChannel *)display, sizeof(DisplayChannelClient),
>> +        client, stream, mig_target, TRUE,
>> +        common_caps, num_common_caps,
>> +        caps, num_caps);
>> +    spice_return_val_if_fail(dcc, NULL);
>> +
>> +    ring_init(&dcc->palette_cache_lru);
>> +    dcc->palette_cache_available = CLIENT_PALETTE_CACHE_SIZE;
>> +
>> +    return dcc;
>> +}
>> diff --git a/server/display-channel.h b/server/display-channel.h
>> index e1ddc11..5d2eee5 100644
>> --- a/server/display-channel.h
>> +++ b/server/display-channel.h
>> @@ -15,14 +15,47 @@
>>     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_WORKER_CLIENT_H_
>> -# define RED_WORKER_CLIENT_H_
>> +#ifndef DISPLAY_CHANNEL_H_
>> +# define DISPLAY_CHANNEL_H_
>> +
>> +#include <setjmp.h>
>>
>>  #include "red_worker.h"
>> +#include "reds_stream.h"
>>  #include "cache-item.h"
>>  #include "pixmap-cache.h"
>> +#ifdef USE_OPENGL
>> +#include "common/ogl_ctx.h"
>> +#include "reds_gl_canvas.h"
>> +#endif /* USE_OPENGL */
>> +#include "reds_sw_canvas.h"
>> +#include "glz_encoder_dictionary.h"
>> +#include "glz_encoder.h"
>> +#include "stat.h"
>> +#include "reds.h"
>> +#include "mjpeg_encoder.h"
>> +#include "red_memslots.h"
>> +#include "red_parse_qxl.h"
>> +#include "red_record_qxl.h"
>> +#include "jpeg_encoder.h"
>> +#ifdef USE_LZ4
>> +#include "lz4_encoder.h"
>> +#endif
>> +#include "demarshallers.h"
>> +#include "zlib_encoder.h"
>> +#include "red_channel.h"
>> +#include "red_dispatcher.h"
>> +#include "dispatcher.h"
>> +#include "main_channel.h"
>> +#include "migration_protocol.h"
>> +#include "main_dispatcher.h"
>> +#include "spice_server_utils.h"
>> +#include "spice_bitmap_utils.h"
>> +#include "spice_image_cache.h"
>>  #include "utils.h"
>>
>> +typedef struct DisplayChannel DisplayChannel;
>> +
>>  typedef struct Drawable Drawable;
>>
>>  #define PALETTE_CACHE_HASH_SHIFT 8
>> @@ -30,6 +63,8 @@ typedef struct Drawable Drawable;
>>  #define PALETTE_CACHE_HASH_MASK (PALETTE_CACHE_HASH_SIZE - 1)
>>  #define PALETTE_CACHE_HASH_KEY(id) ((id) & PALETTE_CACHE_HASH_MASK)
>>
>> +#define CLIENT_PALETTE_CACHE_SIZE 128
>> +
>>  /* Each drawable can refer to at most 3 images: src, brush and mask */
>>  #define MAX_DRAWABLE_PIXMAP_CACHE_ITEMS 3
>>
>> @@ -193,4 +228,13 @@ struct DisplayChannelClient {
>>      uint64_t streams_max_bit_rate;
>>  };
>>
>> -#endif /* RED_WORKER_CLIENT_H_ */
>> +DisplayChannelClient*      dcc_new
>> (DisplayChannel *display,
>> +
>> RedClient
>> *client,
>> +
>> RedsStream
>> *stream,
>> +                                                                      int
>> mig_target,
>> +
>> uint32_t
>> *common_caps,
>> +                                                                      int
>> num_common_caps,
>> +
>> uint32_t
>> *caps,
>> +                                                                      int
>> num_caps);
>> +
>> +#endif /* DISPLAY_CHANNEL_H_ */
>> diff --git a/server/red_worker.c b/server/red_worker.c
>> index 68cb153..89cb25c 100644
>> --- a/server/red_worker.c
>> +++ b/server/red_worker.c
>> @@ -43,7 +43,6 @@
>>  #include <poll.h>
>>  #include <pthread.h>
>>  #include <netinet/tcp.h>
>> -#include <setjmp.h>
>>  #include <openssl/ssl.h>
>>  #include <inttypes.h>
>>  #include <glib.h>
>> @@ -58,41 +57,11 @@
>>  #include "common/ring.h"
>>  #include "common/generated_server_marshallers.h"
>>
>> -#ifdef USE_OPENGL
>> -#include "common/ogl_ctx.h"
>> -#include "reds_gl_canvas.h"
>> -#endif /* USE_OPENGL */
>> +#include "display-channel.h"
>>
>>  #include "spice.h"
>>  #include "red_worker.h"
>> -#include "reds_stream.h"
>> -#include "reds_sw_canvas.h"
>> -#include "glz_encoder_dictionary.h"
>> -#include "glz_encoder.h"
>> -#include "stat.h"
>> -#include "reds.h"
>> -#include "mjpeg_encoder.h"
>> -#include "red_memslots.h"
>> -#include "red_parse_qxl.h"
>> -#include "red_record_qxl.h"
>> -#include "jpeg_encoder.h"
>> -#ifdef USE_LZ4
>> -#include "lz4_encoder.h"
>> -#endif
>> -#include "demarshallers.h"
>> -#include "zlib_encoder.h"
>> -#include "red_channel.h"
>> -#include "red_dispatcher.h"
>> -#include "dispatcher.h"
>> -#include "main_channel.h"
>> -#include "migration_protocol.h"
>>  #include "spice_timer_queue.h"
>> -#include "main_dispatcher.h"
>> -#include "spice_server_utils.h"
>> -#include "spice_bitmap_utils.h"
>> -#include "spice_image_cache.h"
>> -#include "pixmap-cache.h"
>> -#include "display-channel.h"
>>  #include "cursor-channel.h"
>>
>>  //#define COMPRESS_STAT
>> @@ -294,8 +263,6 @@ typedef struct StreamActivateReportItem {
>>  #define WIDE_CLIENT_ACK_WINDOW 40
>>  #define NARROW_CLIENT_ACK_WINDOW 20
>>
>> -#define CLIENT_PALETTE_CACHE_SIZE 128
>> -
>>  typedef struct ImageItem {
>>      PipeItem link;
>>      int refs;
>> @@ -311,8 +278,6 @@ typedef struct ImageItem {
>>      uint8_t data[0];
>>  } ImageItem;
>>
>> -typedef struct DisplayChannel DisplayChannel;
>> -
>>  enum {
>>      STREAM_FRAME_NONE,
>>      STREAM_FRAME_NATIVE,
>> @@ -9472,28 +9437,6 @@ CommonChannelClient
>> *common_channel_new_client(CommonChannel *common,
>>  }
>>
>>
>> -DisplayChannelClient *display_channel_client_create(CommonChannel *common,
>> -                                                    RedClient *client,
>> RedsStream *stream,
>> -                                                    int mig_target,
>> -                                                    uint32_t *common_caps,
>> int num_common_caps,
>> -                                                    uint32_t *caps, int
>> num_caps)
>> -{
>> -    DisplayChannelClient *dcc =
>> -        (DisplayChannelClient*)common_channel_new_client(
>> -            common, sizeof(DisplayChannelClient), client, stream,
>> -            mig_target,
>> -            TRUE,
>> -            common_caps, num_common_caps,
>> -            caps, num_caps);
>> -
>> -    if (!dcc) {
>> -        return NULL;
>> -    }
>> -    ring_init(&dcc->palette_cache_lru);
>> -    dcc->palette_cache_available = CLIENT_PALETTE_CACHE_SIZE;
>> -    return dcc;
>> -}
>> -
>>  RedChannel *red_worker_new_channel(RedWorker *worker, int size,
>>                                     uint32_t channel_type, int
>>                                     migration_flags,
>>                                     ChannelCbs *channel_cbs,
>> @@ -9776,13 +9719,8 @@ static void handle_new_display_channel(RedWorker
>> *worker, RedClient *client, Red
>>      }
>>      display_channel = worker->display_channel;
>>      spice_info("add display channel client");
>> -    dcc = display_channel_client_create(&display_channel->common, client,
>> stream,
>> -                                        migrate,
>> -                                        common_caps, num_common_caps,
>> -                                        caps, num_caps);
>> -    if (!dcc) {
>> -        return;
>> -    }
>
> The removal of this test does not make sense looking at same test in
> display-channel.c. If can be NULL inside dcc_new why not checking here?
> Or are we just going to accept the core in next lines?
>
> I would add again these lines and ack it.

Same feeling here. common_channel_new_client() can return NULL inside
the dcc_new(), so the check is still valid.

ACK keeping the check.

>
>> +    dcc = dcc_new(display_channel, client, stream, migrate,
>> +                  common_caps, num_common_caps, caps, num_caps);
>>      spice_info("New display (client %p) dcc %p stream %p", client, dcc,
>>      stream);
>>      stream_buf_size = 32*1024;
>>      dcc->send_data.stream_outbuf = spice_malloc(stream_buf_size);
>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
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]