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 3:14 PM, Fabiano Fidêncio <fabiano@xxxxxxxxxxxx> wrote:
> On Mon, Nov 2, 2015 at 2:50 PM, Fabiano Fidêncio <fidencio@xxxxxxxxxx> wrote:
>> 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.
>
> Getting the ACK back. Would be better to have these whole series
> rebased to the git master.

Okay, too late :-\

>
>>
>>>
>>>> +    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
>
>
>
> --
> Fabiano Fidêncio



-- 
Fabiano Fidêncio
_______________________________________________
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]