[Spice-devel] DisplayChannel, Stream and StreamAgent redefinitions

Christophe Fergeau cfergeau at redhat.com
Tue Aug 30 15:36:10 UTC 2016


Hey,

For what it's worth, these 2 attached patches allow to compile on el6
(at least few weeks ago, did not try again just now).

Christophe

On Thu, Aug 18, 2016 at 06:38:12PM +0200, Francois Gouget wrote:
> 
> The commit below causes compilation errors on RHEL 6.8 :-(
> (gcc 4.4.7)
> 
>   CC     red-worker.lo
> In file included from display-channel.h:43,
>                  from red-worker.c:47:
> stream.h:46: error: redefinition of typedef 'DisplayChannel'
> dcc.h:44: note: previous declaration of 'DisplayChannel' was here
> stream.h:48: error: redefinition of typedef 'Stream'
> dcc.h:45: note: previous declaration of 'Stream' was here
> stream.h:89: error: redefinition of typedef 'StreamAgent'
> dcc.h:46: note: previous declaration of 'StreamAgent' was here
> 
> 
> 
> commit a84a433e0840d0eb65d4eb5c51f6cdde94a2c210
> Author: Jonathon Jongsma <jjongsma at redhat.com>
> Date:   Fri Aug 5 14:02:29 2016 -0500
> 
>     Limit direct access to DisplayChannelClient
>     
>     Add a few more methods and accessors so that other files don't need to
>     manipulate the struct members directly. Move the struct definition to a
>     private header which only the dcc-* files will include.
>     
>     Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
>     Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>     Acked-by: Frediano Ziglio <fziglio at redhat.com>
> 
> 
> Presumably that's temporary and can pretty much only be endured?
> 
> -- 
> Francois Gouget <fgouget at codeweavers.com>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
From 62c307ef54893b3c624e414ab4d48b6585abd704 Mon Sep 17 00:00:00 2001
From: Christophe Fergeau <cfergeau at redhat.com>
Date: Thu, 4 Aug 2016 09:51:57 +0200
Subject: [PATCH 1/2] fix RHEL6 compilation

Mostly GMutex API changes, and replacing g_queue_free_full()
---
 server/char-device.c       |    3 ++-
 server/gstreamer-encoder.c |   30 +++++++++++++++---------------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/server/char-device.c b/server/char-device.c
index 957fb26..d04c5a8 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -189,7 +189,8 @@ static void red_char_device_client_free(RedCharDevice *dev,
         dev_client->wait_for_tokens_timer = NULL;
     }
 
-    g_queue_free_full(dev_client->send_queue, (GDestroyNotify)red_pipe_item_unref);
+    g_queue_foreach(dev_client->send_queue, (GFunc)red_pipe_item_unref, NULL);
+    g_queue_free(dev_client->send_queue);
 
     /* remove write buffers that are associated with the client */
     spice_debug("write_queue_is_empty %d", ring_is_empty(&dev->priv->write_queue) && !dev->priv->cur_write_buf);
diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
index e6790af..86079a0 100644
--- a/server/gstreamer-encoder.c
+++ b/server/gstreamer-encoder.c
@@ -118,8 +118,8 @@ typedef struct SpiceGstEncoder {
     uint32_t set_pipeline;
 
     /* Output buffer */
-    GMutex outbuf_mutex;
-    GCond outbuf_cond;
+    GMutex *outbuf_mutex;
+    GCond *outbuf_cond;
     VideoBuffer *outbuf;
 
     /* The video bit rate. */
@@ -815,10 +815,10 @@ static GstBusSyncReply handle_pipeline_message(GstBus *bus, GstMessage *msg, gpo
         g_clear_error(&err);
 
         /* Unblock the main thread */
-        g_mutex_lock(&encoder->outbuf_mutex);
+        g_mutex_lock(encoder->outbuf_mutex);
         encoder->outbuf = (VideoBuffer*)create_gst_video_buffer();
-        g_cond_signal(&encoder->outbuf_cond);
-        g_mutex_unlock(&encoder->outbuf_mutex);
+        g_cond_signal(encoder->outbuf_cond);
+        g_mutex_unlock(encoder->outbuf_mutex);
     }
     return GST_BUS_PASS;
 }
@@ -848,10 +848,10 @@ static GstFlowReturn new_sample(GstAppSink *gstappsink, gpointer video_encoder)
 #endif
 
     /* Notify the main thread that the output buffer is ready */
-    g_mutex_lock(&encoder->outbuf_mutex);
+    g_mutex_lock(encoder->outbuf_mutex);
     encoder->outbuf = (VideoBuffer*)outbuf;
-    g_cond_signal(&encoder->outbuf_cond);
-    g_mutex_unlock(&encoder->outbuf_mutex);
+    g_cond_signal(encoder->outbuf_cond);
+    g_mutex_unlock(encoder->outbuf_mutex);
 
     return GST_FLOW_OK;
 }
@@ -1399,13 +1399,13 @@ static int push_raw_frame(SpiceGstEncoder *encoder,
 static int pull_compressed_buffer(SpiceGstEncoder *encoder,
                                   VideoBuffer **outbuf)
 {
-    g_mutex_lock(&encoder->outbuf_mutex);
+    g_mutex_lock(encoder->outbuf_mutex);
     while (!encoder->outbuf) {
-        g_cond_wait(&encoder->outbuf_cond, &encoder->outbuf_mutex);
+        g_cond_wait(encoder->outbuf_cond, encoder->outbuf_mutex);
     }
     *outbuf = encoder->outbuf;
     encoder->outbuf = NULL;
-    g_mutex_unlock(&encoder->outbuf_mutex);
+    g_mutex_unlock(encoder->outbuf_mutex);
 
     if ((*outbuf)->data) {
         return VIDEO_ENCODER_FRAME_ENCODE_DONE;
@@ -1425,8 +1425,8 @@ static void spice_gst_encoder_destroy(VideoEncoder *video_encoder)
     SpiceGstEncoder *encoder = (SpiceGstEncoder*)video_encoder;
 
     free_pipeline(encoder);
-    g_mutex_clear(&encoder->outbuf_mutex);
-    g_cond_clear(&encoder->outbuf_cond);
+    g_mutex_free(encoder->outbuf_mutex);
+    g_cond_free(encoder->outbuf_cond);
 
     /* Unref any lingering bitmap opaque structures from past frames */
     clear_zero_copy_queue(encoder, TRUE);
@@ -1711,8 +1711,8 @@ VideoEncoder *gstreamer_encoder_new(SpiceVideoCodecType codec_type,
     encoder->starting_bit_rate = starting_bit_rate;
     encoder->bitmap_ref = bitmap_ref;
     encoder->bitmap_unref = bitmap_unref;
-    g_mutex_init(&encoder->outbuf_mutex);
-    g_cond_init(&encoder->outbuf_cond);
+    encoder->outbuf_mutex = g_mutex_new();
+    encoder->outbuf_cond = g_cond_new();
 
     /* All the other fields are initialized to zero by spice_new0(). */
 
-- 
1.7.1

From af2d4454e5d452de036e1705f44eb80374045df5 Mon Sep 17 00:00:00 2001
From: Christophe Fergeau <cfergeau at redhat.com>
Date: Thu, 4 Aug 2016 09:57:25 +0200
Subject: [PATCH 2/2] Remove duplicate typedef

This fixes compilation on el6. This is achieved by moving the most problematic
typedef to a red-types.h header. Ideally, it will go away once we have put some
order in the header content/API/... For example, StreamAgent would be better as
an opaque type.
---
 server/Makefile.am           |    1 +
 server/image-cache.h         |    4 +---
 server/main-channel-client.h |    6 ++----
 server/main-channel.c        |    1 +
 server/main-channel.h        |    2 +-
 server/red-channel.h         |    2 +-
 server/red-types.h           |   26 ++++++++++++++++++++++++++
 server/red-worker.c          |    2 +-
 server/reds.c                |    1 +
 server/sound.c               |    1 +
 server/stream.c              |    3 ++-
 server/stream.h              |    3 ---
 12 files changed, 38 insertions(+), 14 deletions(-)
 create mode 100644 server/red-types.h

diff --git a/server/Makefile.am b/server/Makefile.am
index 921b082..8482e1c 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -150,6 +150,7 @@ libserver_la_SOURCES =				\
 	display-limits.h			\
 	image-encoders.c					\
 	image-encoders.h					\
+	red-types.h				\
 	$(NULL)
 
 if HAVE_LZ4
diff --git a/server/image-cache.h b/server/image-cache.h
index b90a3b3..c5ea40b 100644
--- a/server/image-cache.h
+++ b/server/image-cache.h
@@ -23,9 +23,7 @@
 #include <common/canvas_base.h>
 #include <common/ring.h>
 
-/* FIXME: move back to display-channel.h (once structs are private) */
-typedef struct Drawable Drawable;
-typedef struct DisplayChannelClient DisplayChannelClient;
+#include "red-types.h"
 
 typedef struct ImageCacheItem {
     RingItem lru_link;
diff --git a/server/main-channel-client.h b/server/main-channel-client.h
index c74f847..001cd67 100644
--- a/server/main-channel-client.h
+++ b/server/main-channel-client.h
@@ -17,11 +17,9 @@
 #ifndef __MAIN_CHANNEL_CLIENT_H__
 #define __MAIN_CHANNEL_CLIENT_H__
 
+#include "main-channel.h"
 #include "red-channel.h"
-
-/* FIXME: remove extra MainChannel typedef when possible */
-typedef struct MainChannel MainChannel;
-typedef struct MainChannelClient MainChannelClient;
+#include "red-types.h"
 
 MainChannelClient *main_channel_client_create(MainChannel *main_chan, RedClient *client,
                                               RedsStream *stream, uint32_t connection_id,
diff --git a/server/main-channel.c b/server/main-channel.c
index 42195e6..974dec3 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -23,6 +23,7 @@
 
 #include "red-common.h"
 #include "main-channel.h"
+#include "main-channel-client.h"
 #include "reds.h"
 
 int main_channel_is_connected(MainChannel *main_chan)
diff --git a/server/main-channel.h b/server/main-channel.h
index 868a14a..d4b63f6 100644
--- a/server/main-channel.h
+++ b/server/main-channel.h
@@ -23,7 +23,7 @@
 #include <common/marshaller.h>
 
 #include "red-channel.h"
-#include "main-channel-client.h"
+#include "red-types.h"
 
 // TODO: Defines used to calculate receive buffer size, and also by reds.c
 // other options: is to make a reds_main_consts.h, to duplicate defines.
diff --git a/server/red-channel.h b/server/red-channel.h
index 5b38f57..018c63c 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -33,6 +33,7 @@
 #include "reds-stream.h"
 #include "stat.h"
 #include "red-pipe-item.h"
+#include "red-types.h"
 
 #define MAX_SEND_BUFS 1000
 #define CLIENT_ACK_WINDOW 20
@@ -131,7 +132,6 @@ typedef struct OutgoingHandler {
 typedef struct RedChannel RedChannel;
 typedef struct RedChannelClient RedChannelClient;
 typedef struct RedClient RedClient;
-typedef struct MainChannelClient MainChannelClient;
 
 /* Messages handled by red_channel
  * SET_ACK - sent to client on channel connection
diff --git a/server/red-types.h b/server/red-types.h
new file mode 100644
index 0000000..a920074
--- /dev/null
+++ b/server/red-types.h
@@ -0,0 +1,26 @@
+/*
+    Copyright (C) 2016 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/>.
+*/
+
+#ifndef _H_RED_TYPES
+#define _H_RED_TYPES
+
+typedef struct MainChannelClient MainChannelClient;
+typedef struct DisplayChannelClient DisplayChannelClient;
+typedef struct DisplayChannel DisplayChannel;
+typedef struct Drawable Drawable;
+
+#endif
diff --git a/server/red-worker.c b/server/red-worker.c
index 9238632..b8e1ada 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -45,8 +45,8 @@
 #include <common/ring.h>
 
 #include "display-channel.h"
+#include "main-channel-client.h"
 #include "stream.h"
-
 #include "spice.h"
 #include "red-worker.h"
 #include "cursor-channel.h"
diff --git a/server/reds.c b/server/reds.c
index 6f88649..9ab917f 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -57,6 +57,7 @@
 #include "agent-msg-filter.h"
 #include "inputs-channel.h"
 #include "main-channel.h"
+#include "main-channel-client.h"
 #include "red-qxl.h"
 #include "main-dispatcher.h"
 #include "sound.h"
diff --git a/server/sound.c b/server/sound.c
index 8335101..3e6eec4 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -32,6 +32,7 @@
 #include "spice.h"
 #include "red-common.h"
 #include "main-channel.h"
+#include "main-channel-client.h"
 #include "reds.h"
 #include "red-qxl.h"
 #include "sound.h"
diff --git a/server/stream.c b/server/stream.c
index 96aca55..8d92895 100644
--- a/server/stream.c
+++ b/server/stream.c
@@ -18,8 +18,9 @@
 #include <config.h>
 #endif
 
-#include "stream.h"
 #include "display-channel.h"
+#include "main-channel-client.h"
+#include "stream.h"
 
 #define FPS_TEST_INTERVAL 1
 #define FOREACH_STREAMS(display, item)                  \
diff --git a/server/stream.h b/server/stream.h
index c40c188..812b8d1 100644
--- a/server/stream.h
+++ b/server/stream.h
@@ -42,9 +42,6 @@
 #define RED_STREAM_DEFAULT_LOW_START_BIT_RATE (2.5 * 1024 * 1024) // 2.5Mbps
 #define MAX_FPS 30
 
-/* move back to display_channel once struct private */
-typedef struct DisplayChannel DisplayChannel;
-
 typedef struct Stream Stream;
 
 typedef struct RedStreamActivateReportItem {
-- 
1.7.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160830/18c86eae/attachment.sig>


More information about the Spice-devel mailing list