[Spice-devel] [PATCH 01/10] server: move display_channel_client_create() to dcc_new()
Fabiano Fidêncio
fidencio at redhat.com
Mon Nov 2 05:50:49 PST 2015
On Mon, Nov 2, 2015 at 11:02 AM, Frediano Ziglio <fziglio at redhat.com> wrote:
>
>>
>> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>>
>> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list