[Spice-devel] [PATCH 01/10] server: move display_channel_client_create() to dcc_new()
Fabiano Fidêncio
fabiano at fidencio.org
Mon Nov 2 06:14:31 PST 2015
On Mon, Nov 2, 2015 at 2:50 PM, Fabiano Fidêncio <fidencio at redhat.com> wrote:
> 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.
Getting the ACK back. Would be better to have these whole series
rebased to the git master.
>
>>
>>> + 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
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Fabiano Fidêncio
More information about the Spice-devel
mailing list