[Spice-devel] [PATCH spice 6/11] server: Add VP8 support, a video codec preference list and compatibility checks with the Spice client.
@@ void red_dispatcher_on_sv_change(void)
}
}
+void red_dispatcher_on_vc_change(void)
+{
+RedWorkerMessageSetVideoCodecs payload;
+int compression_level = calc_compression_level();
+RedDispatcher *now = dispatchers;
+while (now) {
+now-qxl-st-qif-set_compression_level(now-qxl, compression_level);
+payload.num_video_codecs = num_video_codecs;
+payload.video_codecs = video_codecs;
+dispatcher_send_message(now-dispatcher,
+RED_WORKER_MESSAGE_SET_VIDEO_CODECS,
+payload);
+now = now-next;
+}
+}
+
void red_dispatcher_set_mouse_mode(uint32_t mode)
{
RedWorkerMessageSetMouseMode payload;
@@ -1084,6 +1209,8 @@ void red_dispatcher_init(QXLInstance *qxl)
init_data.pending = red_dispatcher-pending;
init_data.num_renderers = num_renderers;
memcpy(init_data.renderers, renderers, sizeof(init_data.renderers));
+init_data.num_video_codecs = num_video_codecs;
+memcpy(init_data.video_codecs, video_codecs,
sizeof(init_data.video_codecs));
pthread_mutex_init(red_dispatcher-async_lock, NULL);
init_data.image_compression = image_compression;
diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
index 907b7c7..eb3aee4 100644
--- a/server/red_dispatcher.h
+++ b/server/red_dispatcher.h
@@ -22,6 +22,7 @@
struct RedChannelClient;
struct RedDispatcher;
+typedef struct RedVideoCodec RedVideoCodec;
typedef struct AsyncCommand AsyncCommand;
void red_dispatcher_init(QXLInstance *qxl);
@@ -29,11 +30,13 @@ void red_dispatcher_init(QXLInstance *qxl);
void red_dispatcher_set_mm_time(uint32_t);
void red_dispatcher_on_ic_change(void);
void red_dispatcher_on_sv_change(void);
+void red_dispatcher_on_vc_change(void);
void red_dispatcher_set_mouse_mode(uint32_t mode);
void red_dispatcher_on_vm_stop(void);
void red_dispatcher_on_vm_start(void);
int red_dispatcher_count(void);
int red_dispatcher_add_renderer(const char *name);
+int red_dispatcher_set_video_codecs(const char* codecs);
uint32_t red_dispatcher_qxl_ram_size(void);
int red_dispatcher_qxl_count(void);
void red_dispatcher_async_complete(struct RedDispatcher *, AsyncCommand *);
@@ -174,6 +177,11 @@ typedef struct RedWorkerMessageSetStreamingVideo {
uint32_t streaming_video;
} RedWorkerMessageSetStreamingVideo;
+typedef struct RedWorkerMessageSetVideoCodecs {
+uint32_t num_video_codecs;
+RedVideoCodec* video_codecs;
+} RedWorkerMessageSetVideoCodecs;
+
typedef struct RedWorkerMessageSetMouseMode {
uint32_t mode;
} RedWorkerMessageSetMouseMode;
diff --git a/server/red_worker.c b/server/red_worker.c
index 19e27c5..8e94e26 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1,6 +1,7 @@
/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
/*
Copyright (C) 2009 Red Hat, Inc.
+ Copyright (C) 2015 Francois Gouget
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
@@ -994,6 +995,8 @@ typedef struct RedWorker {
uint32_t mouse_mode;
uint32_t streaming_video;
+uint32_t num_video_codecs;
+RedVideoCodec video_codecs[RED_MAX_VIDEO_CODECS];
Stream streams_buf[NUM_STREAMS];
Stream *free_streams;
Ring streams;
@@ -3074,10 +3077,41 @@ static void
red_stream_update_client_playback_latency(void *opaque, uint32_t del
main_dispatcher_set_mm_time_latency(agent-dcc-common.base.client,
agent-dcc-streams_max_latency);
}
+static VideoEncoder* red_display_create_video_encoder(DisplayChannelClient
*dcc, uint64_t starting_bit_rate, VideoEncoderRateControlCbs *cbs, void
*cbs_opaque)
+{
+RedWorker* worker = dcc-common.worker;
+int i;
+int client_has_multi_codec =
red_channel_client_test_remote_cap(dcc-common.base,
SPICE_DISPLAY_CAP_MULTI_CODEC);
+
+for (i = 0; i worker-num_video_codecs; i++)
+{
+RedVideoCodec* video_codec = worker-video_codecs[i];
+VideoEncoder* video_encoder;
+
+if (!client_has_multi_codec
+video_codec-type != SPICE_VIDEO_CODEC_TYPE_MJPEG)
+/* Old clients only support MJPEG */
+continue;
+
+if (client_has_multi_codec
!red_channel_client_test_remote_cap(dcc-common.base, video_codec-cap))
+/* The client is recent but does not support this codec */
+continue;
+
+video_encoder = video_codec-create(video_codec-type,
starting_bit_rate, cbs, cbs_opaque);
+if (video_encoder)
+return video_encoder;
+}
+
+/* Try to use the builtin MJPEG video encoder as a fallback */
+if (!client_has_multi_codec ||
red_channel_client_test_remote_cap(dcc-common.base,
SPICE_DISPLAY_CAP_CODEC_MJPEG))
+return create_mjpeg_encoder(SPICE_VIDEO_CODEC_TYPE_MJPEG,
starting_bit_rate, cbs, cbs_opaque);
+
+return NULL;
+}
+
static void red_display_create_stream(DisplayChannelClient *dcc, Stream
[Spice-devel] [PATCH qxl 7/11] Add SpiceVideoCodecs and $XSPICE_VIDEO_CODECS for specifying video codec preferences.
---
examples/spiceqxl.xorg.conf.example | 7 +++
src/qxl.h | 1 +
src/qxl_driver.c| 2 ++
src/spiceqxl_spice_server.c | 16
4 files changed, 26 insertions(+)
diff --git a/examples/spiceqxl.xorg.conf.example
b/examples/spiceqxl.xorg.conf.example
index d15f7f2..a82c2be 100644
--- a/examples/spiceqxl.xorg.conf.example
+++ b/examples/spiceqxl.xorg.conf.example
@@ -51,6 +51,13 @@ Section Device
# defaults to filter.
#Option SpiceStreamingVideo
+# Set video codecs to use. Provide a semicolon list of
+# codecs, in preference order. Each codec requires an encoder
+# which can be one of spice or gstreamer, and then a codec type,
+# for instance mjpeg or vp8. The default is spice:mjpeg,
+# which uses the builtin mjpeg encoder.
+#Option SpiceVideoCodecs
+
# Set zlib glz wan compression. Options are auto, never, always.
# defaults to auto.
#Option SpiceZlibGlzWanCompression
diff --git a/src/qxl.h b/src/qxl.h
index f46bc58..14449b8 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -158,6 +158,7 @@ enum {
OPTION_SURFACE_BUFFER_SIZE,
OPTION_COMMAND_BUFFER_SIZE,
OPTION_SPICE_SMARTCARD_FILE,
+OPTION_SPICE_VIDEO_CODECS,
#endif
OPTION_COUNT,
};
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index 942067f..88b38db 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -155,6 +155,8 @@ const OptionInfoRec DefaultOptions[] =
CommandBufferSize,OPTV_INTEGER,
{DEFAULT_COMMAND_BUFFER_SIZE}, FALSE},
{ OPTION_SPICE_SMARTCARD_FILE,
SpiceSmartcardFile, OPTV_STRING,{0}, FALSE},
+{ OPTION_SPICE_VIDEO_CODECS,
+ SpiceVideoCodecs, OPTV_STRING,{0}, FALSE},
#endif
{ -1, NULL, OPTV_NONE, {0}, FALSE }
diff --git a/src/spiceqxl_spice_server.c b/src/spiceqxl_spice_server.c
index 14ee752..d74b29d 100644
--- a/src/spiceqxl_spice_server.c
+++ b/src/spiceqxl_spice_server.c
@@ -173,6 +173,9 @@ void xspice_set_spice_server_options(OptionInfoPtr options)
const char *streaming_video =
get_str_option(options, OPTION_SPICE_STREAMING_VIDEO,
XSPICE_STREAMING_VIDEO);
+const char *video_codecs =
+get_str_option(options, OPTION_SPICE_VIDEO_CODECS,
+ XSPICE_VIDEO_CODECS);
int agent_mouse =
get_bool_option(options, OPTION_SPICE_AGENT_MOUSE,
XSPICE_AGENT_MOUSE);
@@ -295,6 +298,19 @@ void xspice_set_spice_server_options(OptionInfoPtr options)
spice_server_set_streaming_video(spice_server, streaming_video_opt);
}
+if (video_codecs) {
+#if SPICE_SERVER_VERSION = 0x000c06 /* 0.12.6 */
+if (spice_server_set_video_codecs(spice_server, video_codecs))
+{
+fprintf(stderr, spice: invalid video encoder %s\n, video_codecs);
+exit(1);
+}
+#else
+fprintf(stderr, spice: video_codecs are not available (spice =
0.12.6 required)\n);
+exit(1);
+#endif
+}
+
spice_server_set_agent_mouse
(spice_server, agent_mouse);
spice_server_set_playback_compression
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH protocol 5/11] Add support for the VP8 video codec and for advertising supported video codecs.
Clients that support other codecs besides MJPEG should advertise the
SPICE_DISPLAY_CAP_MULTI_CODEC capability and one SPICE_DISPLAY_CAP_CODEC_XXX
per supported codec.
---
spice/enums.h| 1 +
spice/protocol.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/spice/enums.h b/spice/enums.h
index 18e2f74..14675f9 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -139,6 +139,7 @@ typedef enum SpicePathFlags {
typedef enum SpiceVideoCodecType {
SPICE_VIDEO_CODEC_TYPE_MJPEG = 1,
+SPICE_VIDEO_CODEC_TYPE_VP8,
SPICE_VIDEO_CODEC_TYPE_ENUM_END
} SpiceVideoCodecType;
diff --git a/spice/protocol.h b/spice/protocol.h
index bea376c..7f0b322 100644
--- a/spice/protocol.h
+++ b/spice/protocol.h
@@ -134,6 +134,9 @@ enum {
SPICE_DISPLAY_CAP_A8_SURFACE,
SPICE_DISPLAY_CAP_STREAM_REPORT,
SPICE_DISPLAY_CAP_LZ4_COMPRESSION,
+SPICE_DISPLAY_CAP_MULTI_CODEC,
+SPICE_DISPLAY_CAP_CODEC_MJPEG,
+SPICE_DISPLAY_CAP_CODEC_VP8,
};
enum {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support
This is a followup on Jeremy White's concept patch to use GStreamer: http://www.spinics.net/lists/spice-devel/msg10030.html There is still a lot of work ahead but this patches series has all the infrastructure so we're sending it out to verify that it looks ok. First, with this patch series: 1. You can use the SpiceVideoCodecs option in the xorg.conf file (or $XSPICE_VIDEO_CODECS) to set your video encoder and codec preferences. The default is still the builtin MJPEG encoder. 2. You can do the same from QEmu by setting the video-codecs option. 3. Clients can expose new display capabilities to advertise that they support either or both of the MJPEG and VP8 codecs. The new server will automatically pick a codec they support based on the above preference list. Older clients that don't advertise these new capabilities will automatically get an MJPEG stream. So old and new clients and servers should all be compatible. 4. The GTK and and HTML5 clients have been updated to support both MJPEG and VP8. 5. Using GStreamer as the video encoder should work well for either codecs as long as the stream does not hit your network connection's bandwidth limit. Essentially this means it should be ok on LANs and fast WiFi networks. This should even work with multiple clients. There's one known issue in the framework: if the Spice server and clients have no codec in common, red_display_create_stream() will set video_encoder to NULL and that's not handled well. Currently this should really not happen as all clients support MJPEG and we have the builtin MJPEG encoder as a fallback. The root of the issue is that the red_display_create_stream() callers assume it will never fail. But I don't know how to change that yet. The GStreamer backend still has a number of limitations: 1. It's still based on GStreamer 0.10. I don't think moving to 1.0 will be a problem, if anything it should help (famous last words). For instance support for dynamically changing the bitrate appears to be better in 1.0. 2. It still does not have any rate control but should work fine as long as the bandwidth is sufficient. The issue with rate control is that, at least in 0.10, GStreamer encoders don't like bitrate changes, need some time after each change to actually match the new bitrate, still tend to exceed it from time to time, and sometimes just won't get anywhere near it. Part of these issues are likely to be bugs in the current implementation and it should be possible to work around most others. Feedback would be greatly appreciated. Cheers, -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 4/11] server: Add a GStreamer 0.10 MJPEG video encoder and use it by default.
The GStreamer video encoder is quite primitive and mostly meant to
introduce, test and debug the basic infrastructure.
The main limitation is the lack of rate control: the bitrate is set at
startup and will not react to changes in the network conditions. Still
it should work fine on LANs.
---
configure.ac | 18 ++
server/Makefile.am | 8 +
server/gstreamer_encoder.c | 441 +
server/red_worker.c| 11 +-
server/video_encoder.h | 10 +-
5 files changed, 484 insertions(+), 4 deletions(-)
create mode 100644 server/gstreamer_encoder.c
diff --git a/configure.ac b/configure.ac
index 7a9fc4b..ae11301 100644
--- a/configure.ac
+++ b/configure.ac
@@ -91,6 +91,15 @@ if test x$enable_smartcard = xyes; then
AC_DEFINE([USE_SMARTCARD], [1], [Define if supporting smartcard proxying])
fi
+AC_ARG_ENABLE(gstreamer-0.10,
+[ --enable-gstreamer-0.10 Enable GStreamer 0.10 support],,
+[enable_gstreamer_0_10=yes])
+AS_IF([test x$enable_gstreamer_0_10 != xno], [enable_gstreamer_0_10=yes])
+AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$enable_gstreamer_0_10 != xno)
+if test x$enable_gstreamer_0_10 = xyes; then
+ AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer 0.10])
+fi
+
AC_ARG_ENABLE(automated_tests,
[ --enable-automated-tests Enable automated tests using spicy-screenshot
(part of spice--gtk)],,
[enable_automated_tests=no])
@@ -127,6 +136,13 @@ if test x$enable_smartcard = xyes; then
AS_VAR_APPEND([SPICE_REQUIRES], [ libcacard = 0.1.2])
fi
+if test x$enable_gstreamer_0_10 = xyes; then
+PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10])
+AC_SUBST(GSTREAMER_0_10_LIBS)
+AC_SUBST(GSTREAMER_0_10_CFLAGS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10])
+fi
+
PKG_CHECK_MODULES([GLIB2], [glib-2.0 = 2.22])
AS_VAR_APPEND([SPICE_REQUIRES], [ glib-2.0 = 2.22])
@@ -359,6 +375,8 @@ echo
Smartcard:${enable_smartcard}
+GStreamer-0.10: ${enable_gstreamer-0.10}
+
SASL support: ${enable_sasl}
Automated tests: ${enable_automated_tests}
diff --git a/server/Makefile.am b/server/Makefile.am
index 36b6d45..28c4a46 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -11,6 +11,7 @@ AM_CPPFLAGS = \
$(SASL_CFLAGS) \
$(SLIRP_CFLAGS) \
$(SMARTCARD_CFLAGS) \
+ $(GSTREAMER_0_10_CFLAGS)\
$(SSL_CFLAGS) \
$(VISIBILITY_HIDDEN_CFLAGS) \
$(WARN_CFLAGS) \
@@ -41,6 +42,7 @@ libspice_server_la_LIBADD =
\
$(PIXMAN_LIBS) \
$(SASL_LIBS)\
$(SLIRP_LIBS) \
+ $(GSTREAMER_0_10_LIBS)
\
$(SSL_LIBS) \
$(Z_LIBS) \
$(SPICE_NONPKGCONFIG_LIBS) \
@@ -138,6 +140,12 @@ libspice_server_la_SOURCES += \
$(NULL)
endif
+if SUPPORT_GSTREAMER_0_10
+libspice_server_la_SOURCES += \
+ gstreamer_encoder.c \
+ $(NULL)
+endif
+
EXTRA_DIST = \
glz_encode_match_tmpl.c \
glz_encode_tmpl.c \
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
new file mode 100644
index 000..ef71b73
--- /dev/null
+++ b/server/gstreamer_encoder.c
@@ -0,0 +1,441 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ Copyright (C) 2015 Jeremy White
+ Copyright (C) 2015 Francois Gouget
+
+ 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 A
HREF=http://www.gnu.org/licenses/;http://www.gnu.org/licenses//A.
+*/
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include gst/gst.h
+#include gst/app/gstappsrc.h
+#include gst/app/gstappsink.h
+
+#include red_common.h
+
+typedef struct GstEncoder GstEncoder;
+#define VIDEO_ENCODER_T
[Spice-devel] [PATCH spice 2/11] server: Remove the rate_control_is_active field from MJpegEncoder.
It is redundant with the corresponding callbacks.
---
This patch only depends on patch 1/11 and is independent from the rest
of the series.
server/mjpeg_encoder.c | 22 +-
server/mjpeg_encoder.h | 2 +-
server/red_worker.c| 4 ++--
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 95d841f..9a41ef3 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -164,7 +164,6 @@ struct MJpegEncoder {
unsigned int bytes_per_pixel; /* bytes per pixel of the input buffer */
void (*pixel_converter)(uint8_t *src, uint8_t *dest);
-int rate_control_is_active;
MJpegEncoderRateControl rate_control;
MJpegEncoderRateControlCbs cbs;
void *cbs_opaque;
@@ -185,21 +184,25 @@ static uint32_t get_min_required_playback_delay(uint64_t
frame_enc_size,
uint64_t byte_rate,
uint32_t latency);
-MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t
starting_bit_rate,
+static inline int rate_control_is_active(MJpegEncoder* encoder)
+{
+return encoder-cbs.get_roundtrip_ms != NULL;
+}
+
+MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
MJpegEncoderRateControlCbs *cbs, void *opaque)
{
MJpegEncoder *enc;
-spice_assert(!bit_rate_control || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
+spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
enc = spice_new0(MJpegEncoder, 1);
enc-first_frame = TRUE;
-enc-rate_control_is_active = bit_rate_control;
enc-rate_control.byte_rate = starting_bit_rate / 8;
enc-starting_bit_rate = starting_bit_rate;
-if (bit_rate_control) {
+if (cbs) {
struct timespec time;
clock_gettime(CLOCK_MONOTONIC, time);
@@ -211,6 +214,7 @@ MJpegEncoder *mjpeg_encoder_new(int bit_rate_control,
uint64_t starting_bit_rate
enc-rate_control.quality_eval_data.reason =
MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
enc-rate_control.warmup_start_time = ((uint64_t) time.tv_sec) *
10 + time.tv_nsec;
} else {
+enc-cbs.get_roundtrip_ms = NULL;
mjpeg_encoder_reset_quality(enc, MJPEG_LEGACY_STATIC_QUALITY_ID,
MJPEG_MAX_FPS, 0);
}
@@ -607,7 +611,7 @@ static void
mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
uint32_t latency = 0;
uint32_t src_fps;
-spice_assert(encoder-rate_control_is_active);
+spice_assert(rate_control_is_active(encoder));
rate_control = encoder-rate_control;
quality_eval = rate_control-quality_eval_data;
@@ -692,7 +696,7 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder,
uint64_t now)
MJpegEncoderRateControl *rate_control = encoder-rate_control;
uint64_t adjusted_fps_time_passed;
-spice_assert(encoder-rate_control_is_active);
+spice_assert(rate_control_is_active(encoder));
adjusted_fps_time_passed = (now - rate_control-adjusted_fps_start_time) /
1000 / 1000;
@@ -734,7 +738,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
SpiceBitmapFmt format,
{
uint32_t quality;
-if (encoder-rate_control_is_active) {
+if (rate_control_is_active(encoder)) {
MJpegEncoderRateControl *rate_control = encoder-rate_control;
struct timespec time;
uint64_t now;
@@ -1131,7 +1135,7 @@ void mjpeg_encoder_client_stream_report(MJpegEncoder
*encoder,
end_frame_mm_time - start_frame_mm_time,
end_frame_delay, audio_delay);
-if (!encoder-rate_control_is_active) {
+if (!rate_control_is_active(encoder)) {
spice_debug(rate control was not activated: ignoring);
return;
}
diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
index 741ea1c..d584b92 100644
--- a/server/mjpeg_encoder.h
+++ b/server/mjpeg_encoder.h
@@ -49,7 +49,7 @@ typedef struct MJpegEncoderStats {
double avg_quality;
} MJpegEncoderStats;
-MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t
starting_bit_rate,
+MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
MJpegEncoderRateControlCbs *cbs, void *opaque);
void mjpeg_encoder_destroy(MJpegEncoder *encoder);
diff --git a/server/red_worker.c b/server/red_worker.c
index 5deb30b..e0ff8e9 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3100,9 +3100,9 @@ static void
red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
mjpeg_cbs.update_client_playback_delay =
red_stream_update_client_playback_latency;
initial_bit_rate = red_stream_get_initial_bit_rate(dcc, stream);
-agent-mjpeg_encoder = mjpeg_encoder_new(TRUE, initial_bit_rate,
mjpeg_cbs, agent);
+agent-mjpeg_encoder = mjpeg_encoder_new(initial_bit_rate, mjpeg_cbs,
agent);
} else {
-agent-mjpeg_encoder =
[Spice-devel] [PATCH spice 1/11] server: Convert a couple of rate control checks into asserts in the mjpeg video encoder.
The checks would lead the reader to think these functions can be called
when bit rate control is off when in fact they are only called when it
is active.
---
This patch makes sense independently from the rest of the series.
server/mjpeg_encoder.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 12447da..95d841f 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -607,9 +607,7 @@ static void
mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
uint32_t latency = 0;
uint32_t src_fps;
-if (!encoder-rate_control_is_active) {
-return;
-}
+spice_assert(encoder-rate_control_is_active);
rate_control = encoder-rate_control;
quality_eval = rate_control-quality_eval_data;
@@ -694,9 +692,8 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder,
uint64_t now)
MJpegEncoderRateControl *rate_control = encoder-rate_control;
uint64_t adjusted_fps_time_passed;
-if (!encoder-rate_control_is_active) {
-return;
-}
+spice_assert(encoder-rate_control_is_active);
+
adjusted_fps_time_passed = (now - rate_control-adjusted_fps_start_time) /
1000 / 1000;
if (!rate_control-during_quality_eval
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 3/11] server: Refactor the Spice server video encoding so alternative implementations can be added.
:
reds_get_mm_time();
+
outbuf_size = dcc-send_data.stream_outbuf_size;
-ret = mjpeg_encoder_start_frame(agent-mjpeg_encoder,
image-u.bitmap.format,
-width, height,
-dcc-send_data.stream_outbuf,
-outbuf_size,
-frame_mm_time);
+ret = agent-video_encoder-encode_frame(agent-video_encoder,
+image-u.bitmap, width, height,
+drawable-red_drawable-u.copy.src_area,
+stream-top_down, frame_mm_time,
+dcc-send_data.stream_outbuf, outbuf_size, n);
+
switch (ret) {
-case MJPEG_ENCODER_FRAME_DROP:
-spice_assert(dcc-use_mjpeg_encoder_rate_control);
+case VIDEO_ENCODER_FRAME_DROP:
+spice_assert(dcc-use_video_encoder_rate_control);
#ifdef STREAM_STATS
agent-stats.num_drops_fps++;
#endif
return TRUE;
-case MJPEG_ENCODER_FRAME_UNSUPPORTED:
+case VIDEO_ENCODER_FRAME_UNSUPPORTED:
return FALSE;
-case MJPEG_ENCODER_FRAME_ENCODE_START:
+case VIDEO_ENCODER_FRAME_ENCODE_DONE:
break;
default:
-spice_error(bad return value (%d) from mjpeg_encoder_start_frame,
ret);
-return FALSE;
-}
-
-if (!encode_frame(dcc, drawable-red_drawable-u.copy.src_area,
- image-u.bitmap, stream)) {
+spice_error(bad return value (%d) from VideoEncoder::encode_frame,
ret);
return FALSE;
}
-n = mjpeg_encoder_end_frame(agent-mjpeg_encoder);
dcc-send_data.stream_outbuf_size = outbuf_size;
if (!drawable-sized_stream) {
@@ -10255,7 +10187,7 @@ static int
display_channel_handle_stream_report(DisplayChannelClient *dcc,
return FALSE;
}
stream_agent = dcc-stream_agents[stream_report-stream_id];
-if (!stream_agent-mjpeg_encoder) {
+if (!stream_agent-video_encoder) {
spice_info(stream_report: no encoder for stream id %u.
Probably the stream has been destroyed,
stream_report-stream_id);
return TRUE;
@@ -10266,7 +10198,7 @@ static int
display_channel_handle_stream_report(DisplayChannelClient *dcc,
stream_agent-report_id, stream_report-unique_id);
return TRUE;
}
-mjpeg_encoder_client_stream_report(stream_agent-mjpeg_encoder,
+
stream_agent-video_encoder-client_stream_report(stream_agent-video_encoder,
stream_report-num_frames,
stream_report-num_drops,
stream_report-start_frame_mm_time,
diff --git a/server/video_encoder.h b/server/video_encoder.h
new file mode 100644
index 000..8a9f9c6
--- /dev/null
+++ b/server/video_encoder.h
@@ -0,0 +1,162 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ Copyright (C) 2009 Red Hat, Inc.
+ Copyright (C) 2015 Jeremy White
+ Copyright (C) 2015 Francois Gouget
+
+ 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 A
HREF=http://www.gnu.org/licenses/;http://www.gnu.org/licenses//A.
+*/
+
+#ifndef _H_video_encoder
+#define _H_video_encoder
+
+#include red_common.h
+
+enum {
+VIDEO_ENCODER_FRAME_UNSUPPORTED = -1,
+VIDEO_ENCODER_FRAME_DROP,
+VIDEO_ENCODER_FRAME_ENCODE_DONE,
+};
+
+typedef struct VideoEncoderStats {
+uint64_t starting_bit_rate;
+uint64_t cur_bit_rate;
+double avg_quality;
+} VideoEncoderStats;
+
+typedef struct VideoEncoder VideoEncoder;
+#ifndef VIDEO_ENCODER_T
+# define VIDEO_ENCODER_T VideoEncoder
+#endif
+
+struct VideoEncoder {
+/* Releases the video encoder's resources */
+void (*destroy)(VIDEO_ENCODER_T *encoder);
+
+/* Compresses the specified src image area into the outbuf buffer.
+ *
+ * @encoder: The video encoder.
+ * @bitmap:The Spice screen.
+ * @width: The width of the Spice screen.
+ * @height:The heigth of the Spice screen.
+ * @src: A rectangle specifying the area occupied by the video.
+ * @top_down: If true the first video line is specified by src.top.
+ * @outbuf:The buffer for the compressed frame. This must either be
+ * NULL or point to a buffer allocated by malloc since it may
be
+ * reallocated, if its size is too small.
+ * @outbuf_size
[Spice-devel] [PATCH spice] server: Round the framerate estimate to the nearest integer.
This is more accurate for typical values like 23.976 fps.
---
server/red_worker.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index cc0a116..0207977 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3159,15 +3159,15 @@ static void red_stream_input_fps_timer_cb(void *opaque)
{
Stream *stream = opaque;
uint64_t now = red_now();
-double duration_sec;
spice_assert(opaque);
if (now == stream-input_fps_timer_start) {
spice_warning(timer start and expiry time are equal);
return;
}
-duration_sec = (now - stream-input_fps_timer_start)/(1000.0*1000*1000);
-stream-input_fps = stream-num_input_frames / duration_sec;
+/* Round to the nearest integer, for instance 24 for 23.976 */
+uint64_t duration = now - stream-input_fps_timer_start;
+stream-input_fps = ((uint64_t)stream-num_input_frames * 1000 * 1000 *
1000 + duration / 2) / duration;
spice_debug(input-fps=%u, stream-input_fps);
stream-num_input_frames = 0;
stream-input_fps_timer_start = now;
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support
On Thu, 4 Jun 2015, Fabio Fantoni wrote: [...] No bandwidth limit, on client I saw use between 1 and 3 mb, both server and client have 1 gbps cabled network (lan). Server used for testing is a dell PE T310 with cpu xeon X3450 is asus K53S with core i5-2430M. Server cpu usage of qemu process is over 300% on streming video. spice streaming video setting is 'all' and codec 'gstreamer:vp8', no error/warning in qemu logs. I used github.com/fgouget/spice-gtk/commits/gst including avoid gstreamer deadlock and github.com/fgouget/spice/commits/gst in latest test. Client cpu and network usage seems low but streaming video is bad (with vp8), here a small video example done with smartphone: http://fantu.it/vari/spice-gtk-vp8.mp4 The video looks like there are two issues: * The video corruption. It looks like an I frame was lost and the following P frames were thus decoded incorrectly. This is a bit strange because in my experience VP8 throws the P frames in such a case (so they are not displayed, unlike in h264's case). * The short pause every few seconds. This may be related to the previous issue: I frames are bigger than P frames. Maybe they take too long coming across, thus forcing the client to pause waiting for them, and maybe that's also why they sometimes get lost. But given your network specifications that should not be the case. I have done tests with StreamingVideo=all here but it worked as well as when set to filter so that's probably not the source of the difference. Overall I suspect more a problem on the server-side. Here's what you can do: * First on the server check for messages that indicate some frames were dropped like the ones below (SPICE_DEBUG_LEVEL=9): (../xf86-video-qxl/scripts/Xspice:31980): SpiceWorker-Debug **: red_worker.c:3378:pre_stream_item_swap: ignoring drop, same pcg as previous frame 296 (../xf86-video-qxl/scripts/Xspice:31980): SpiceWorker-Debug **: red_worker.c:10643:display_channel_release_item: not pushed (101) As far as I understand it's the second message that indicates a frame has been dropped, but it systematically follows the first one so they are related. These happen even when there's no bandwidth issue and only happen in the GStreamer 1.0 case. And I have no idea what's causing this so far :-( * You can also look for 'server frame drop' in the server log. Unlike the messages above, those would indicate a network problem. * In gstreamer_encoder.c you may try adding a line like the following near the start of adjust_bit_rate() to see if constraining the bandwidth helps: encoder-bit_rate = 1024 * 1024 * 3; In the server log you can also look for 'base-bit-rate' and 'setting the bit rate' to see what Spice and gstreamer_encoder think of the bitrate respectively. * Do you have the same issue if you use gstreamer:mjpeg on the server? If gstreamer:mjpeg works fine then we'll know it's not a general GStreamer issue but more a codec-specific one. * The next thing to try would be gstreamer:h264. h264 is quite similar to vp8 in caracteristics: it has I and P frames too and also uses quite a bit of CPU, though it seems to handle threading a bit better. * Then if you can try running the server with GStreamer 0.10 this would let us determine if it's a GStreamer version problem. * The CPU usage on the server, 300%, does seem a bit high though for a 4C/8T CPU it should be manageable. Unfortunately gstreamer_encoder already tunes vp8enc for speed. Maybe setting threads=4 or 5, or cpu-used=16 can speed it up some more. For network usage maybe max-intra-bitrate can help. -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] RFC spice: Should the input-fps be refreshed regularly?
Spice provides an estimate of the input video fps to the video streaming backends. This is done by red_stream_input_fps_timer_cb() in red_worker.c which is called 5 seconds after the streaming starts. That callback contains code so it can be called again: stream-num_input_frames = 0; stream-input_fps_timer_start = now; However the corresponding timer is not rearmed so it is in fact called only once. Is that the correct behavior or do we want the input_fps field to be refreshed regularly? For video the framerate tends to be pretty constant so calculating it once could be sufficient. But maybe there are other cases where we would want more up to date information. Games maybe? -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Status of spice-html5 and video streaming
On Fri, 5 Jun 2015, Jeremy White wrote: [...] I haven't had a chance to work this in with the vp8 code, which I had hoped would provide a dramatic improvement. Unfortunately, the sized stream message really creates havoc with vp8 streams, and you really can't play a video without the sized stream messages. See: http://lists.freedesktop.org/archives/spice-devel/2015-May/019948.html One way to test things without running into the sized stream issues is to use mplayer. It has no GUI so no progress bar or other menus to confuse Spice: DISPLAY=:3.0 mplayer big_buck_bunny_480p_h264.mov -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH qxl] spiceqxl: Recognize the same set of boolean values as in xorg.conf.
Report an error if an invalid boolean value is used, just like for other
options.
---
This seems simple enough and may avoid confusion.
src/qxl_option_helpers.c | 27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/src/qxl_option_helpers.c b/src/qxl_option_helpers.c
index 8801b53..c9ad726 100644
--- a/src/qxl_option_helpers.c
+++ b/src/qxl_option_helpers.c
@@ -3,6 +3,7 @@
#endif
#include stdlib.h
+#include strings.h
#include xf86.h
#include xf86Opt.h
@@ -30,12 +31,24 @@ const char *get_str_option(OptionInfoPtr options, int
option_index,
int get_bool_option(OptionInfoPtr options, int option_index,
const char *env_name)
{
-if (getenv(env_name)) {
-/* we don't support the whole range of boolean true and
- * false values documented in man xorg.conf, just the c
- * convention - 0 is false, anything else is true, so
- * just like a number. */
-return !!atoi(getenv(env_name));
+const char* value = getenv(env_name);
+
+if (!value) {
+return options[option_index].value.bool;
+}
+if (strcmp(value, 1) == 0 ||
+strcasecmp(value, on) == 0 ||
+strcasecmp(value, true) == 0 ||
+strcasecmp(value, yes) == 0) {
+return TRUE;
}
-return options[option_index].value.bool;
+if (strcmp(value, 0) == 0 ||
+strcasecmp(value, off) == 0 ||
+strcasecmp(value, false) == 0 ||
+strcasecmp(value, no) == 0) {
+return FALSE;
+}
+
+fprintf(stderr, spice: invalid %s: %s\n, env_name, value);
+exit(1);
}
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH qxl] Xspice: Don't set defaults for the options.
Otherwise they override Spice server's real builtin defaults, the Xorg
configuration file settings, and even the XSPICE_XXX environment variables.
---
Without this patch, calling Xspice _without_ the '--streaming-video'
option forces this setting to 'filter', overriding the
XSPICE_STREAMING_VIDEO environment variable and the SpiceStreamingVideo
spiceqxl.xorg.conf setting.
scripts/Xspice | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/scripts/Xspice b/scripts/Xspice
index 281535d..f565eb1 100755
--- a/scripts/Xspice
+++ b/scripts/Xspice
@@ -45,7 +45,7 @@ else:
cgdb = None
def add_boolean(flag, *args, **kw):
-parser.add_argument(flag, action='store_const', const='1', default='0',
+parser.add_argument(flag, action='store_const', const='1',
*args, **kw)
wan_compression_options = ['auto', 'never', 'always']
@@ -65,7 +65,7 @@ parser.add_argument('--deferred-fps', type=int, help='If
given, render to a buff
parser.add_argument('--tls-port', type=int, help='spice tls port', default=0)
add_boolean('--disable-ticketing', help=do not require a client password)
add_boolean('--sasl', help=enable sasl)
-parser.add_argument('--x509-dir', help=x509 directory for tls, default='.')
+parser.add_argument('--x509-dir', help=x509 directory for tls)
parser.add_argument('--cacert-file', help=ca certificate file for tls)
parser.add_argument('--x509-cert-file', help=server certificate file for tls)
parser.add_argument('--x509-key-file', help=server key file for tls)
@@ -76,16 +76,16 @@ parser.add_argument('--password', help=set password
required to connect to serv
parser.add_argument('--image-compression',
choices = ['off', 'auto_glz', 'auto_lz', 'quic',
'glz', 'lz'],
-default='auto_glz', help='auto_glz by default')
+help='auto_glz by default')
parser.add_argument('--jpeg-wan-compression',
choices=wan_compression_options,
-default='auto', help='auto by default')
+help='auto by default')
parser.add_argument('--zlib-glz-wan-compression',
choices=wan_compression_options,
-default='auto', help='auto by default')
+help='auto by default')
# TODO - sound support
parser.add_argument('--streaming-video', choices=['off', 'all', 'filter'],
-default='filter', help='filter by default')
+help='filter by default')
add_boolean('--ipv4-only')
add_boolean('--ipv6-only')
parser.add_argument('--vdagent', action='store_true', dest='vdagent_enabled',
default=False, help='launch vdagent vdagentd. They provide clipboard
resolution automation')
@@ -96,7 +96,7 @@ parser.add_argument('--vdagent-exec', help='path to
spice-vdagent (used with --v
parser.add_argument('--vdagent-no-launch', default=True, action='store_false',
dest='vdagent_launch', help='do not launch vdagent vdagentd, used for
debugging or if some external script wants to take care of that')
parser.add_argument('--vdagent-uid', default=str(os.getuid()), help='set
vdagent user id. changing it makes sense only in conjunction with
--vdagent-no-launch')
parser.add_argument('--vdagent-gid', default=str(os.getgid()), help='set
vdagent group id. changing it makes sense only in conjunction with
--vdagent-no-launch')
-parser.add_argument('--audio-fifo-dir', default='', help=set fifo directory
for playback audio. designed to work with PulseAudio's module-pipe-sink)
+parser.add_argument('--audio-fifo-dir', help=set fifo directory for playback
audio. designed to work with PulseAudio's module-pipe-sink)
#TODO
#Option SpiceAddr
@@ -105,7 +105,7 @@ parser.add_argument('--audio-fifo-dir', default='',
help=set fifo directory for
#Option EnableFallbackCache True
#Option EnableSurfaces True
#Option NumHeads 4
-#parser.add_argument('--playback-compression', choices=['0', '1'],
default='1', help='enabled by default')
+#parser.add_argument('--playback-compression', choices=['0', '1'],
help='enabled by default')
#Option SpiceDisableCopyPaste False
if cgdb:
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice] server: Remove an unused structure.
On Wed, 27 May 2015, Francois Gouget wrote: --- This struct was introduced in the first commit, c1b79eb0, about 6 years ago, but has never been used. So I think it's safe to remove. Is something holding up this patch? Does it need modifications for inclusion? -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice 1/2] server: Refresh the input fps every 5 second, without a timer.
On Thu, 11 Jun 2015, Francois Gouget wrote: Signed-off-by: Francois Gouget fgou...@codeweavers.com --- This implements Marc-André Lureau's suggestion: http://lists.freedesktop.org/archives/spice-devel/2015-June/020202.html And supersedes the input-fps rounding patch. http://lists.freedesktop.org/archives/spice-devel/2015-June/020176.html Does this patch series need modifications for inclusion? -- Francois Gouget fgou...@codeweavers.com___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice 1/12] server: Convert a couple of rate control checks into asserts in the mjpeg video encoder. (take 3)
On Wed, 10 Jun 2015, Francois Gouget wrote:
The checks would lead the reader to think these functions can be called when
bit rate control is off when in fact they are only called when it is active.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This patch stands on its own and I think it makes sense even if the
remainder of the series is not applied. It seemed ok in round 1 so
again no change this time around. Let me know if it needs further
refinements for inclusion.
This patch was acked a while ago but has not been committed yet.
Does it need changes? Should I drop it from the series?
(Same thing for the next one, 2/12)
http://lists.freedesktop.org/archives/spice-devel/2015-May/019843.html
server/mjpeg_encoder.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 12447da..95d841f 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -607,9 +607,7 @@ static void
mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
uint32_t latency = 0;
uint32_t src_fps;
-if (!encoder-rate_control_is_active) {
-return;
-}
+spice_assert(encoder-rate_control_is_active);
rate_control = encoder-rate_control;
quality_eval = rate_control-quality_eval_data;
@@ -694,9 +692,8 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder
*encoder, uint64_t now)
MJpegEncoderRateControl *rate_control = encoder-rate_control;
uint64_t adjusted_fps_time_passed;
-if (!encoder-rate_control_is_active) {
-return;
-}
+spice_assert(encoder-rate_control_is_active);
+
adjusted_fps_time_passed = (now - rate_control-adjusted_fps_start_time)
/ 1000 / 1000;
if (!rate_control-during_quality_eval
--
2.1.4
--
Francois Gouget fgou...@codeweavers.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH common 5/12] spice.proto: Add support for the VP8 and H264 video codecs. (take 3)
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Changes since take 2:
- This patch also adds h264.
spice.proto | 2 ++
1 file changed, 2 insertions(+)
diff --git a/spice.proto b/spice.proto
index 01493c9..95123ae 100644
--- a/spice.proto
+++ b/spice.proto
@@ -329,6 +329,8 @@ flags8 path_flags { /* TODO: C enum names changes */
enum8 video_codec_type {
MJPEG = 1,
+VP8,
+H264,
};
flags8 stream_flags {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 4/12] server: Add a GStreamer 0.10 MJPEG video encoder and use it by default. (take 3)
The GStreamer video encoder supports both regular and sized streamsi.
It is otherwise quite basic and lacks any rate control: the bitrate is set at
startup and will not react to changes in the network conditions. Still it
should work fine on LANs.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This does not use encodebin because we really need to know much about
the encoder:
- We need to disable B frames for encoders that support inter-frame
compression (see the VP8 and h264 patches).
- We need to be able to set the bitrate which the encoder may want to
get in either bits/second or kilobits/second.
- We also have to configure the encoder for speed since we need it to
work in real time.
As far as I can tell there is no standardized way to specify this so we
need intimate knowledge of the encoder parameters.
Changes since take 2:
- I resolved the buffer timestamping issues. Appsrc is no longer a live
source (it had no reason to be) and it performs the timestamping. The
only remaining annoyance is that ffenc_mjpeg requires removing the
pipeline's default clock otherwise it gets stuck with the following
message:
ERROR ffmpeg :0:: Error, Invalid timestamp=0, last=0
I consider this to be an ffenc_mjpeg bug since none of the other
encoders (VP8, H264 and even avenc_mpjpeg in GStreamer 1.0) has this
issue. So I'm keeping the clock removal as a workaround for
ffenc_mjpeg.
- I went back to ffmpegcolorspace because it's the standard way to do
color conversion in GStreamer 0.10 (present in gst-plugins-base),
while autovideoconvert is only present in gst-plugins-bad which I
believe we don't depend on.
- I simplified the source frame copy code (push_raw_frame()). It's also
ready for adding zero-copy once GStreamer 1.0 support is added.
- I improved the autoconf GStreamer 0.10 detection: it's now possible
to require GStreamer support, or just let it be included if present.
This also paves the way for GStreamer 1.0.
- Changed set_appsrc_caps() and construct_pipeline() to return a
gboolean since they only return TRUE/FALSE.
- Fixed some formatting issues, mostly braces placement.
Changes since take 1:
- The width/height comments are now correct in the previous patch of
the series.
- Fixed the pipeline reconfiguration (for video size changes) so it
still works if we have to fall back to rebuilding the pipeline from
scratch.
- Added a comment suggesting to avoid copying the compressed buffer as
a future improvement. I think this will require changing the way the
output buffer is allocated though. So I'm leaving that for after the
basic support committed.
configure.ac | 27 ++-
server/Makefile.am | 8 +
server/gstreamer_encoder.c | 434 +
server/red_worker.c| 11 +-
server/video_encoder.h | 6 +
5 files changed, 483 insertions(+), 3 deletions(-)
create mode 100644 server/gstreamer_encoder.c
diff --git a/configure.ac b/configure.ac
index 7a9fc4b..34e1a4a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -91,6 +91,30 @@ if test x$enable_smartcard = xyes; then
AC_DEFINE([USE_SMARTCARD], [1], [Define if supporting smartcard proxying])
fi
+AC_ARG_ENABLE(gstreamer,
+ AS_HELP_STRING([--enable-gstreamer=@:@auto/yes/no@:@],
+ [Enable GStreamer 0.10 support]),
+ [],
+ [enable_gstreamer=auto])
+
+if test x$enable_gstreamer != xno; then
+PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10],
+ [enable_gstreamer=yes
+ have_gstreamer_0_10=yes],
+ [have_gstreamer_0_10=no])
+if test x$have_gstreamer_0_10 = xyes; then
+AC_SUBST(GSTREAMER_0_10_CFLAGS)
+AC_SUBST(GSTREAMER_0_10_LIBS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10 gstreamer-app-0.10])
+AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer
0.10])
+elif test x$enable_gstreamer = xyes; then
+AC_MSG_ERROR([GStreamer 0.10 support requested but not found. You may
set GSTREAMER_0_10_CFLAGS and GSTREAMER_0_10_LIBS to avoid the need to call
pkg-config.])
+fi
+else
+have_gstreamer_0_10=no
+fi
+AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$have_gstreamer_0_10 = xyes)
+
AC_ARG_ENABLE(automated_tests,
[ --enable-automated-tests Enable automated tests using spicy-screenshot
(part of spice--gtk)],,
[enable_automated_tests=no])
@@ -127,7 +151,6 @@ if test x$enable_smartcard = xyes; then
AS_VAR_APPEND([SPICE_REQUIRES], [ libcacard = 0.1.2])
fi
-
PKG_CHECK_MODULES([GLIB2], [glib-2.0 = 2.22])
AS_VAR_APPEND([SPICE_REQUIRES], [ glib-2.0 = 2.22])
@@ -359,6 +382,8 @@ echo
Smartcard:${enable_smartcard}
+GStreamer 0.10: ${have_gstreamer_0_10}
+
SASL support: ${enable_sasl}
Automated
[Spice-devel] [PATCH spice 7/12] server: Add VP8 support, a video codec preference list and compatibility checks with the Spice client. (take 3)
The video encoder preferences can be expressed by building a semi-colon
separated list of encoder:codec pairs. For instance 'gstreamer:vp8;spice:mjpeg'
to pick first the GStreamer VP8 video encoder first and used the original MJPEG
video encoder one as a fallback.
The server has a default preference list which can also be selected by
specifying 'auto' as the preferences list.
The client compatibility check relies on the following new capabilities:
* SPICE_DISPLAY_CAP_MULTI_CODEC which denotes a recent client that supports
multiple codecs. This capability is needed to not have to hardcode that MJPEG
is supported. This makes it possible to write clients that don't support MJPEG.
* SPICE_DISPLAY_CAP_CODEC_XXX, where XXX is a supported codec, for now MJPEG
and VP8.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Note that this patch should be preceded by one that grabs the right
spice-common commit so we get the new codec and capability constants.
Changes since take 2:
- We now allow the VP8 encoder to produce P frames since this does not
increase the latency (i.e. it still lets us get the compressed frames
right away), while improving the compression ratio.
- It turns out the above also helps with CPU usage and lets us
further configure the encoder for speed.
- Only construct_pipeline() really needs to know the encoder name so
we no longer keep it in a GstEncoder field.
- Fixed some formatting issues, mostly braces placement.
Changes since take 1:
- Only set the parameters supported by the current encoder.
- reconfigure_pipeline() handling has been fixed in the previous patch.
- The video codecs list now has a more sensible default and it's
possible to explicitly request this default by specifying 'auto' in
the configuration files.
server/gstreamer_encoder.c | 61 +---
server/mjpeg_encoder.c | 7 +-
server/red_dispatcher.c| 172 -
server/red_dispatcher.h| 8 +++
server/red_worker.c| 65 ++---
server/red_worker.h| 18 +
server/reds.c | 13
server/spice-server.h | 1 +
server/spice-server.syms | 1 +
server/video_encoder.h | 18 +++--
10 files changed, 321 insertions(+), 43 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 3600b1b..e7c9a56 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -178,10 +178,27 @@ static gboolean set_appsrc_caps(GstEncoder *encoder)
static gboolean construct_pipeline(GstEncoder *encoder, const SpiceBitmap
*bitmap)
{
+gboolean no_clock = FALSE;
+const gchar* gstenc_name;
+switch (encoder-base.codec_type)
+{
+case SPICE_VIDEO_CODEC_TYPE_MJPEG:
+gstenc_name = ffenc_mjpeg;
+no_clock = TRUE;
+break;
+case SPICE_VIDEO_CODEC_TYPE_VP8:
+gstenc_name = vp8enc;
+break;
+default:
+spice_warning(unsupported codec type %d, encoder-base.codec_type);
+return FALSE;
+}
+
GError *err = NULL;
-const gchar* desc = appsrc name=src format=2 do-timestamp=true !
ffmpegcolorspace ! ffenc_mjpeg name=encoder ! appsink name=sink;
+gchar *desc = g_strdup_printf(appsrc name=src format=2 do-timestamp=true
! ffmpegcolorspace ! %s name=encoder ! appsink name=sink, gstenc_name);
spice_debug(GStreamer pipeline: %s, desc);
encoder-pipeline = gst_parse_launch_full(desc, NULL,
GST_PARSE_FLAG_FATAL_ERRORS, err);
+g_free(desc);
if (!encoder-pipeline) {
spice_warning(GStreamer error: %s, err-message);
g_clear_error(err);
@@ -191,19 +208,31 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
encoder-gstenc = gst_bin_get_by_name(GST_BIN(encoder-pipeline),
encoder);
encoder-appsink =
GST_APP_SINK(gst_bin_get_by_name(GST_BIN(encoder-pipeline), sink));
+/* Set the encoder initial bit rate, and ask for a low latency */
+adjust_bit_rate(encoder);
+g_object_set(G_OBJECT(encoder-gstenc), bitrate, encoder-bit_rate,
NULL);
+if (encoder-base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8) {
+/* See http://www.webmproject.org/docs/encoder-parameters/ */
+g_object_set(G_OBJECT(encoder-gstenc),
+ mode, 1, /* CBR */
+ max-latency, 0,
+ speed, 7,
+ resize-allowed, TRUE,
+ threads, g_get_num_processors() - 1,
+ NULL);
+}
+
/* Set the source caps */
encoder-src_caps = NULL;
if (!set_appsrc_caps(encoder)) {
return FALSE;
}
-/* Set the encoder initial bit rate */
-adjust_bit_rate(encoder);
-g_object_set(G_OBJECT(encoder-gstenc), bitrate, encoder-bit_rate,
NULL);
-
/* Start playing */
-spice_debug(removing the pipeline clock);
-gst_pipeline_use_clock(GST_PIPELINE(encoder-pipeline), NULL
[Spice-devel] [PATCH spice 12/12] server: Add h264 support to the GStreamer video encoder.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Having support for h264 is interesting in its own right but this shows
adding extra codecs is quite easy.
server/gstreamer_encoder.c | 17 -
server/red_dispatcher.c| 2 ++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 6a47668..451518f 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -204,6 +204,9 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
case SPICE_VIDEO_CODEC_TYPE_VP8:
gstenc_name = vp8enc;
break;
+case SPICE_VIDEO_CODEC_TYPE_H264:
+gstenc_name = x264enc;
+break;
default:
spice_warning(unsupported codec type %d, encoder-base.codec_type);
return FALSE;
@@ -254,6 +257,17 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
threads, g_get_num_processors() - 1,
NULL);
break;
+case SPICE_VIDEO_CODEC_TYPE_H264:
+g_object_set(G_OBJECT(encoder-gstenc),
+ bitrate, encoder-bit_rate / 1024,
+ byte-stream, TRUE,
+ aud, FALSE,
+ tune, 4, /* Zero latency */
+ intra-refresh, TRUE,
+ sliced-threads, TRUE,
+ speed-preset, 1, /* ultrafast */
+ NULL);
+break;
default:
spice_warning(unknown encoder type %d, encoder-base.codec_type);
reset_pipeline(encoder);
@@ -587,7 +601,8 @@ GstEncoder *create_gstreamer_encoder(SpiceVideoCodecType
codec_type, uint64_t st
spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
if (codec_type != SPICE_VIDEO_CODEC_TYPE_MJPEG
-codec_type != SPICE_VIDEO_CODEC_TYPE_VP8) {
+codec_type != SPICE_VIDEO_CODEC_TYPE_VP8
+codec_type != SPICE_VIDEO_CODEC_TYPE_H264) {
spice_warning(unsupported codec type %d, codec_type);
return NULL;
}
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index 08623e2..a034949 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -267,12 +267,14 @@ static create_video_encoder_proc video_encoder_procs[] = {
static const EnumNames video_codec_names[] = {
{SPICE_VIDEO_CODEC_TYPE_MJPEG, mjpeg},
{SPICE_VIDEO_CODEC_TYPE_VP8, vp8},
+{SPICE_VIDEO_CODEC_TYPE_H264, h264},
{0, NULL},
};
static const EnumNames video_cap_names[] = {
{SPICE_DISPLAY_CAP_CODEC_MJPEG, mjpeg},
{SPICE_DISPLAY_CAP_CODEC_VP8, vp8},
+{SPICE_DISPLAY_CAP_CODEC_H264, h264},
{0, NULL},
};
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 11/12] server: Avoid copying the input frame in the GStreamer encoder.
To do so we reference the source bitmap chunks in the GStreamer buffer and rely
on the buffer's lifetime being short enough.
Note that we can only avoid copies for the first 1 Mpixels or so. That's
because Spice splits larger frames into more chunks and we can fit memory
fragments inside a GStreamer buffer. So for those we will avoid copies for the
first 3840 KB and copy the rest.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This makes it possible to avoid copying the source frame when using
GStreamer 1.0. Paradoxically we are still forced to do some copying for
the larger frames.
Here is how it works: Spice's frame buffer is composed of multiple
memory chunks. So we use GStreamer's GstMemory objects to wrap each
chunk and put them all to form the GstBuffer we pass to the pipeline.
However there's a limit to the number of memory objects that we can put
in a GstBuffer. Usually that limit is 16. Furthermore Spice splits the
frame into 256KB chunks. so this approach only works up to 4MB or
between 1 and 1.3Mpixels. For larger frames we use the zero-copy
approach for the first 15 frame chunks, and copy all the rest into the
16th memory chunk.
So on my machine, for a 720x304 video the frame copy time goes from
around 265us (line-by-line) to 5us. For a 1440x900 frame it goes from
about 1610us with the old line-by-line copy code, down to 1490us for the
new chunk-by-chunk one, and down to about 440us when minimizing copies.
The latter matches well with the ~0.3Mpixels we have to copy after the
first 1Mpixels.
To avoid these copies it will be necessary to either increase the size
of the Spice chunks or to increase the number of memory objects that
GstBuffers can hold.
server/gstreamer_encoder.c | 108 +
1 file changed, 100 insertions(+), 8 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 97e6e13..9731ebb 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -33,6 +33,10 @@ typedef struct GstEncoder GstEncoder;
#define DEFAULT_FPS 30
+#ifndef HAVE_GSTREAMER_0_10
+# define DO_ZERO_COPY
+#endif
+
typedef struct {
SpiceBitmapFmt spice_format;
@@ -64,6 +68,9 @@ struct GstEncoder {
int height;
SpiceFormatForGStreamer *format;
SpiceBitmapFmt spice_format;
+#ifdef DO_ZERO_COPY
+gboolean needs_bitmap;
+#endif
/* Rate control information */
uint64_t bit_rate;
@@ -289,21 +296,28 @@ static void reconfigure_pipeline(GstEncoder *encoder)
}
}
+#ifdef DO_ZERO_COPY
+static void unref_bitmap(gpointer mem)
+{
+GstEncoder *encoder = (GstEncoder*)mem;
+encoder-needs_bitmap = FALSE;
+}
+#endif
+
static int push_raw_frame(GstEncoder *encoder, const SpiceBitmap *bitmap,
const SpiceRect *src, int top_down)
{
const uint32_t stream_height = src-bottom - src-top;
const uint32_t stream_stride = (src-right - src-left) *
encoder-format-bpp / 8;
uint32_t len = stream_stride * stream_height;
-GstBuffer *buffer = gst_buffer_new_and_alloc(len);
#ifdef HAVE_GSTREAMER_0_10
+GstBuffer *buffer = gst_buffer_new_and_alloc(len);
uint8_t *b = GST_BUFFER_DATA(buffer);
+uint8_t *dst = b;
#else
-GstMapInfo map;
-gst_buffer_map(buffer, map, GST_MAP_WRITE);
-uint8_t *b = map.data;
+GstBuffer *buffer = gst_buffer_new();
+GstMapInfo map = { .memory = NULL };
#endif
-uint8_t *dst = b;
/* Note that we should not reorder the lines, even if top_down is false.
* It just changes the number of lines to skip at the start of the bitmap.
@@ -315,6 +329,70 @@ static int push_raw_frame(GstEncoder *encoder, const
SpiceBitmap *bitmap,
if (stream_stride == bitmap-stride) {
/* We can copy the bitmap chunk by chunk */
+#ifdef DO_ZERO_COPY
+/* We cannot control the lifetime of the bitmap but we can wrap it in
+ * the buffer anyway because:
+ * - Before returning from gst_encoder_encode_frame() we wait for the
+ * pipeline to have converted this frame into a compressed buffer.
+ * So it has to have gone through the frame at least once.
+ * - For all encoders but MJPEG, the first element of the pipeline will
+ * convert the bitmap to another image format which entails copying
+ * it. So by the time the encoder starts its work, this buffer will
+ * not be needed anymore.
+ * - The MJPEG encoder does not perform inter-frame compression and
thus
+ * does not need to keep hold of this buffer once it has processed
it.
+ */
+while (chunk_offset = chunks-chunk[chunk].len) {
+/* Make sure that the chunk is not padded */
+if (chunks-chunk[chunk].len % bitmap-stride != 0) {
+gst_buffer_unref(buffer);
+return VIDEO_ENCODER_FRAME_UNSUPPORTED;
+}
+chunk_offset -= chunks-chunk
[Spice-devel] [PATCH 0/12] Add GStreamer support for video streams (take 3)
This follows up on the previous patch series with the following main improvements: - GStreamer 1.0 support. - Zero-copy support for the input frame. - H264 support. This time around I'm only sending the patches needed for the Spice server to limit the size of the series. To simplify testing I have put the spice-gtk patches on GitHub. Actually all the patches are there on their respective 'gst' branches. spice: https://github.com/fgouget/spice spice-gtk: https://github.com/fgouget/spice-gtk xf86-video-qxl: https://github.com/fgouget/xf86-video-qxl spice-common: https://github.com/fgouget/spice-common spice-protocol: https://github.com/fgouget/spice-protocol (there's also 'extras' branches with more experimental/future patches for the curious) For spice-html5 and QEMU one would have to refer to the patches posted previously on spice-devel. They should still work with this series. Let me know if there are changes that are needed for inclusion. Otherwise I think the next step is adding proper bitrate control. -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 1/12] server: Convert a couple of rate control checks into asserts in the mjpeg video encoder. (take 3)
The checks would lead the reader to think these functions can be called when
bit rate control is off when in fact they are only called when it is active.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This patch stands on its own and I think it makes sense even if the
remainder of the series is not applied. It seemed ok in round 1 so
again no change this time around. Let me know if it needs further
refinements for inclusion.
server/mjpeg_encoder.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 12447da..95d841f 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -607,9 +607,7 @@ static void
mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
uint32_t latency = 0;
uint32_t src_fps;
-if (!encoder-rate_control_is_active) {
-return;
-}
+spice_assert(encoder-rate_control_is_active);
rate_control = encoder-rate_control;
quality_eval = rate_control-quality_eval_data;
@@ -694,9 +692,8 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder,
uint64_t now)
MJpegEncoderRateControl *rate_control = encoder-rate_control;
uint64_t adjusted_fps_time_passed;
-if (!encoder-rate_control_is_active) {
-return;
-}
+spice_assert(encoder-rate_control_is_active);
+
adjusted_fps_time_passed = (now - rate_control-adjusted_fps_start_time) /
1000 / 1000;
if (!rate_control-during_quality_eval
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 2/12] server: Remove the rate_control_is_active field from MJpegEncoder. (take 3)
It is redundant with the corresponding callbacks.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This patch only depends on the first one and I think it makes sense even
if the remainder of the series is not applied. It seemed ok in round 1
so again no change this time around. Let me know if it needs further
refinements for inclusion.
server/mjpeg_encoder.c | 22 +-
server/mjpeg_encoder.h | 2 +-
server/red_worker.c| 4 ++--
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 95d841f..9a41ef3 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -164,7 +164,6 @@ struct MJpegEncoder {
unsigned int bytes_per_pixel; /* bytes per pixel of the input buffer */
void (*pixel_converter)(uint8_t *src, uint8_t *dest);
-int rate_control_is_active;
MJpegEncoderRateControl rate_control;
MJpegEncoderRateControlCbs cbs;
void *cbs_opaque;
@@ -185,21 +184,25 @@ static uint32_t get_min_required_playback_delay(uint64_t
frame_enc_size,
uint64_t byte_rate,
uint32_t latency);
-MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t
starting_bit_rate,
+static inline int rate_control_is_active(MJpegEncoder* encoder)
+{
+return encoder-cbs.get_roundtrip_ms != NULL;
+}
+
+MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
MJpegEncoderRateControlCbs *cbs, void *opaque)
{
MJpegEncoder *enc;
-spice_assert(!bit_rate_control || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
+spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
enc = spice_new0(MJpegEncoder, 1);
enc-first_frame = TRUE;
-enc-rate_control_is_active = bit_rate_control;
enc-rate_control.byte_rate = starting_bit_rate / 8;
enc-starting_bit_rate = starting_bit_rate;
-if (bit_rate_control) {
+if (cbs) {
struct timespec time;
clock_gettime(CLOCK_MONOTONIC, time);
@@ -211,6 +214,7 @@ MJpegEncoder *mjpeg_encoder_new(int bit_rate_control,
uint64_t starting_bit_rate
enc-rate_control.quality_eval_data.reason =
MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
enc-rate_control.warmup_start_time = ((uint64_t) time.tv_sec) *
10 + time.tv_nsec;
} else {
+enc-cbs.get_roundtrip_ms = NULL;
mjpeg_encoder_reset_quality(enc, MJPEG_LEGACY_STATIC_QUALITY_ID,
MJPEG_MAX_FPS, 0);
}
@@ -607,7 +611,7 @@ static void
mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
uint32_t latency = 0;
uint32_t src_fps;
-spice_assert(encoder-rate_control_is_active);
+spice_assert(rate_control_is_active(encoder));
rate_control = encoder-rate_control;
quality_eval = rate_control-quality_eval_data;
@@ -692,7 +696,7 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder,
uint64_t now)
MJpegEncoderRateControl *rate_control = encoder-rate_control;
uint64_t adjusted_fps_time_passed;
-spice_assert(encoder-rate_control_is_active);
+spice_assert(rate_control_is_active(encoder));
adjusted_fps_time_passed = (now - rate_control-adjusted_fps_start_time) /
1000 / 1000;
@@ -734,7 +738,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
SpiceBitmapFmt format,
{
uint32_t quality;
-if (encoder-rate_control_is_active) {
+if (rate_control_is_active(encoder)) {
MJpegEncoderRateControl *rate_control = encoder-rate_control;
struct timespec time;
uint64_t now;
@@ -1131,7 +1135,7 @@ void mjpeg_encoder_client_stream_report(MJpegEncoder
*encoder,
end_frame_mm_time - start_frame_mm_time,
end_frame_delay, audio_delay);
-if (!encoder-rate_control_is_active) {
+if (!rate_control_is_active(encoder)) {
spice_debug(rate control was not activated: ignoring);
return;
}
diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
index 741ea1c..d584b92 100644
--- a/server/mjpeg_encoder.h
+++ b/server/mjpeg_encoder.h
@@ -49,7 +49,7 @@ typedef struct MJpegEncoderStats {
double avg_quality;
} MJpegEncoderStats;
-MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t
starting_bit_rate,
+MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
MJpegEncoderRateControlCbs *cbs, void *opaque);
void mjpeg_encoder_destroy(MJpegEncoder *encoder);
diff --git a/server/red_worker.c b/server/red_worker.c
index 5deb30b..e0ff8e9 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3100,9 +3100,9 @@ static void
red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
mjpeg_cbs.update_client_playback_delay =
red_stream_update_client_playback_latency;
initial_bit_rate = red_stream_get_initial_bit_rate(dcc, stream);
-agent
[Spice-devel] [PATCH spice 12/12] server: Add h264 support to the GStreamer video encoder. (take 3b)
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Updated so it applies after the VP8 'take 3b' patch.
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index c638c3e..58c4c7e 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -206,6 +206,9 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
case SPICE_VIDEO_CODEC_TYPE_VP8:
gstenc_name = vp8enc;
break;
+case SPICE_VIDEO_CODEC_TYPE_H264:
+gstenc_name = x264enc;
+break;
default:
spice_warning(unsupported codec type %d, encoder-base.codec_type);
return FALSE;
@@ -262,6 +265,17 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
NULL);
break;
}
+case SPICE_VIDEO_CODEC_TYPE_H264:
+g_object_set(G_OBJECT(encoder-gstenc),
+ bitrate, encoder-bit_rate / 1024,
+ byte-stream, TRUE,
+ aud, FALSE,
+ tune, 4, /* Zero latency */
+ intra-refresh, TRUE,
+ sliced-threads, TRUE,
+ speed-preset, 1, /* ultrafast */
+ NULL);
+break;
default:
spice_warning(unknown encoder type %d, encoder-base.codec_type);
reset_pipeline(encoder);
@@ -601,7 +615,8 @@ GstEncoder *create_gstreamer_encoder(SpiceVideoCodecType
codec_type, uint64_t st
spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
if (codec_type != SPICE_VIDEO_CODEC_TYPE_MJPEG
-codec_type != SPICE_VIDEO_CODEC_TYPE_VP8) {
+codec_type != SPICE_VIDEO_CODEC_TYPE_VP8
+codec_type != SPICE_VIDEO_CODEC_TYPE_H264) {
spice_warning(unsupported codec type %d, codec_type);
return NULL;
}
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index 08623e2..a034949 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -267,12 +267,14 @@ static create_video_encoder_proc video_encoder_procs[] = {
static const EnumNames video_codec_names[] = {
{SPICE_VIDEO_CODEC_TYPE_MJPEG, mjpeg},
{SPICE_VIDEO_CODEC_TYPE_VP8, vp8},
+{SPICE_VIDEO_CODEC_TYPE_H264, h264},
{0, NULL},
};
static const EnumNames video_cap_names[] = {
{SPICE_DISPLAY_CAP_CODEC_MJPEG, mjpeg},
{SPICE_DISPLAY_CAP_CODEC_VP8, vp8},
+{SPICE_DISPLAY_CAP_CODEC_H264, h264},
{0, NULL},
};
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 7/12] server: Add VP8 support, a video codec preference list and compatibility checks with the Spice client. (take 3b)
The video encoder preferences can be expressed by building a semi-colon
separated list of
encoder:codec pairs. For instance 'gstreamer:vp8;spice:mjpeg' to pick first the
GStreamer VP8
video encoder first and used the original MJPEG video encoder one as a fallback.
The server has a default preference list which can also be selected by
specifying 'auto' as the
preferences list.
The client compatibility check relies on the following new capabilities:
* SPICE_DISPLAY_CAP_MULTI_CODEC which denotes a recent client that supports
multiple codecs.
This capability is needed to not have to hardcode that MJPEG is supported. This
makes it possible
to write clients that don't support MJPEG.
* SPICE_DISPLAY_CAP_CODEC_XXX, where XXX is a supported codec, for now MJPEG
and VP8.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
On Thu, 11 Jun 2015, Fabio Fantoni wrote:
[...]
../../server/.libs/libspice-server.so: undefined reference to
`g_get_num_processors'
toso in spice chat told that was added in glib 2.36check
pkg-config
--modversion glib-2.0 (this should probably bump the configure version
for glib tho)
I think it's better to check directly for the presence of
g_get_num_processors(). That way it works even if some distribution
decides to backport it or removes it later on. Here is an updated patch.
Note that this will impact the GStreamer 1.0 and h264 patches that
follow.
diff --git a/configure.ac b/configure.ac
index 34e1a4a..ed3a337 100644
--- a/configure.ac
+++ b/configure.ac
@@ -154,6 +154,10 @@ fi
PKG_CHECK_MODULES([GLIB2], [glib-2.0 = 2.22])
AS_VAR_APPEND([SPICE_REQUIRES], [ glib-2.0 = 2.22])
+AC_CHECK_LIB(glib-2.0, g_get_num_processors,
+ AC_DEFINE([HAVE_G_GET_NUMPROCESSORS], 1, [Defined if we have
g_get_num_processors()]),,
+ $GLIB2_LIBS)
+
PKG_CHECK_MODULES(PIXMAN, pixman-1 = 0.17.7)
AC_SUBST(PIXMAN_CFLAGS)
AC_SUBST(PIXMAN_LIBS)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 3600b1b..8fa9071 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -178,10 +178,27 @@ static gboolean set_appsrc_caps(GstEncoder *encoder)
static gboolean construct_pipeline(GstEncoder *encoder, const SpiceBitmap
*bitmap)
{
+gboolean no_clock = FALSE;
+const gchar* gstenc_name;
+switch (encoder-base.codec_type)
+{
+case SPICE_VIDEO_CODEC_TYPE_MJPEG:
+gstenc_name = ffenc_mjpeg;
+no_clock = TRUE;
+break;
+case SPICE_VIDEO_CODEC_TYPE_VP8:
+gstenc_name = vp8enc;
+break;
+default:
+spice_warning(unsupported codec type %d, encoder-base.codec_type);
+return FALSE;
+}
+
GError *err = NULL;
-const gchar* desc = appsrc name=src format=2 do-timestamp=true !
ffmpegcolorspace ! ffenc_mjpeg name=encoder ! appsink name=sink;
+gchar *desc = g_strdup_printf(appsrc name=src format=2 do-timestamp=true
! ffmpegcolorspace ! %s name=encoder ! appsink name=sink, gstenc_name);
spice_debug(GStreamer pipeline: %s, desc);
encoder-pipeline = gst_parse_launch_full(desc, NULL,
GST_PARSE_FLAG_FATAL_ERRORS, err);
+g_free(desc);
if (!encoder-pipeline) {
spice_warning(GStreamer error: %s, err-message);
g_clear_error(err);
@@ -191,19 +208,36 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
encoder-gstenc = gst_bin_get_by_name(GST_BIN(encoder-pipeline),
encoder);
encoder-appsink =
GST_APP_SINK(gst_bin_get_by_name(GST_BIN(encoder-pipeline), sink));
+/* Set the encoder initial bit rate, and ask for a low latency */
+adjust_bit_rate(encoder);
+g_object_set(G_OBJECT(encoder-gstenc), bitrate, encoder-bit_rate,
NULL);
+if (encoder-base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8) {
+/* See http://www.webmproject.org/docs/encoder-parameters/ */
+#ifdef HAVE_G_GET_NUMPROCESSORS
+int core_count = g_get_num_processors();
+#else
+int core_count = 1;
+#endif
+g_object_set(G_OBJECT(encoder-gstenc),
+ mode, 1, /* CBR */
+ max-latency, 0,
+ speed, 7,
+ resize-allowed, TRUE,
+ threads, core_count - 1,
+ NULL);
+ }
+
/* Set the source caps */
encoder-src_caps = NULL;
if (!set_appsrc_caps(encoder)) {
return FALSE;
}
-/* Set the encoder initial bit rate */
-adjust_bit_rate(encoder);
-g_object_set(G_OBJECT(encoder-gstenc), bitrate, encoder-bit_rate,
NULL);
-
/* Start playing */
-spice_debug(removing the pipeline clock);
-gst_pipeline_use_clock(GST_PIPELINE(encoder-pipeline), NULL);
+if (no_clock) {
+spice_debug(removing the pipeline clock);
+gst_pipeline_use_clock(GST_PIPELINE(encoder-pipeline), NULL);
+}
spice_debug(setting state to PLAYING);
if (gst_element_set_state(encoder-pipeline, GST_STATE_PLAYING
[Spice-devel] [PATCH spice 10/12] server: Add GStreamer 1.0 support. (take 3b)
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Updated so it applies after the new VP8 patch.
diff --git a/configure.ac b/configure.ac
index ed3a337..6a5ddb9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -92,14 +92,32 @@ if test x$enable_smartcard = xyes; then
fi
AC_ARG_ENABLE(gstreamer,
- AS_HELP_STRING([--enable-gstreamer=@:@auto/yes/no@:@],
- [Enable GStreamer 0.10 support]),
+ AS_HELP_STRING([--enable-gstreamer=@:@auto/0.10/1.0/yes/no@:@],
+ [Enable GStreamer support]),
[],
[enable_gstreamer=auto])
-if test x$enable_gstreamer != xno; then
+if test x$enable_gstreamer != xno test x$enable_gstreamer != x0.10;
then
+PKG_CHECK_MODULES(GSTREAMER_1_0, [gstreamer-1.0, gstreamer-app-1.0],
+ [enable_gstreamer=1.0
+ have_gstreamer_1_0=yes],
+ [have_gstreamer_1_0=no])
+if test x$have_gstreamer_1_0 = xyes; then
+AC_SUBST(GSTREAMER_1_0_CFLAGS)
+AC_SUBST(GSTREAMER_1_0_LIBS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-1.0 gstreamer-app-1.0])
+AC_DEFINE([HAVE_GSTREAMER_1_0], [1], [Define if supporting GStreamer
1.0])
+elif test x$enable_gstreamer = x1.0; then
+AC_MSG_ERROR([GStreamer 1.0 support requested but not found. You may
set GSTREAMER_1_0_CFLAGS and GSTREAMER_1_0_LIBS to avoid the need to call
pkg-config.])
+fi
+else
+have_gstreamer_1_0=no
+fi
+AM_CONDITIONAL(SUPPORT_GSTREAMER_1_0, test x$have_gstreamer_1_0 = xyes)
+
+if test x$enable_gstreamer != xno test x$enable_gstreamer != x1.0;
then
PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10],
- [enable_gstreamer=yes
+ [enable_gstreamer=0.10
have_gstreamer_0_10=yes],
[have_gstreamer_0_10=no])
if test x$have_gstreamer_0_10 = xyes; then
@@ -107,7 +125,7 @@ if test x$enable_gstreamer != xno; then
AC_SUBST(GSTREAMER_0_10_LIBS)
AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10 gstreamer-app-0.10])
AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer
0.10])
-elif test x$enable_gstreamer = xyes; then
+elif test x$enable_gstreamer = x0.10; then
AC_MSG_ERROR([GStreamer 0.10 support requested but not found. You may
set GSTREAMER_0_10_CFLAGS and GSTREAMER_0_10_LIBS to avoid the need to call
pkg-config.])
fi
else
@@ -115,6 +133,11 @@ else
fi
AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$have_gstreamer_0_10 = xyes)
+if test x$enable_gstreamer = xyes; then
+AC_MSG_ERROR(GStreamer support requested but not found)
+fi
+AS_IF([test x$enable_gstreamer = xauto], [enable_gstreamer=no])
+
AC_ARG_ENABLE(automated_tests,
[ --enable-automated-tests Enable automated tests using spicy-screenshot
(part of spice--gtk)],,
[enable_automated_tests=no])
@@ -386,7 +409,7 @@ echo
Smartcard:${enable_smartcard}
-GStreamer 0.10: ${have_gstreamer_0_10}
+GStreamer:${enable_gstreamer}
SASL support: ${enable_sasl}
diff --git a/server/Makefile.am b/server/Makefile.am
index 4921bc3..9fb0c8e 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -12,6 +12,7 @@ AM_CPPFLAGS = \
$(SLIRP_CFLAGS) \
$(SMARTCARD_CFLAGS) \
$(GSTREAMER_0_10_CFLAGS)\
+ $(GSTREAMER_1_0_CFLAGS) \
$(SSL_CFLAGS) \
$(VISIBILITY_HIDDEN_CFLAGS) \
$(WARN_CFLAGS) \
@@ -42,7 +43,8 @@ libspice_server_la_LIBADD =
\
$(PIXMAN_LIBS) \
$(SASL_LIBS)\
$(SLIRP_LIBS) \
- $(GSTREAMER_0_10_LIBS)
\
+ $(GSTREAMER_0_10_LIBS) \
+ $(GSTREAMER_1_0_LIBS) \
$(SSL_LIBS) \
$(Z_LIBS) \
$(SPICE_NONPKGCONFIG_LIBS) \
@@ -142,7 +144,13 @@ endif
if SUPPORT_GSTREAMER_0_10
libspice_server_la_SOURCES += \
- gstreamer_encoder.c \
+ gstreamer_encoder.c \
+ $(NULL)
+endif
+
+if SUPPORT_GSTREAMER_1_0
+libspice_server_la_SOURCES += \
+ gstreamer_encoder.c \
$(NULL)
endif
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 8fa9071..0743879 100644
--- a/server
[Spice-devel] [PATCH protocol 6/12] Add support for the VP8 and H264 video codecs and for advertising supported video codecs. (take 3)
Clients that support multiple codecs should advertise the
SPICE_DISPLAY_CAP_MULTI_CODEC capability and one SPICE_DISPLAY_CAP_CODEC_XXX
per supported codec.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This should be followed by a spice-common commit that ensures we get the
right version of these headers.
Changes since take 2:
- This patch also adds h264.
spice/enums.h| 2 ++
spice/protocol.h | 4
2 files changed, 6 insertions(+)
diff --git a/spice/enums.h b/spice/enums.h
index 18e2f74..2bb2739 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -139,6 +139,8 @@ typedef enum SpicePathFlags {
typedef enum SpiceVideoCodecType {
SPICE_VIDEO_CODEC_TYPE_MJPEG = 1,
+SPICE_VIDEO_CODEC_TYPE_VP8,
+SPICE_VIDEO_CODEC_TYPE_H264,
SPICE_VIDEO_CODEC_TYPE_ENUM_END
} SpiceVideoCodecType;
diff --git a/spice/protocol.h b/spice/protocol.h
index bea376c..d53642a 100644
--- a/spice/protocol.h
+++ b/spice/protocol.h
@@ -134,6 +134,10 @@ enum {
SPICE_DISPLAY_CAP_A8_SURFACE,
SPICE_DISPLAY_CAP_STREAM_REPORT,
SPICE_DISPLAY_CAP_LZ4_COMPRESSION,
+SPICE_DISPLAY_CAP_MULTI_CODEC,
+SPICE_DISPLAY_CAP_CODEC_MJPEG,
+SPICE_DISPLAY_CAP_CODEC_VP8,
+SPICE_DISPLAY_CAP_CODEC_H264,
};
enum {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH qxl 8/12] spiceqxl: Add SpiceVideoCodecs and $XSPICE_VIDEO_CODECS for specifying video codec preferences. (take 3)
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Changes since take 1:
- Fixed a brace placement.
examples/spiceqxl.xorg.conf.example | 7 +++
src/qxl.h | 1 +
src/qxl_driver.c| 2 ++
src/spiceqxl_spice_server.c | 15 +++
4 files changed, 25 insertions(+)
diff --git a/examples/spiceqxl.xorg.conf.example
b/examples/spiceqxl.xorg.conf.example
index d15f7f2..a82c2be 100644
--- a/examples/spiceqxl.xorg.conf.example
+++ b/examples/spiceqxl.xorg.conf.example
@@ -51,6 +51,13 @@ Section Device
# defaults to filter.
#Option SpiceStreamingVideo
+# Set video codecs to use. Provide a semicolon list of
+# codecs, in preference order. Each codec requires an encoder
+# which can be one of spice or gstreamer, and then a codec type,
+# for instance mjpeg or vp8. The default is spice:mjpeg,
+# which uses the builtin mjpeg encoder.
+#Option SpiceVideoCodecs
+
# Set zlib glz wan compression. Options are auto, never, always.
# defaults to auto.
#Option SpiceZlibGlzWanCompression
diff --git a/src/qxl.h b/src/qxl.h
index ff55604..5cc8d05 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -158,6 +158,7 @@ enum {
OPTION_SURFACE_BUFFER_SIZE,
OPTION_COMMAND_BUFFER_SIZE,
OPTION_SPICE_SMARTCARD_FILE,
+OPTION_SPICE_VIDEO_CODECS,
#endif
OPTION_COUNT,
};
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index ce0a88e..d036dac 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -155,6 +155,8 @@ const OptionInfoRec DefaultOptions[] =
CommandBufferSize,OPTV_INTEGER,
{DEFAULT_COMMAND_BUFFER_SIZE}, FALSE},
{ OPTION_SPICE_SMARTCARD_FILE,
SpiceSmartcardFile, OPTV_STRING,{0}, FALSE},
+{ OPTION_SPICE_VIDEO_CODECS,
+ SpiceVideoCodecs, OPTV_STRING,{0}, FALSE},
#endif
{ -1, NULL, OPTV_NONE, {0}, FALSE }
diff --git a/src/spiceqxl_spice_server.c b/src/spiceqxl_spice_server.c
index 14ee752..2f39561 100644
--- a/src/spiceqxl_spice_server.c
+++ b/src/spiceqxl_spice_server.c
@@ -173,6 +173,9 @@ void xspice_set_spice_server_options(OptionInfoPtr options)
const char *streaming_video =
get_str_option(options, OPTION_SPICE_STREAMING_VIDEO,
XSPICE_STREAMING_VIDEO);
+const char *video_codecs =
+get_str_option(options, OPTION_SPICE_VIDEO_CODECS,
+ XSPICE_VIDEO_CODECS);
int agent_mouse =
get_bool_option(options, OPTION_SPICE_AGENT_MOUSE,
XSPICE_AGENT_MOUSE);
@@ -295,6 +298,18 @@ void xspice_set_spice_server_options(OptionInfoPtr options)
spice_server_set_streaming_video(spice_server, streaming_video_opt);
}
+if (video_codecs) {
+#if SPICE_SERVER_VERSION = 0x000c06 /* 0.12.6 */
+if (spice_server_set_video_codecs(spice_server, video_codecs)) {
+fprintf(stderr, spice: invalid video encoder %s\n, video_codecs);
+exit(1);
+}
+#else
+fprintf(stderr, spice: video_codecs are not available (spice =
0.12.6 required)\n);
+exit(1);
+#endif
+}
+
spice_server_set_agent_mouse
(spice_server, agent_mouse);
spice_server_set_playback_compression
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH qxl 9/12] Xspice: Add a --video-codecs option to specify which encoder:codec to use.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This adds a --video-codecs option to Xspice as an alternative to
$XSPICE_VIDEO_CODECS and the SpiceVideoCodecs xorg.conf setting.
scripts/Xspice | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/Xspice b/scripts/Xspice
index 281535d..02875f5 100755
--- a/scripts/Xspice
+++ b/scripts/Xspice
@@ -86,6 +86,7 @@ parser.add_argument('--zlib-glz-wan-compression',
# TODO - sound support
parser.add_argument('--streaming-video', choices=['off', 'all', 'filter'],
default='filter', help='filter by default')
+parser.add_argument('--video-codecs', help=Sets a semicolon-separated list of
preferred video codecs to use. Each takes the form encoder:codec, with
spice:mjpeg being the default and other options being provided by gstreamer for
the mjpeg, vp8 and h264 codecs.)
add_boolean('--ipv4-only')
add_boolean('--ipv6-only')
parser.add_argument('--vdagent', action='store_true', dest='vdagent_enabled',
default=False, help='launch vdagent vdagentd. They provide clipboard
resolution automation')
@@ -251,7 +252,7 @@ var_args = ['port', 'tls_port', 'disable_ticketing',
'x509_key_file', 'x509_key_password',
'tls_ciphers', 'dh_file', 'password', 'image_compression',
'jpeg_wan_compression', 'zlib_glz_wan_compression',
-'streaming_video', 'deferred_fps', 'exit_on_disconnect',
+'streaming_video', 'video_codecs', 'deferred_fps', 'exit_on_disconnect',
'vdagent_enabled', 'vdagent_virtio_path', 'vdagent_uinput_path',
'vdagent_uid', 'vdagent_gid']
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 10/12] server: Add GStreamer 1.0 support.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This adds GStreamer 1.0 support!
By default GStreamer 1.0 is used if available, otherwise GStreamer 0.10
is used and Spice is compiled without GStreamer support as a last
resort. It's possible to explicitly require a specific Gstreamer version
with --enable-gstreamer=1.0 and --enable-gstreamer=0.10, or for any
version with --enable-gstreamer=yes.
If you get an error while building the pipeline for MJPEG, then it
probably means you need the fix for this bug:
https://bugzilla.gnome.org/show_bug.cgi?id=750398
configure.ac | 35 +
server/Makefile.am | 12 ++--
server/gstreamer_encoder.c | 77 --
server/red_dispatcher.c| 2 +-
4 files changed, 108 insertions(+), 18 deletions(-)
diff --git a/configure.ac b/configure.ac
index 34e1a4a..3fc46f9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -92,14 +92,32 @@ if test x$enable_smartcard = xyes; then
fi
AC_ARG_ENABLE(gstreamer,
- AS_HELP_STRING([--enable-gstreamer=@:@auto/yes/no@:@],
- [Enable GStreamer 0.10 support]),
+ AS_HELP_STRING([--enable-gstreamer=@:@auto/0.10/1.0/yes/no@:@],
+ [Enable GStreamer support]),
[],
[enable_gstreamer=auto])
-if test x$enable_gstreamer != xno; then
+if test x$enable_gstreamer != xno test x$enable_gstreamer != x0.10;
then
+PKG_CHECK_MODULES(GSTREAMER_1_0, [gstreamer-1.0, gstreamer-app-1.0],
+ [enable_gstreamer=1.0
+ have_gstreamer_1_0=yes],
+ [have_gstreamer_1_0=no])
+if test x$have_gstreamer_1_0 = xyes; then
+AC_SUBST(GSTREAMER_1_0_CFLAGS)
+AC_SUBST(GSTREAMER_1_0_LIBS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-1.0 gstreamer-app-1.0])
+AC_DEFINE([HAVE_GSTREAMER_1_0], [1], [Define if supporting GStreamer
1.0])
+elif test x$enable_gstreamer = x1.0; then
+AC_MSG_ERROR([GStreamer 1.0 support requested but not found. You may
set GSTREAMER_1_0_CFLAGS and GSTREAMER_1_0_LIBS to avoid the need to call
pkg-config.])
+fi
+else
+have_gstreamer_1_0=no
+fi
+AM_CONDITIONAL(SUPPORT_GSTREAMER_1_0, test x$have_gstreamer_1_0 = xyes)
+
+if test x$enable_gstreamer != xno test x$enable_gstreamer != x1.0;
then
PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10],
- [enable_gstreamer=yes
+ [enable_gstreamer=0.10
have_gstreamer_0_10=yes],
[have_gstreamer_0_10=no])
if test x$have_gstreamer_0_10 = xyes; then
@@ -107,7 +125,7 @@ if test x$enable_gstreamer != xno; then
AC_SUBST(GSTREAMER_0_10_LIBS)
AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10 gstreamer-app-0.10])
AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer
0.10])
-elif test x$enable_gstreamer = xyes; then
+elif test x$enable_gstreamer = x0.10; then
AC_MSG_ERROR([GStreamer 0.10 support requested but not found. You may
set GSTREAMER_0_10_CFLAGS and GSTREAMER_0_10_LIBS to avoid the need to call
pkg-config.])
fi
else
@@ -115,6 +133,11 @@ else
fi
AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$have_gstreamer_0_10 = xyes)
+if test x$enable_gstreamer = xyes; then
+AC_MSG_ERROR(GStreamer support requested but not found)
+fi
+AS_IF([test x$enable_gstreamer = xauto], [enable_gstreamer=no])
+
AC_ARG_ENABLE(automated_tests,
[ --enable-automated-tests Enable automated tests using spicy-screenshot
(part of spice--gtk)],,
[enable_automated_tests=no])
@@ -382,7 +405,7 @@ echo
Smartcard:${enable_smartcard}
-GStreamer 0.10: ${have_gstreamer_0_10}
+GStreamer:${enable_gstreamer}
SASL support: ${enable_sasl}
diff --git a/server/Makefile.am b/server/Makefile.am
index 4921bc3..9fb0c8e 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -12,6 +12,7 @@ AM_CPPFLAGS = \
$(SLIRP_CFLAGS) \
$(SMARTCARD_CFLAGS) \
$(GSTREAMER_0_10_CFLAGS)\
+ $(GSTREAMER_1_0_CFLAGS) \
$(SSL_CFLAGS) \
$(VISIBILITY_HIDDEN_CFLAGS) \
$(WARN_CFLAGS) \
@@ -42,7 +43,8 @@ libspice_server_la_LIBADD =
\
$(PIXMAN_LIBS) \
$(SASL_LIBS)\
$(SLIRP_LIBS) \
- $(GSTREAMER_0_10_LIBS)
\
+ $(GSTREAMER_0_10_LIBS
Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support
On Thu, 28 May 2015, Fabio Fantoni wrote: [...] Thanks for reply, I'm preparing spice-gtk with vp8 patches armhf for test it on arm thin client (for example raspberry 2), I want try to use gst-omx for hardware decoding of mjpeg and vp8, can I simply build and install gst-omx instead of gst-ffmpeg or a change in spice-gtk is needed for use gst-omx? I have patches that could probably be useful to you for this: a simplification of the pipeline creation, optionally the use of decoebin, and for the performance aspects zero-copy for both the input buffer and the ouput frame. So I have created some github repositories to make it easier to get these: https://github.com/fgouget/spice-gtk.git * gst branch All the above except for decodebin. * decodebin branch Uses decodebin instead of manually picking the decoders. However on my machine decodebin tries to use vaapi which, given my test configuration, immediately triggers an assert, thus killing the process. Uninstalling vaapi fixes that but it's annoying. So I don't really like the decodebin approach though it's probably more a vaapi bug than a decodebin issue. https://github.com/fgouget/spice.git * gst branch The patches adding GStreamer and VP8 support. * gst-1.0 branch Experimental branch with non-functional GStreamer 1.0 and h264 code. Note that GStreamer 1.0 support can be disabled using './configure --disable-gstreamer-1.0' so H264 support can be tested too. https://github.com/fgouget/xf86-video-qxl.git * gst branch Has the patch to make it possible to specify the encoder:codec pairs in either SpiceVideoCodecs or XSPICE_VIDEO_CODECS. There are also spice-common and spice-protocol repositories but these should get pulled automatically. -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support
On Fri, 29 May 2015, Fabio Fantoni wrote: [...] I'm trying the gst1 support but build fails with this error: gstreamer_encoder.c: In function 'construct_pipeline': gstreamer_encoder.c:219:41: error: 'GstEncoder' has no member named 'frc' deadline, encoder-frc.period * 1000 / 2, Why you tell that 1.0 is not functional? I suppose not only for the build error I had. It's a merge bug. Sorry. I have pushed an update that fixes this so you may want to rebase. The initial push had caps issues, mostly it was missing format=BGRx, which caused GStreamer 1.0 not to work. Note that the failure mode caused the video to be sent through the regular non-stream channel so it looked like it worked. Now the GStreamer 1.0 situation is as follows: * VP8 works. * MJPEG fails because of a caps negotiation error despite the videoconvert element. * H264 seems to work but the client fails to display it. So I'm not totally sure. Also I finally found the right parameters to improve the VP8 encoder performance. Cheers, -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support
On Mon, 1 Jun 2015, Fabio Fantoni wrote: [...] I suppose that these warning I saw on client are related: http://pastebin.com/GeTPyscG 0:00:00.150227452 27501 0x29f98f0 WARNvideodecoder gstvideodecoder.c:2026:gst_video_decoder_chain:vp8dec0 Received buffer without a new-segment. Assuming timestamps start from 0. 0:00:02.889876193 27501 0x29f9ed0 WARNvideodecoder gstvideodecoder.c:2026:gst_video_decoder_chain:vp8dec1 Received buffer without a new-segment. Assuming timestamps start from 0. It would be cleaner not to have these warnings (and I'll try to fix them), but I don't think they can cause display artifacts. What kind of video application are you using for testing? Flash/YouTube? A local file with a media application like mplayer? -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 3/7] server: Refactor the Spice server video encoding so alternative implementations can be added. (take 2)
) {
agent-frames--;
#ifdef STREAM_STATS
@@ -8603,33 +8539,29 @@ static inline int
red_marshall_stream_data(RedChannelClient *rcc,
frame_mm_time = drawable-red_drawable-mm_time ?
drawable-red_drawable-mm_time :
reds_get_mm_time();
+
outbuf_size = dcc-send_data.stream_outbuf_size;
-ret = mjpeg_encoder_start_frame(agent-mjpeg_encoder,
image-u.bitmap.format,
-width, height,
-dcc-send_data.stream_outbuf,
-outbuf_size,
-frame_mm_time);
+ret = agent-video_encoder-encode_frame(agent-video_encoder,
+image-u.bitmap, width, height,
+drawable-red_drawable-u.copy.src_area,
+stream-top_down, frame_mm_time,
+dcc-send_data.stream_outbuf, outbuf_size, n);
+
switch (ret) {
-case MJPEG_ENCODER_FRAME_DROP:
-spice_assert(dcc-use_mjpeg_encoder_rate_control);
+case VIDEO_ENCODER_FRAME_DROP:
+spice_assert(dcc-use_video_encoder_rate_control);
#ifdef STREAM_STATS
agent-stats.num_drops_fps++;
#endif
return TRUE;
-case MJPEG_ENCODER_FRAME_UNSUPPORTED:
+case VIDEO_ENCODER_FRAME_UNSUPPORTED:
return FALSE;
-case MJPEG_ENCODER_FRAME_ENCODE_START:
+case VIDEO_ENCODER_FRAME_ENCODE_DONE:
break;
default:
-spice_error(bad return value (%d) from mjpeg_encoder_start_frame,
ret);
-return FALSE;
-}
-
-if (!encode_frame(dcc, drawable-red_drawable-u.copy.src_area,
- image-u.bitmap, stream)) {
+spice_error(bad return value (%d) from VideoEncoder::encode_frame,
ret);
return FALSE;
}
-n = mjpeg_encoder_end_frame(agent-mjpeg_encoder);
dcc-send_data.stream_outbuf_size = outbuf_size;
if (!drawable-sized_stream) {
@@ -10255,7 +10187,7 @@ static int
display_channel_handle_stream_report(DisplayChannelClient *dcc,
return FALSE;
}
stream_agent = dcc-stream_agents[stream_report-stream_id];
-if (!stream_agent-mjpeg_encoder) {
+if (!stream_agent-video_encoder) {
spice_info(stream_report: no encoder for stream id %u.
Probably the stream has been destroyed,
stream_report-stream_id);
return TRUE;
@@ -10266,7 +10198,7 @@ static int
display_channel_handle_stream_report(DisplayChannelClient *dcc,
stream_agent-report_id, stream_report-unique_id);
return TRUE;
}
-mjpeg_encoder_client_stream_report(stream_agent-mjpeg_encoder,
+
stream_agent-video_encoder-client_stream_report(stream_agent-video_encoder,
stream_report-num_frames,
stream_report-num_drops,
stream_report-start_frame_mm_time,
diff --git a/server/video_encoder.h b/server/video_encoder.h
new file mode 100644
index 000..d07138b
--- /dev/null
+++ b/server/video_encoder.h
@@ -0,0 +1,162 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ Copyright (C) 2009 Red Hat, Inc.
+ Copyright (C) 2015 Jeremy White
+ Copyright (C) 2015 Francois Gouget
+
+ 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 A
HREF=http://www.gnu.org/licenses/;http://www.gnu.org/licenses//A.
+*/
+
+#ifndef _H_video_encoder
+#define _H_video_encoder
+
+#include red_common.h
+
+enum {
+VIDEO_ENCODER_FRAME_UNSUPPORTED = -1,
+VIDEO_ENCODER_FRAME_DROP,
+VIDEO_ENCODER_FRAME_ENCODE_DONE,
+};
+
+typedef struct VideoEncoderStats {
+uint64_t starting_bit_rate;
+uint64_t cur_bit_rate;
+double avg_quality;
+} VideoEncoderStats;
+
+typedef struct VideoEncoder VideoEncoder;
+#ifndef VIDEO_ENCODER_T
+# define VIDEO_ENCODER_T VideoEncoder
+#endif
+
+struct VideoEncoder {
+/* Releases the video encoder's resources */
+void (*destroy)(VIDEO_ENCODER_T *encoder);
+
+/* Compresses the specified src image area into the outbuf buffer.
+ *
+ * @encoder: The video encoder.
+ * @bitmap:The Spice screen.
+ * @width: The width of the video area. This always matches src.
+ * @height:The height of the video area. This always matches src.
+ * @src: A rectangle specifying the area occupied by the video.
+ * @top_down: If true
[Spice-devel] [PATCH spice 2/7] server: Remove the rate_control_is_active field from MJpegEncoder. (take 2)
It is redundant with the corresponding callbacks.
---
As far as I can tell this patch was ok in the previous round so no
change this time around.
server/mjpeg_encoder.c | 22 +-
server/mjpeg_encoder.h | 2 +-
server/red_worker.c| 4 ++--
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 95d841f..9a41ef3 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -164,7 +164,6 @@ struct MJpegEncoder {
unsigned int bytes_per_pixel; /* bytes per pixel of the input buffer */
void (*pixel_converter)(uint8_t *src, uint8_t *dest);
-int rate_control_is_active;
MJpegEncoderRateControl rate_control;
MJpegEncoderRateControlCbs cbs;
void *cbs_opaque;
@@ -185,21 +184,25 @@ static uint32_t get_min_required_playback_delay(uint64_t
frame_enc_size,
uint64_t byte_rate,
uint32_t latency);
-MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t
starting_bit_rate,
+static inline int rate_control_is_active(MJpegEncoder* encoder)
+{
+return encoder-cbs.get_roundtrip_ms != NULL;
+}
+
+MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
MJpegEncoderRateControlCbs *cbs, void *opaque)
{
MJpegEncoder *enc;
-spice_assert(!bit_rate_control || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
+spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
enc = spice_new0(MJpegEncoder, 1);
enc-first_frame = TRUE;
-enc-rate_control_is_active = bit_rate_control;
enc-rate_control.byte_rate = starting_bit_rate / 8;
enc-starting_bit_rate = starting_bit_rate;
-if (bit_rate_control) {
+if (cbs) {
struct timespec time;
clock_gettime(CLOCK_MONOTONIC, time);
@@ -211,6 +214,7 @@ MJpegEncoder *mjpeg_encoder_new(int bit_rate_control,
uint64_t starting_bit_rate
enc-rate_control.quality_eval_data.reason =
MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
enc-rate_control.warmup_start_time = ((uint64_t) time.tv_sec) *
10 + time.tv_nsec;
} else {
+enc-cbs.get_roundtrip_ms = NULL;
mjpeg_encoder_reset_quality(enc, MJPEG_LEGACY_STATIC_QUALITY_ID,
MJPEG_MAX_FPS, 0);
}
@@ -607,7 +611,7 @@ static void
mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
uint32_t latency = 0;
uint32_t src_fps;
-spice_assert(encoder-rate_control_is_active);
+spice_assert(rate_control_is_active(encoder));
rate_control = encoder-rate_control;
quality_eval = rate_control-quality_eval_data;
@@ -692,7 +696,7 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder,
uint64_t now)
MJpegEncoderRateControl *rate_control = encoder-rate_control;
uint64_t adjusted_fps_time_passed;
-spice_assert(encoder-rate_control_is_active);
+spice_assert(rate_control_is_active(encoder));
adjusted_fps_time_passed = (now - rate_control-adjusted_fps_start_time) /
1000 / 1000;
@@ -734,7 +738,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
SpiceBitmapFmt format,
{
uint32_t quality;
-if (encoder-rate_control_is_active) {
+if (rate_control_is_active(encoder)) {
MJpegEncoderRateControl *rate_control = encoder-rate_control;
struct timespec time;
uint64_t now;
@@ -1131,7 +1135,7 @@ void mjpeg_encoder_client_stream_report(MJpegEncoder
*encoder,
end_frame_mm_time - start_frame_mm_time,
end_frame_delay, audio_delay);
-if (!encoder-rate_control_is_active) {
+if (!rate_control_is_active(encoder)) {
spice_debug(rate control was not activated: ignoring);
return;
}
diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
index 741ea1c..d584b92 100644
--- a/server/mjpeg_encoder.h
+++ b/server/mjpeg_encoder.h
@@ -49,7 +49,7 @@ typedef struct MJpegEncoderStats {
double avg_quality;
} MJpegEncoderStats;
-MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t
starting_bit_rate,
+MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
MJpegEncoderRateControlCbs *cbs, void *opaque);
void mjpeg_encoder_destroy(MJpegEncoder *encoder);
diff --git a/server/red_worker.c b/server/red_worker.c
index 5deb30b..e0ff8e9 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3100,9 +3100,9 @@ static void
red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
mjpeg_cbs.update_client_playback_delay =
red_stream_update_client_playback_latency;
initial_bit_rate = red_stream_get_initial_bit_rate(dcc, stream);
-agent-mjpeg_encoder = mjpeg_encoder_new(TRUE, initial_bit_rate,
mjpeg_cbs, agent);
+agent-mjpeg_encoder = mjpeg_encoder_new(initial_bit_rate, mjpeg_cbs,
agent);
} else {
-
[Spice-devel] [PATCH common 5/7] Add support for the VP8 video codec. (take 2)
---
Thanks to Fabio Fantoni for noticing that spice.proto needed updating
too. I think I did not notice the issue because the top-level autogen.sh
keeps rechecking out spice-protocol so spice-protocol/spice/enums.h
ended up always being newer than spice.proto.
spice.proto | 1 +
1 file changed, 1 insertion(+)
diff --git a/spice.proto b/spice.proto
index 01493c9..ecc6308 100644
--- a/spice.proto
+++ b/spice.proto
@@ -329,6 +329,7 @@ flags8 path_flags { /* TODO: C enum names changes */
enum8 video_codec_type {
MJPEG = 1,
+VP8,
};
flags8 stream_flags {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 7/7] server: Add VP8 support, a video codec preference list and compatibility checks with the Spice client. (take 2)
;
+static RedVideoCodec new_codecs[RED_MAX_VIDEO_CODECS];
+int count;
+const char* c;
-if (num_renderers == RED_MAX_RENDERERS || !(inf = find_renderer(name))) {
-return FALSE;
+if (strcmp(codecs, auto) == 0)
+codecs = VIDEO_ENCODER_DEFAULT_PREFERENCE;
+
+c = codecs;
+count = 0;
+while ( (c = parse_video_codecs(c, encoder_name, codec_name)) ) {
+int convert_failed = FALSE;
+if (count == RED_MAX_VIDEO_CODECS ||
+!name_to_enum(video_encoder_names, encoder_name, encoder) ||
+!name_to_enum(video_codec_names, codec_name, type) ||
+!name_to_enum(video_cap_names, codec_name, cap))
+convert_failed = TRUE;
+
+free(encoder_name);
+free(codec_name);
+
+if (convert_failed)
+return FALSE;
+if (!video_encoder_procs[encoder])
+/* A valid but unsupported video encoder, skip it */
+continue;
+
+new_codecs[count].create = video_encoder_procs[encoder];
+new_codecs[count].type = type;
+new_codecs[count].cap = cap;
+count++;
}
-renderers[num_renderers++] = inf-id;
+num_video_codecs = count;
+memcpy(video_codecs, new_codecs, sizeof(video_codecs));
return TRUE;
}
@@ -793,6 +906,22 @@ void red_dispatcher_on_sv_change(void)
}
}
+void red_dispatcher_on_vc_change(void)
+{
+RedWorkerMessageSetVideoCodecs payload;
+int compression_level = calc_compression_level();
+RedDispatcher *now = dispatchers;
+while (now) {
+now-qxl-st-qif-set_compression_level(now-qxl, compression_level);
+payload.num_video_codecs = num_video_codecs;
+payload.video_codecs = video_codecs;
+dispatcher_send_message(now-dispatcher,
+RED_WORKER_MESSAGE_SET_VIDEO_CODECS,
+payload);
+now = now-next;
+}
+}
+
void red_dispatcher_set_mouse_mode(uint32_t mode)
{
RedWorkerMessageSetMouseMode payload;
@@ -1094,6 +1223,10 @@ void red_dispatcher_init(QXLInstance *qxl)
init_data.pending = red_dispatcher-pending;
init_data.num_renderers = num_renderers;
memcpy(init_data.renderers, renderers, sizeof(init_data.renderers));
+if (num_video_codecs 0)
+red_dispatcher_set_video_codecs(auto);
+init_data.num_video_codecs = num_video_codecs;
+memcpy(init_data.video_codecs, video_codecs,
sizeof(init_data.video_codecs));
pthread_mutex_init(red_dispatcher-async_lock, NULL);
init_data.image_compression = image_compression;
diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
index 907b7c7..eb3aee4 100644
--- a/server/red_dispatcher.h
+++ b/server/red_dispatcher.h
@@ -22,6 +22,7 @@
struct RedChannelClient;
struct RedDispatcher;
+typedef struct RedVideoCodec RedVideoCodec;
typedef struct AsyncCommand AsyncCommand;
void red_dispatcher_init(QXLInstance *qxl);
@@ -29,11 +30,13 @@ void red_dispatcher_init(QXLInstance *qxl);
void red_dispatcher_set_mm_time(uint32_t);
void red_dispatcher_on_ic_change(void);
void red_dispatcher_on_sv_change(void);
+void red_dispatcher_on_vc_change(void);
void red_dispatcher_set_mouse_mode(uint32_t mode);
void red_dispatcher_on_vm_stop(void);
void red_dispatcher_on_vm_start(void);
int red_dispatcher_count(void);
int red_dispatcher_add_renderer(const char *name);
+int red_dispatcher_set_video_codecs(const char* codecs);
uint32_t red_dispatcher_qxl_ram_size(void);
int red_dispatcher_qxl_count(void);
void red_dispatcher_async_complete(struct RedDispatcher *, AsyncCommand *);
@@ -174,6 +177,11 @@ typedef struct RedWorkerMessageSetStreamingVideo {
uint32_t streaming_video;
} RedWorkerMessageSetStreamingVideo;
+typedef struct RedWorkerMessageSetVideoCodecs {
+uint32_t num_video_codecs;
+RedVideoCodec* video_codecs;
+} RedWorkerMessageSetVideoCodecs;
+
typedef struct RedWorkerMessageSetMouseMode {
uint32_t mode;
} RedWorkerMessageSetMouseMode;
diff --git a/server/red_worker.c b/server/red_worker.c
index 19e27c5..b8c2259 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1,6 +1,7 @@
/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
/*
Copyright (C) 2009 Red Hat, Inc.
+ Copyright (C) 2015 Francois Gouget
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
@@ -994,6 +995,8 @@ typedef struct RedWorker {
uint32_t mouse_mode;
uint32_t streaming_video;
+uint32_t num_video_codecs;
+RedVideoCodec video_codecs[RED_MAX_VIDEO_CODECS];
Stream streams_buf[NUM_STREAMS];
Stream *free_streams;
Ring streams;
@@ -3074,10 +3077,41 @@ static void
red_stream_update_client_playback_latency(void *opaque, uint32_t del
main_dispatcher_set_mm_time_latency(agent-dcc-common.base.client,
agent-dcc-streams_max_latency);
}
+static VideoEncoder* red_display_create_video_encoder
[Spice-devel] [PATCH protocol 6/7] Add support for the VP8 video codec and for advertising supported video codecs. (take 2)
Clients that support multiple codecs should advertise the
SPICE_DISPLAY_CAP_MULTI_CODEC capability and one SPICE_DISPLAY_CAP_CODEC_XXX
per supported codec.
---
As far as I can tell this patch was ok in the previous round so no
change this time around.
spice/enums.h| 1 +
spice/protocol.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/spice/enums.h b/spice/enums.h
index 18e2f74..14675f9 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -139,6 +139,7 @@ typedef enum SpicePathFlags {
typedef enum SpiceVideoCodecType {
SPICE_VIDEO_CODEC_TYPE_MJPEG = 1,
+SPICE_VIDEO_CODEC_TYPE_VP8,
SPICE_VIDEO_CODEC_TYPE_ENUM_END
} SpiceVideoCodecType;
diff --git a/spice/protocol.h b/spice/protocol.h
index bea376c..7f0b322 100644
--- a/spice/protocol.h
+++ b/spice/protocol.h
@@ -134,6 +134,9 @@ enum {
SPICE_DISPLAY_CAP_A8_SURFACE,
SPICE_DISPLAY_CAP_STREAM_REPORT,
SPICE_DISPLAY_CAP_LZ4_COMPRESSION,
+SPICE_DISPLAY_CAP_MULTI_CODEC,
+SPICE_DISPLAY_CAP_CODEC_MJPEG,
+SPICE_DISPLAY_CAP_CODEC_VP8,
};
enum {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 4/7] server: Add a GStreamer 0.10 MJPEG video encoder and use it by default. (take 2)
The GStreamer video encoder is quite primitive and mostly meant to introduce,
test and debug the basic infrastructure.
The main limitation is the lack of rate control: the bitrate is set at startup
and will not react to changes in the network conditions. Still it should work
fine on LANs.
---
Changes since take 1:
- Use autovideoconvert instead of ffmpegcolorspace.
- The width/height comments are now correct in the previous patch of
the series.
- Added a comment suggesting to avoid copying the compressed buffer as
a future improvement.
configure.ac | 18 ++
server/Makefile.am | 8 +
server/gstreamer_encoder.c | 440 +
server/red_worker.c| 11 +-
server/video_encoder.h | 6 +
5 files changed, 481 insertions(+), 2 deletions(-)
create mode 100644 server/gstreamer_encoder.c
diff --git a/configure.ac b/configure.ac
index 7a9fc4b..919bf88 100644
--- a/configure.ac
+++ b/configure.ac
@@ -91,6 +91,15 @@ if test x$enable_smartcard = xyes; then
AC_DEFINE([USE_SMARTCARD], [1], [Define if supporting smartcard proxying])
fi
+AC_ARG_ENABLE(gstreamer-0.10,
+[ --enable-gstreamer-0.10 Enable GStreamer 0.10 support],,
+[enable_gstreamer_0_10=yes])
+AS_IF([test x$enable_gstreamer_0_10 != xno], [enable_gstreamer_0_10=yes])
+AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$enable_gstreamer_0_10 != xno)
+if test x$enable_gstreamer_0_10 = xyes; then
+ AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer 0.10])
+fi
+
AC_ARG_ENABLE(automated_tests,
[ --enable-automated-tests Enable automated tests using spicy-screenshot
(part of spice--gtk)],,
[enable_automated_tests=no])
@@ -127,6 +136,13 @@ if test x$enable_smartcard = xyes; then
AS_VAR_APPEND([SPICE_REQUIRES], [ libcacard = 0.1.2])
fi
+if test x$enable_gstreamer_0_10 = xyes; then
+PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10])
+AC_SUBST(GSTREAMER_0_10_LIBS)
+AC_SUBST(GSTREAMER_0_10_CFLAGS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10])
+fi
+
PKG_CHECK_MODULES([GLIB2], [glib-2.0 = 2.22])
AS_VAR_APPEND([SPICE_REQUIRES], [ glib-2.0 = 2.22])
@@ -359,6 +375,8 @@ echo
Smartcard:${enable_smartcard}
+GStreamer-0.10: ${enable_gstreamer_0_10}
+
SASL support: ${enable_sasl}
Automated tests: ${enable_automated_tests}
diff --git a/server/Makefile.am b/server/Makefile.am
index 36b6d45..28c4a46 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -11,6 +11,7 @@ AM_CPPFLAGS = \
$(SASL_CFLAGS) \
$(SLIRP_CFLAGS) \
$(SMARTCARD_CFLAGS) \
+ $(GSTREAMER_0_10_CFLAGS)\
$(SSL_CFLAGS) \
$(VISIBILITY_HIDDEN_CFLAGS) \
$(WARN_CFLAGS) \
@@ -41,6 +42,7 @@ libspice_server_la_LIBADD =
\
$(PIXMAN_LIBS) \
$(SASL_LIBS)\
$(SLIRP_LIBS) \
+ $(GSTREAMER_0_10_LIBS)
\
$(SSL_LIBS) \
$(Z_LIBS) \
$(SPICE_NONPKGCONFIG_LIBS) \
@@ -138,6 +140,12 @@ libspice_server_la_SOURCES += \
$(NULL)
endif
+if SUPPORT_GSTREAMER_0_10
+libspice_server_la_SOURCES += \
+ gstreamer_encoder.c \
+ $(NULL)
+endif
+
EXTRA_DIST = \
glz_encode_match_tmpl.c \
glz_encode_tmpl.c \
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
new file mode 100644
index 000..cd80cc9
--- /dev/null
+++ b/server/gstreamer_encoder.c
@@ -0,0 +1,440 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ Copyright (C) 2015 Jeremy White
+ Copyright (C) 2015 Francois Gouget
+
+ 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 A
HREF=http://www.gnu.org/licenses/;http
Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support
On Thu, 21 May 2015, Fabio Fantoni wrote: [...] Thanks for improving spice, I tried your patches, I applied the spice-gtk, spice-protocol and I updated the git submodule to use right spice-protocol but make of spice-common/common generate a new enum.h and override the spice-protocol patch failing the spice-gtk build with vp8 codec undefined. Seems that also a spice-common patch (at least in spice.proto) is needed but missed in the patch serie posted. Thanks for pointing this out. I could not see it initially because the top-level autogen.sh keeps rechecking out spice-protocol so spice-protocol/spice/enums.h ended up always being newer than spice.proto. I have added a spice-common patch in the new series. -- Francois Gouget fgou...@free.fr http://fgouget.free.fr/ A particle is an irreducible representation of the Poincaré Group - Eugene Wigner___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice] server: Remove an unused structure.
---
This struct was introduced in the first commit, c1b79eb0, about 6 years
ago, but has never been used. So I think it's safe to remove.
server/red_dispatcher.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index e8a1347..9d02d20 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -69,16 +69,6 @@ struct RedDispatcher {
QXLDevSurfaceCreate surface_create;
};
-typedef struct RedWorkeState {
-uint8_t *io_base;
-unsigned long phys_delta;
-
-uint32_t x_res;
-uint32_t y_res;
-uint32_t bits;
-uint32_t stride;
-} RedWorkeState;
-
extern uint32_t streaming_video;
extern spice_image_compression_t image_compression;
extern spice_wan_compression_t jpeg_state;
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support
On Tue, 26 May 2015, Fabio Fantoni wrote:
[...]
After din't show gstreamer warning anymore but still have image freeze
and also spice-gtk crash after open video fullscreen, here the full gdb
datas:
http://pastebin.com/idTkZLh0
It looks like the crash happened when trying to access the st-out_frame
buffer that was set up by stream_gst_data() in the client.
I don't know why that would be the case: as far as I can tell that
pointer is either valid or NULL; even if we return early from
push_frame() or pull_frame().
It may not be related to the issue you've run into but the patch below
should help quite a bit if you try running the client more than a minute
or two:
diff --git a/gtk/channel-display-gst.c b/gtk/channel-display-gst.c
index b880ce4..9da078a 100644
--- a/gtk/channel-display-gst.c
+++ b/gtk/channel-display-gst.c
@@ -221,6 +221,7 @@ static void pull_frame(display_stream *st)
// TODO seems like poor memory management
if (gst_memory_map(memory, mem_info, GST_MAP_READ)) {
+g_free(st-out_frame);
st-out_frame = g_malloc0(mem_info.size);
memcpy(st-out_frame, mem_info.data, mem_info.size);
I also have a patch that avoids copying the out_frame buffer but given
the low CPU usage of the client that should not be an issue.
After I tried with gstreamer using ffmpeg, vp8 doesn't crashed, probably
was problem of gstreamer0.10-plugins-bad but image freeze problem remain
When does the freeze happen? A short freeze is normal during the
transition from the regular transport to the video streaming but that
happens in the mjpeg case too. Is the freeze temporary?
[...]
About vp8 image freeze here some seconds of gst log debug on spice-gtk
when problem happen: http://pastebin.com/PP2R43Yf
Nothing jumped at me in this log.
Using spice:vp8 seems only have low performance.
In my experience the VP8 encoder saturates a core which is why it is not
smooth. When running a test pipeline through gst-laucnh I'm able to
solve that by playing with vp8enc's speed and threads parameters but for
some reason these have no effect in Spice.
gst-launch videotestsrc ! video/x-raw-rgb,width=1024,height=768 ! \
ffmpegcolorspace ! vp8enc speed=2 threads=4 ! \
vp8dec ! ffmpegcolorspace ! fpsdisplaysink
--
Francois Gouget fgou...@codeweavers.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 0/11] Add GStreamer and VP8 support
On Tue, 2 Jun 2015, Fabio Fantoni wrote: [...] Video on youtube, Does it look like the video moves back and forth? Or maybe just the progress bar area, with some shearing too. That's a known issue caused by the progress bar popping up and down. hd trailer (file) with vlc (without vp8 works correctly) and win media player. I have not been able to really test with VLC here. I use Xspice which apparently does not support Xshm and that causes VLC to crash. And since I'm not testing with a VM windows media player is out. Is bandwidth limited between the server and the client? With VP8, if an I frame gets dropped then the following P frames will be broken. I'm not sure how it handles that. Does adding error-resilient=1 or error-resilient=2 to vp8enc make a difference? -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice] HAVE_CLOCK_GETTIME is not used so remove it.
Signed-off-by: Francois Gouget fgou...@codeweavers.com --- Maybe we should actually use it but currently clock_gettime() uses are all over the place and this macro was only used in the client code anyway (see 18769714 and 3f3283ee). configure.ac | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 7a9fc4b..62c714e 100644 --- a/configure.ac +++ b/configure.ac @@ -108,10 +108,7 @@ AC_SUBST(COMMON_CFLAGS) AC_CHECK_LIBM AC_SUBST(LIBM) -AC_CHECK_LIB(rt, clock_gettime, - AC_DEFINE([HAVE_CLOCK_GETTIME], 1, [Defined if we have clock_gettime()]) - LIBRT=-lrt - ) +AC_CHECK_LIB(rt, clock_gettime, LIBRT=-lrt) AC_SUBST(LIBRT) AS_VAR_APPEND([SPICE_NONPKGCONFIG_LIBS], [ -pthread $LIBM $LIBRT]) -- 2.1.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 1/2] server: Refresh the input fps every 5 second, without a timer.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This implements Marc-André Lureau's suggestion:
http://lists.freedesktop.org/archives/spice-devel/2015-June/020202.html
And supersedes the input-fps rounding patch.
http://lists.freedesktop.org/archives/spice-devel/2015-June/020176.html
server/red_worker.c | 45 +++--
1 file changed, 15 insertions(+), 30 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 5deb30b..a07a78a 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -120,7 +120,7 @@
#define RED_STREAM_GRADUAL_FRAMES_START_CONDITION 0.2
#define RED_STREAM_FRAMES_RESET_CONDITION 100
#define RED_STREAM_MIN_SIZE (96 * 96)
-#define RED_STREAM_INPUT_FPS_TIMEOUT (5 * 1000) // 5 sec
+#define RED_STREAM_INPUT_FPS_TIMEOUT ((uint64_t)5 * 1000 * 1000 * 1000) // 5
sec
#define RED_STREAM_CHANNEL_CAPACITY 0.8
/* the client's stream report frequency is the minimum of the 2 values below */
#define RED_STREAM_CLIENT_REPORT_WINDOW 5 // #frames
@@ -450,9 +450,8 @@ struct Stream {
Stream *next;
RingItem link;
-SpiceTimer *input_fps_timer;
uint32_t num_input_frames;
-uint64_t input_fps_timer_start;
+uint64_t input_fps_start_time;
uint32_t input_fps;
};
@@ -2532,9 +2531,6 @@ static int is_same_drawable(RedWorker *worker, Drawable
*d1, Drawable *d2)
static inline void red_free_stream(RedWorker *worker, Stream *stream)
{
-if (stream-input_fps_timer) {
-spice_timer_remove(stream-input_fps_timer);
-}
stream-next = worker-free_streams;
worker-free_streams = stream;
}
@@ -2613,7 +2609,17 @@ static void red_attach_stream(RedWorker *worker,
Drawable *drawable, Stream *str
stream-current = drawable;
drawable-stream = stream;
stream-last_time = drawable-creation_time;
-stream-num_input_frames++;
+
+uint64_t duration = drawable-creation_time - stream-input_fps_start_time;
+if (duration = RED_STREAM_INPUT_FPS_TIMEOUT) {
+/* Round to the nearest integer, for instance 24 for 23.976 */
+stream-input_fps = ((uint64_t)stream-num_input_frames * 1000 * 1000
* 1000 + duration / 2) / duration;
+spice_debug(input-fps=%u, stream-input_fps);
+stream-num_input_frames = 0;
+stream-input_fps_start_time = drawable-creation_time;
+} else {
+stream-num_input_frames++;
+}
WORKER_FOREACH_DCC_SAFE(worker, item, next, dcc) {
StreamAgent *agent;
@@ -3123,24 +3129,6 @@ static void
red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
#endif
}
-static void red_stream_input_fps_timer_cb(void *opaque)
-{
-Stream *stream = opaque;
-uint64_t now = red_now();
-double duration_sec;
-
-spice_assert(opaque);
-if (now == stream-input_fps_timer_start) {
-spice_warning(timer start and expiry time are equal);
-return;
-}
-duration_sec = (now - stream-input_fps_timer_start)/(1000.0*1000*1000);
-stream-input_fps = stream-num_input_frames / duration_sec;
-spice_debug(input-fps=%u, stream-input_fps);
-stream-num_input_frames = 0;
-stream-input_fps_timer_start = now;
-}
-
static void red_create_stream(RedWorker *worker, Drawable *drawable)
{
DisplayChannelClient *dcc;
@@ -3167,12 +3155,9 @@ static void red_create_stream(RedWorker *worker,
Drawable *drawable)
SpiceBitmap *bitmap = drawable-red_drawable-u.copy.src_bitmap-u.bitmap;
stream-top_down = !!(bitmap-flags SPICE_BITMAP_FLAGS_TOP_DOWN);
drawable-stream = stream;
-stream-input_fps_timer =
spice_timer_queue_add(red_stream_input_fps_timer_cb, stream);
-spice_assert(stream-input_fps_timer);
-spice_timer_set(stream-input_fps_timer, RED_STREAM_INPUT_FPS_TIMEOUT);
-stream-num_input_frames = 0;
-stream-input_fps_timer_start = red_now();
stream-input_fps = MAX_FPS;
+stream-num_input_frames = 0;
+stream-input_fps_start_time = drawable-creation_time;
worker-streams_size_total += stream-width * stream-height;
worker-stream_count++;
WORKER_FOREACH_DCC_SAFE(worker, dcc_ring_item, next, dcc) {
--
2.1.4___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 2/2] server: Provide a framerate estimate based on the frames that lead to the stream creation.
This way the video encoder can actually count on a real estimate when it is
initializing.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Initialisation time is when video encoders need an fps most since they
have no other information at that point. Yet, all they were getting is
MAX_FPS (30).
This patch takes advantage of the fact that Spice needs to keep track of
the related drawables in order to determine whether creating a video
stream makes sense. So it stores the time of the first drawable and then
propagates it. So when the time comes to create the stream we can
provide a framerate estimate based on the first 20 frames. In my tests
the result falls within one 1 fps of the later estimates and thus is
much more useful than MAX_FPS.
server/red_worker.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index a07a78a..8374f54 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -844,6 +844,7 @@ struct Drawable {
Ring glz_ring;
red_time_t creation_time;
+red_time_t first_frame_time;
int frames_count;
int gradual_frames_count;
int last_gradual_frame;
@@ -915,6 +916,7 @@ typedef struct RedSurface {
typedef struct ItemTrace {
red_time_t time;
+red_time_t first_frame_time;
int frames_count;
int gradual_frames_count;
int last_gradual_frame;
@@ -1926,6 +1928,7 @@ static inline void red_add_item_trace(RedWorker *worker,
Drawable *item)
trace = worker-items_trace[worker-next_item_trace++ ITEMS_TRACE_MASK];
trace-time = item-creation_time;
+trace-first_frame_time = item-first_frame_time;
trace-frames_count = item-frames_count;
trace-gradual_frames_count = item-gradual_frames_count;
trace-last_gradual_frame = item-last_gradual_frame;
@@ -3155,7 +3158,12 @@ static void red_create_stream(RedWorker *worker,
Drawable *drawable)
SpiceBitmap *bitmap = drawable-red_drawable-u.copy.src_bitmap-u.bitmap;
stream-top_down = !!(bitmap-flags SPICE_BITMAP_FLAGS_TOP_DOWN);
drawable-stream = stream;
-stream-input_fps = MAX_FPS;
+/* Provide an fps estimate the video encoder can use when initializing
+ * based on the frames that lead to the creation of the stream. Round to
+ * the nearest integer, for instance 24 for 23.976.
+ */
+uint64_t duration = drawable-creation_time - drawable-first_frame_time;
+stream-input_fps = ((uint64_t)drawable-frames_count * 1000 * 1000 * 1000
+ duration / 2) / duration;
stream-num_input_frames = 0;
stream-input_fps_start_time = drawable-creation_time;
worker-streams_size_total += stream-width * stream-height;
@@ -3163,9 +3171,9 @@ static void red_create_stream(RedWorker *worker, Drawable
*drawable)
WORKER_FOREACH_DCC_SAFE(worker, dcc_ring_item, next, dcc) {
red_display_create_stream(dcc, stream);
}
-spice_debug(stream %d %dx%d (%d, %d) (%d, %d), (int)(stream -
worker-streams_buf), stream-width,
+spice_debug(stream %d %dx%d (%d, %d) (%d, %d) %u fps, (int)(stream -
worker-streams_buf), stream-width,
stream-height, stream-dest_area.left, stream-dest_area.top,
-stream-dest_area.right, stream-dest_area.bottom);
+stream-dest_area.right, stream-dest_area.bottom,
stream-input_fps);
return;
}
@@ -3419,11 +3427,13 @@ static inline int red_is_stream_start(Drawable
*drawable)
// returns whether a stream was created
static int red_stream_add_frame(RedWorker *worker,
Drawable *frame_drawable,
+red_time_t first_frame_time,
int frames_count,
int gradual_frames_count,
int last_gradual_frame)
{
red_update_copy_graduality(worker, frame_drawable);
+frame_drawable-first_frame_time = first_frame_time;
frame_drawable-frames_count = frames_count + 1;
frame_drawable-gradual_frames_count = gradual_frames_count;
@@ -3477,6 +3487,7 @@ static inline void red_stream_maintenance(RedWorker
*worker, Drawable *candidate
} else {
if (red_is_next_stream_frame(worker, candidate, prev) !=
STREAM_FRAME_NONE) {
red_stream_add_frame(worker, candidate,
+ prev-first_frame_time,
prev-frames_count,
prev-gradual_frames_count,
prev-last_gradual_frame);
@@ -3638,6 +3649,7 @@ static inline void red_use_stream_trace(RedWorker
*worker, Drawable *drawable)
trace-dest_area, trace-time, NULL,
FALSE) !=
STREAM_FRAME_NONE) {
if (red_stream_add_frame(worker, drawable,
+ trace-first_frame_time
Re: [Spice-devel] [PATCH spice 6/11] server: Add VP8 support, a video codec preference list and compatibility checks with the Spice client.
On Tue, 19 May 2015, Marc-André Lureau wrote:
[...]
+/* Set the encoder initial bit rate, and ask for a low latency */
+adjust_bit_rate(encoder);
+g_object_set(G_OBJECT(encoder-gstenc), bitrate, encoder-bit_rate,
NULL);
+g_object_set(G_OBJECT(encoder-gstenc), max-latency, 0, NULL);
+g_object_set(G_OBJECT(encoder-gstenc), max-keyframe-distance, 0,
NULL);
+g_object_set(G_OBJECT(encoder-gstenc), lag-in-frames, 0, NULL);
+
Those paremeters do not exist with all encoders, notably ffenc_mjpeg.
I'll tweak it so we only set the properties supported by the current
encoder.
We could rather use or define encodebin profiles.
See discussion in the previous email.
[...]
+if (encoder-base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8) {
+/* vp8enc gets confused if we try to reconfigure the pipeline */
reset_pipeline(encoder);
You'll easily reach this condition with CAP_SIZED_STREAM streams.
It's ok. reset_pipeline() frees the current pipeline, causing
gst_encoder_encode_frame() to call construct_pipeline() as if this was
the stream's first frame.
[...]
/* The GStreamer buffer timestamps and framerate are irrelevant and
would
* be hard to set right because they can arrive a bit irregularly
*/
-GST_BUFFER_TIMESTAMP(buffer) = GST_CLOCK_TIME_NONE;
-GST_BUFFER_DURATION(buffer) = GST_CLOCK_TIME_NONE;
+if (encoder-base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8)
+{
+/* FIXME: Maybe try drop-frame = 0 instead? */
+GST_BUFFER_TIMESTAMP(buffer) = frame_time;
+GST_BUFFER_DURATION(buffer) = 1;
+}
+else
+{
+GST_BUFFER_TIMESTAMP(buffer) = GST_CLOCK_TIME_NONE;
+GST_BUFFER_DURATION(buffer) = GST_CLOCK_TIME_NONE;
+}
What is this supposed to change? Did you consider appsrc do-timestamp
instead?
It's a hack to get both the mjpeg end vp8 encoders working.
Using 'appsrc do-timestamp=1' works for the mjpeg encoder but causes us
to block indefinitely in gst_app_sink_pull_buffer() for the vp8 encoder
(independently of is-live), for an unknown reason.
My suspicion is that the frame gets dropped, presumably by vp8enc
itself, maybe due to pipeline latency issues.
I'm also not sure yet of what the best approach is for timestamping and
setting the framerate in the appsrc caps and their impact on the
bitrate.
One option is to advertise the outgoing framerate, the target bitrate
and use do-timestamping. The outgoing framerate, e.g. 10fps, may be
lower than the source framerate, e.g. 24 fps, which implies dropping
frames before passing them off to the pipeline. This means some frames
will be less than 1/10th second apart and, if I remember correctly,
this causes the pipeline to drop them which is not what we want (and
again causes a freeze in gst_app_sink_pull_buffer()).
Another option is to set the bitrate and framerate to get the desired
compressed buffer size. Then the stream's bitrate can be further reduced
by controlling the outgoing frame rate. In this case I'm not sure using
real timestamps makes sense.
@@ -364,13 +382,13 @@ static int gst_encoder_encode_frame(GstEncoder
*encoder,
encoder-spice_format = bitmap-format;
encoder-width = width;
encoder-height = height;
-if (encoder-pipeline !reconfigure_pipeline(encoder))
-return VIDEO_ENCODER_FRAME_UNSUPPORTED;
+if (encoder-pipeline)
+reconfigure_pipeline(encoder);
Why this change?
reconfigure_pipeline() used to return false if it failed to reconfigure
the pipeline. With the old code this would cause us to return
FRAME_UNSUPPORTED. But in fact in such a case the existing pipeline has
been freed and all we need to do is to rebuild it from scratch which is
what we already do a few lines down.
This should maybe have been part of patch 5 though
reconfigure_pipeline() does not fail with ffenc_mjpeg.
+agent-video_encoder = red_display_create_video_encoder(dcc,
initial_bit_rate, video_cbs, agent);
} else {
-agent-video_encoder = create_video_encoder(0, NULL, NULL);
+agent-video_encoder = red_display_create_video_encoder(dcc, 0,
NULL, NULL);
}
+/* FIXME: We may have failed to create a video encoder which will cause
+ *a crash!
+ */
You may disable video streaming in this case? (worker-streaming_video ==
STREAM_VIDEO_OFF)
The administrator could certainly set this option.
But as for gracefully handling this failure without configuration, as
far as I can tell, by the time red_display_create_stream() fails it's
too late to set streaming_video.
--
Francois Gouget fgou...@codeweavers.com___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice 3/11] server: Refactor the Spice server video encoding so alternative implementations can be added.
On Tue, 19 May 2015, Marc-André Lureau wrote:
[...]
+struct VideoEncoder {
+/* Releases the video encoder's resources */
+void (*destroy)(VIDEO_ENCODER_T *encoder);
+
+/* Compresses the specified src image area into the outbuf buffer.
+ *
+ * @encoder: The video encoder.
+ * @bitmap:The Spice screen.
+ * @width: The width of the Spice screen.
+ * @height:The heigth of the Spice screen.
It's not the screen, it's the video area. It's not clear to me what
would happen if the src size doesn't match the widthheight.
Ah right. I'll fix the comment. I was confused about that but after
looking again I must say I'm still unsure as to why we get both src and
width+height.
My understanding is that this has been introduced to deal with the
YouTube progress bar:
https://bugzilla.redhat.com/show_bug.cgi?id=813826
To sum up, the size of the video area detected by the Spice server
changes when the progress bar pops up or down. As a result some of the
'video frames' it gets don't match the size it detected when it created
the video stream.
* It used to be that this caused such frames to go through the regular
update mechanism instead of through the video stream. This is still
the case with the spicec client for instance and results in the video
seeming to jump back and forth because the video stream is delayed
due to buffering while the regular path is not.
Looking at the impact on src height, either the size of the video
frame, aka src, matches the stream size and then so do width and
height (lines 9 10 below), or it does not and then we don't go
through the video encoder (line 6 below).
* Now clients can advertise support for changing stream sizes with the
SIZED_STREAM capability. This does not totally fix the back and forth
issue but restricts it to the progress bar area as it comes in and
out of the video frame.
In that case width height are either computed from the actual video
frame size, aka src, (lines 3 4 below), or default to the initial
stream size when the video frame size matches (lines 9 10 below).
From red_marshall_stream_data():
1 if (drawable-sized_stream) {
2 if (red_channel_client_test_remote_cap(rcc,
SPICE_DISPLAY_CAP_SIZED_STREAM)) {
3 width = src_rect-right - src_rect-left;
4 height = src_rect-bottom - src_rect-top;
5 } else {
6 return FALSE;
7 }
8 } else {
9 width = stream-width;
10 height = stream-height;
11 }
So in the end width and height *always* match src. So they're redundant,
unless some larger purpose is planned for them.
Now must say having a continuously changing stream size is quite
annoying in the context of GStreamer:
- At a minimum it forces a reconfiguration of the pipeline which,
depending on the encoder, may require rebuilding it. Either cases
seem to cause a reset of the GStreamer's encoder bitrate control
algorithm which tends to cause it to overshoot and may cause frame
drops.
- Ultimately the GStreamer video encoder should support inter-frame
compression (with VP8 or H264 for instance) and a changing stream
size will also cause some disruption in this context.
Things can still work as is so there's not hurry but maybe a better
solution can be devised in time.
For instance it may be better to increase the stream size as the video
frame size increases but then peg it so it does not shrink again. This
would solve the remaining back and forth issue with the YouTube progress
bar, and limit the number of resizes the video encoders have to go
through. It supposes that regular frame updates know to ignore changes
happening inside the stream area even if they don't go through the
regular 'video frame' update path but I have no idea if that's easy to
do.
Alternatively one could split oversize video frames in two: the stream
area which would go through the video encoder, and the remainder which
would go through the regular path. This would also solve the remaining
back and forth issue with the YouTube progress bar and would even be
compatible with non SIZED_STREAM capable clients.
--
Francois Gouget fgou...@codeweavers.com
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice 4/11] server: Add a GStreamer 0.10 MJPEG video encoder and use it by default.
On Tue, 19 May 2015, Marc-André Lureau wrote: [...] I think this should target at least gstreamer 1.0. Do you have good reasons to want 0.10 support? I need to target CentOS 6.5 which does not have GStreamer 1.0. I have started work on adding GStreamer 1.0 support anyway but that's not working yet. +encoder-pipeline = gst_parse_launch_full(appsrc name=src is-live=1 ! ffmpegcolorspace ! ffenc_mjpeg name=encoder ! appsink name=sink, NULL, GST_PARSE_FLAG_FATAL_ERRORS, err); Any reason to pick ffmpegcolorspace instead of autoconvert ? Using autovideoconvert works, I'll use that instead. I would rather see an encodebin with a mjpeg profile or caps. However, I am not sure it will be possible to adjust bitrate easily. That could be done later. There's also another issue which is that VP8 encoder does inter-frame compression. Currently that does not mesh well with neither Spice's video_encoder_encode_frame() API which expects to get one compressed buffer for each input frame, nor the current gstreamer_encoder implementation. So to get VP8 support we configure the encoder to force it to only issue keyframes. But I'm not sure that would be possible through encodebin profiles, particularly in an encoder-independent way (even if just across VP8 encoder implementations). +/* We could create a bunch of GstMemory objects, one per line, to avoid + * copying the raw frame. But this may run into alignment problems and it's + * unclear that it would be any faster, particularly if we're unable to + * cache these objects. + */ It would probably help when encoding fullscreen. Maybe. It's a GStreamer 1.0 feature so I have not had a deep look into it. [...] Although it is much less important than the memcpy on the src, I could imagine the memcpy could be removed eventually, perhaps a FIXME is worth here. It probably requires setting up our own buffer allocator so when the encoder asks for a buffer we can give it the Spice output buffer. That may be making too many assumptions about the encoder though, particularly in GStreamer 1.0. I'll add a FIXME though. -- Francois Gouget fgou...@codeweavers.com___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice 02/13] server: Add a GStreamer 0.10 MJPEG video encoder and use it by default. (take 4)
The GStreamer video encoder supports both regular and sized streams.
It is otherwise quite basic and lacks any rate control: the bitrate is
set at startup and will
not react to changes in the network conditions. Still it should work
fine on LANs.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
On Fri, 31 Jul 2015, Victor Toso wrote:
[...]
+static gboolean set_appsrc_caps(GstEncoder *encoder)
+{
+GstCaps *new_caps = gst_caps_new_simple(video/x-raw-rgb,
+bpp, G_TYPE_INT, encoder-format-bpp,
+depth, G_TYPE_INT, encoder-format-depth,
+width, G_TYPE_INT, encoder-width,
+height, G_TYPE_INT, encoder-height,
+endianness, G_TYPE_INT, encoder-format-endianness,
+red_mask, G_TYPE_INT, encoder-format-red_mask,
+green_mask, G_TYPE_INT, encoder-format-green_mask,
+blue_mask, G_TYPE_INT, encoder-format-blue_mask,
+framerate, GST_TYPE_FRACTION, get_source_fps(encoder), 1,
+NULL);
+if (!new_caps) {
+spice_warning(GStreamer error: could not create the source caps);
+reset_pipeline(encoder);
+return FALSE;
This function is called twice and one of them do reset_pipeline in case
of failure. I would remove reset_pipeline here to be dealt by who called
it.
(I'm not sure if it is possible to gst_caps_new_simple return NULL)
I looked at the GStreamer code and gst_caps_new_simple() is not supposed
to fail. So I tweaked set_appsrc_caps() accordingly.
+/* A helper for gst_encoder_encode_frame(). */
+static gboolean construct_pipeline(GstEncoder *encoder, const SpiceBitmap
*bitmap)
+{
+GError *err = NULL;
+const gchar* desc = appsrc name=src format=2 do-timestamp=true !
ffmpegcolorspace ! ffenc_mjpeg name=encoder ! appsink name=sink;
Either keep * with the type or with variable.
Eh. I tend to prefer keeping it with the type but that's not the Spice
style. I slipped. That's fixed.
+spice_debug(GStreamer pipeline: %s, desc);
+encoder-pipeline = gst_parse_launch_full(desc, NULL,
GST_PARSE_FLAG_FATAL_ERRORS, err);
+if (!encoder-pipeline) {
I guess we want to check the err != NULL as well as
gst_parse_launch_full may return non NULL pipeline even if error is set.
/me believes that if the pipeline had an error to be created we should
not try to play it.
Makes sense. Done.
+/* Start playing */
+spice_debug(removing the pipeline clock);
+gst_pipeline_use_clock(GST_PIPELINE(encoder-pipeline), NULL);
This is related to ffenc_mjpeg described in the mail, right?
I think it would be good to open a bug against 0.10 version and add a
FIXME here to track the issue.
https://bugzilla.gnome.org/enter_bug.cgi?product=GStreamer
I have reported the issue in bug 753257 which is probably identical to
bug 724103:
https://bugzilla.gnome.org/show_bug.cgi?id=753257
https://bugzilla.gnome.org/show_bug.cgi?id=724103
I also added a reference to the bug in the source. Clearly this will not
be fixed for GStreamer 0.10 but maybe there's still hope for GStreamer
1.0.
+/* A helper for gst_encoder_encode_frame(). */
+static int push_raw_frame(GstEncoder *encoder, const SpiceBitmap *bitmap,
+ const SpiceRect *src, int top_down)
+{
[...]
I think we definitely need more help functions here to set the GstBuffer.
Later patches introduce gst 1.0 and the zero-copy and this becomes a bit
hard to track.
The different part are quite strongly coupled. I still managed to split
off the line-by-line copy and zero copy code. Hopefully it makes things
clearer.
+/* A helper for gst_encoder_encode_frame(). */
+static int pull_compressed_buffer(GstEncoder *encoder,
+ uint8_t **outbuf, size_t *outbuf_size,
+ int *data_size)
+{
This will be totally changed when video encoder manage the compressed
buffer.
Yes. Letting the video encoder manage the compressed buffer is a
relatively big change so I prefer to send it separately.
+spice_debug(video format change: width %d - %d, height %d - %d,
format %d - %d, encoder-width, width, encoder-height, height,
encoder-spice_format, bitmap-format);
Please, break this 180 chars line.
Done.
+if (!encoder-pipeline !construct_pipeline(encoder, bitmap)) {
+return VIDEO_ENCODER_FRAME_UNSUPPORTED;
If we reach this we may need a spice_warning, no?
Actually the warning is issued by construct_pipeline(), which is where
we know most about what went wrong. So I don't think we need an
additional warning there.
+static void gst_encoder_notify_server_frame_drop(GstEncoder *encoder)
+{
+spice_debug(server report: getting frame drops...);
This will be improved in following patches
Yes. This is mostly a placeholder at this stage.
diff --git a/configure.ac b/configure.ac
index 5b4caa4..ca91b5a 100644
--- a/configure.ac
[Spice-devel] [PATCH v2 7/7] spice-gtk: Use typefind and decodebin as fallbacks to set up the GStreamer pipeline.
Potentially this means future video codecs can be supported automatically.
One can also force usage of typefind and decodebin by setting the
SPICE_GST_AUTO environment variable. If set to 'decodebin' then only decodebin
will be used.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
src/channel-display-gst.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 59ce3e1..3d9274b 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -110,27 +110,35 @@ static gboolean construct_pipeline(GstDecoder *decoder)
decoder-pipeline_wait = 1;
decoder-samples_count = 0;
-const gchar *src_caps, *gstdec_name;
+const gchar *src_caps = NULL;
+const gchar *gstdec_name = NULL;
switch (decoder-base.codec_type) {
case SPICE_VIDEO_CODEC_TYPE_MJPEG:
-src_caps = image/jpeg;
+src_caps = caps=image/jpeg;
gstdec_name = jpegdec;
break;
case SPICE_VIDEO_CODEC_TYPE_VP8:
-src_caps = video/x-vp8;
+src_caps = caps=video/x-vp8;
gstdec_name = vp8dec;
break;
case SPICE_VIDEO_CODEC_TYPE_H264:
-src_caps = video/x-h264;
+src_caps = caps=video/x-h264;
gstdec_name = h264parse ! avdec_h264;
break;
default:
-spice_warning(Unknown codec type %d, decoder-base.codec_type);
-return -1;
+SPICE_DEBUG(Unknown codec type %d, decoder-base.codec_type);
+break;
+}
+const gchar *gst_auto = getenv(SPICE_GST_AUTO);
+if (!src_caps || (gst_auto strcmp(gst_auto, decodebin))) {
+src_caps = typefind=true; /* Misidentifies VP8 */
+}
+if (!gstdec_name || gst_auto) {
+gstdec_name = decodebin; /* vaapi is assert-happy */
}
GError *err = NULL;
-gchar *desc = g_strdup_printf(appsrc name=src format=2 do-timestamp=1
caps=%s ! %s ! videoconvert ! appsink name=sink caps=video/x-raw,format=BGRx,
src_caps, gstdec_name);
+gchar *desc = g_strdup_printf(appsrc name=src format=2 do-timestamp=1 %s
! %s ! videoconvert ! appsink name=sink caps=video/x-raw,format=BGRx,
src_caps, gstdec_name);
SPICE_DEBUG(GStreamer pipeline: %s, desc);
decoder-pipeline = gst_parse_launch_full(desc, NULL,
GST_PARSE_FLAG_FATAL_ERRORS, err);
g_free(desc);
--
2.4.6
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 6/7] spice-gtk: Make it possible to disable support for the builtin MJPEG video decoder.
This makes it possible to test the GStreamer video decoder with MJPEG streams.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
configure.ac | 8
src/Makefile.am | 7 ++-
src/channel-display.c | 2 ++
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 003e351..1b1df3e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -355,6 +355,14 @@ AM_CONDITIONAL([HAVE_GSTVIDEO], [test x$have_gst_video =
xyes])]
AC_SUBST(GSTVIDEO_CFLAGS)
AC_SUBST(GSTVIDEO_LIBS)
+AC_ARG_ENABLE([builtin-mjpeg],
+ AS_HELP_STRING([--enable-builtin-mjpeg], [Enable the builtin mjpeg video
decoder @:@default=yes@:@]),
+ [],
+ enable_builtin_mjpeg=yes)
+AS_IF([test x$enable_builtin_mjpeg = xyes],
+ [AC_DEFINE([WITH_BUILTIN_MJPEG], 1, [Use the builtin mjpeg decoder?])])
+AM_CONDITIONAL(WITH_BUILTIN_MJPEG, [test x$enable_builtin_mjpeg != xno])
+
AC_CHECK_LIB(jpeg, jpeg_destroy_decompress,
AC_MSG_CHECKING([for jpeglib.h])
AC_TRY_CPP(
diff --git a/src/Makefile.am b/src/Makefile.am
index 925c75f..e744ce9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -255,7 +255,6 @@ libspice_client_glib_2_0_la_SOURCES =
\
channel-cursor.c\
channel-display.c \
channel-display-priv.h \
- channel-display-mjpeg.c \
channel-inputs.c\
channel-main.c \
channel-playback.c \
@@ -342,6 +341,12 @@ libspice_client_glib_2_0_la_SOURCES += \
$(NULL)
endif
+if WITH_BUILTIN_MJPEG
+libspice_client_glib_2_0_la_SOURCES += \
+ channel-display-mjpeg.c \
+ $(NULL)
+endif
+
if HAVE_GSTVIDEO
libspice_client_glib_2_0_la_SOURCES += \
channel-display-gst.c \
diff --git a/src/channel-display.c b/src/channel-display.c
index 7756d35..cb717d3 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1009,9 +1009,11 @@ static void display_handle_stream_create(SpiceChannel
*channel, SpiceMsgIn *in)
display_update_stream_region(st);
switch (op-codec_type) {
+#ifdef WITH_BUILTIN_MJPEG
case SPICE_VIDEO_CODEC_TYPE_MJPEG:
st-video_decoder = create_mjpeg_decoder(op-codec_type, st);
break;
+#endif
default:
st-video_decoder = create_gstreamer_decoder(op-codec_type, st);
}
--
2.4.6
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 4/7] spice-gtk: Add a GStreamer video decoder with support for MJPEG, VP8 and h264.
Based on a patch by Jeremy White. Signed-off-by: Francois Gouget fgou...@codeweavers.com --- Note that this patch depends on the spice-common patch so a spice-common commit is in order, either separately or as part of this commit. configure.ac | 47 +++--- src/Makefile.am| 12 ++- src/channel-display-gst.c | 216 + src/channel-display-priv.h | 1 + src/channel-display.c | 6 +- 5 files changed, 265 insertions(+), 17 deletions(-) create mode 100644 src/channel-display-gst.c diff --git a/configure.ac b/configure.ac index 1b347c0..003e351 100644 --- a/configure.ac +++ b/configure.ac @@ -299,7 +299,7 @@ AC_ARG_WITH([audio], case $with_audio in gstreamer|pulse|auto*) -PKG_CHECK_MODULES(GST, gstreamer-1.0 gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-audio-1.0, [have_gst=yes], [have_gst=no]) +PKG_CHECK_MODULES(GSTAUDIO, gstreamer-1.0 gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-audio-1.0, [have_gst_audio=yes], [have_gst_audio=no]) PKG_CHECK_MODULES(PULSE, libpulse libpulse-mainloop-glib, [have_pulse=yes], [have_pulse=no]) ;; no*) @@ -310,7 +310,7 @@ esac AS_IF([test x$with_audio = xauto test x$have_pulse = xyes], [with_audio=pulse]) -AS_IF([test x$with_audio = xauto test x$have_gst = xyes], +AS_IF([test x$with_audio = xauto test x$have_gst_audio = xyes], [with_audio=gstreamer]) AS_IF([test x$with_audio = xauto], @@ -319,22 +319,41 @@ AS_IF([test x$with_audio = xauto], AS_IF([test x$with_audio = xpulse], [AS_IF([test x$have_pulse = xyes], [AC_DEFINE([WITH_PULSE], 1, [Have pulseaudio?])], - [AC_MSG_ERROR([PulseAudio requested but not found]) - ]) -]) + [AC_MSG_ERROR([PulseAudio requested but not found])] + )] +) AM_CONDITIONAL([WITH_PULSE], [test x$have_pulse = xyes]) AC_SUBST(PULSE_CFLAGS) AC_SUBST(PULSE_LIBS) AS_IF([test x$with_audio = xgstreamer], - [AS_IF([test x$have_gst = xyes], - [AC_DEFINE([WITH_GSTAUDIO], 1, [Have GStreamer 1.0?])], - [AC_MSG_ERROR([GStreamer 1.0 requested but not found]) - ]) -]) -AM_CONDITIONAL([WITH_GSTAUDIO], [test x$have_gst = xyes]) -AC_SUBST(GST_CFLAGS) -AC_SUBST(GST_LIBS) + [AS_IF([test x$have_gst_audio = xyes], + [AC_DEFINE([WITH_GSTAUDIO], 1, [Have GStreamer 1.0 audio?])], + [AC_MSG_ERROR([GStreamer 1.0 audio requested but not found])] + )] +) +AM_CONDITIONAL([WITH_GSTAUDIO], [test x$have_gst_audio = xyes]) +AC_SUBST(GSTAUDIO_CFLAGS) +AC_SUBST(GSTAUDIO_LIBS) + +AC_ARG_ENABLE([gst-video], + AS_HELP_STRING([--enable-gst-video=@:@auto/yes/no@:@], + [Enable GStreamer video support @:@default=auto@:@]), + [], + [enable_gst_video=auto]) + +AS_IF([test x$enable_gst_video != xno], + [PKG_CHECK_MODULES(GSTVIDEO, gstreamer-1.0 gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-video-1.0, [have_gst_video=yes], [have_gst_video=no]) + AS_IF([test x$have_gst_video = xyes], + [AC_DEFINE([HAVE_GSTVIDEO], 1, [Have GStreamer 1.0 video?])], + [AS_IF([test x$enable_gst_video = xyes], +[AC_MSG_ERROR([GStreamer 1.0 video requested but not found])] + )] + )] +) +AM_CONDITIONAL([HAVE_GSTVIDEO], [test x$have_gst_video = xyes])] +AC_SUBST(GSTVIDEO_CFLAGS) +AC_SUBST(GSTVIDEO_LIBS) AC_CHECK_LIB(jpeg, jpeg_destroy_decompress, AC_MSG_CHECKING([for jpeglib.h]) @@ -700,7 +719,7 @@ SPICE_CFLAGS=$SPICE_CFLAGS $WARN_CFLAGS AC_SUBST(SPICE_CFLAGS) -SPICE_GLIB_CFLAGS=$PROTOCOL_CFLAGS $PIXMAN_CFLAGS $PULSE_CFLAGS $GST_CFLAGS $GLIB2_CFLAGS $GIO_CFLAGS $GOBJECT2_CFLAGS $SSL_CFLAGS $SASL_CFLAGS +SPICE_GLIB_CFLAGS=$PROTOCOL_CFLAGS $PIXMAN_CFLAGS $PULSE_CFLAGS $GSTVIDEO_CFLAGS $GSTAUDIO_CFLAGS $GLIB2_CFLAGS $GIO_CFLAGS $GOBJECT2_CFLAGS $SSL_CFLAGS $SASL_CFLAGS SPICE_GTK_CFLAGS=$SPICE_GLIB_CFLAGS $GTK_CFLAGS AC_SUBST(SPICE_GLIB_CFLAGS) diff --git a/src/Makefile.am b/src/Makefile.am index cf02198..925c75f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -96,7 +96,8 @@ SPICE_COMMON_CPPFLAGS = \ $(GOBJECT2_CFLAGS) \ $(SSL_CFLAGS) \ $(SASL_CFLAGS) \ - $(GST_CFLAGS) \ + $(GSTAUDIO_CFLAGS) \ + $(GSTVIDEO_CFLAGS) \ $(SMARTCARD_CFLAGS) \ $(USBREDIR_CFLAGS) \ $(GUDEV_CFLAGS) \ @@ -207,7 +208,8 @@ libspice_client_glib_2_0_la_LIBADD = \ $(PIXMAN_LIBS
[Spice-devel] [PATCH v2 5/7] spice-gtk: Avoid copying the compressed message data in the GStreamer decoder.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
src/channel-display-gst.c | 89 ---
1 file changed, 84 insertions(+), 5 deletions(-)
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 3024356..59ce3e1 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -48,11 +48,43 @@ struct GstDecoder {
/* -- Output frame data -- */
+GMutex pipeline_mutex;
+GCond pipeline_cond;
+int pipeline_wait;
+uint32_t samples_count;
+
GstSample *sample;
GstMapInfo mapinfo;
};
+/* Signals that the pipeline is done processing the last buffer we gave it.
+ *
+ * @decoder: The video decoder object.
+ * @samples: How many samples to add to the available samples count.
+ */
+static void signal_pipeline(GstDecoder *decoder, uint32_t samples)
+{
+g_mutex_lock(decoder-pipeline_mutex);
+decoder-pipeline_wait = 0;
+decoder-samples_count += samples;
+g_cond_signal(decoder-pipeline_cond);
+g_mutex_unlock(decoder-pipeline_mutex);
+}
+
+static void appsrc_need_data_cb(GstAppSrc *src, guint length, gpointer
user_data)
+{
+GstDecoder *decoder = (GstDecoder*)user_data;
+signal_pipeline(decoder, 0);
+}
+
+static GstFlowReturn appsink_new_sample_cb(GstAppSink *appsrc, gpointer
user_data)
+{
+GstDecoder *decoder = (GstDecoder*)user_data;
+signal_pipeline(decoder, 1);
+return GST_FLOW_OK;
+}
+
/* -- GStreamer pipeline -- */
static void reset_pipeline(GstDecoder *decoder)
@@ -66,10 +98,18 @@ static void reset_pipeline(GstDecoder *decoder)
gst_object_unref(decoder-appsink);
gst_object_unref(decoder-pipeline);
decoder-pipeline = NULL;
+
+g_mutex_clear(decoder-pipeline_mutex);
+g_cond_clear(decoder-pipeline_cond);
}
static gboolean construct_pipeline(GstDecoder *decoder)
{
+g_mutex_init(decoder-pipeline_mutex);
+g_cond_init(decoder-pipeline_cond);
+decoder-pipeline_wait = 1;
+decoder-samples_count = 0;
+
const gchar *src_caps, *gstdec_name;
switch (decoder-base.codec_type) {
case SPICE_VIDEO_CODEC_TYPE_MJPEG:
@@ -101,7 +141,12 @@ static gboolean construct_pipeline(GstDecoder *decoder)
}
decoder-appsrc =
GST_APP_SRC(gst_bin_get_by_name(GST_BIN(decoder-pipeline), src));
+GstAppSrcCallbacks appsrc_cbs = {appsrc_need_data_cb, NULL, NULL};
+gst_app_src_set_callbacks(decoder-appsrc, appsrc_cbs, decoder, NULL);
+
decoder-appsink =
GST_APP_SINK(gst_bin_get_by_name(GST_BIN(decoder-pipeline), sink));
+GstAppSinkCallbacks appsink_cbs = {NULL, NULL, appsink_new_sample_cb};
+gst_app_sink_set_callbacks(decoder-appsink, appsink_cbs, decoder, NULL);
if (gst_element_set_state(decoder-pipeline, GST_STATE_PLAYING) ==
GST_STATE_CHANGE_FAILURE) {
SPICE_DEBUG(GStreamer error: Unable to set the pipeline to the
playing state.);
@@ -112,6 +157,11 @@ static gboolean construct_pipeline(GstDecoder *decoder)
return TRUE;
}
+static void release_msg_in(gpointer data)
+{
+spice_msg_in_unref((SpiceMsgIn*)data);
+}
+
static gboolean push_compressed_buffer(GstDecoder *decoder,
SpiceMsgIn *frame_msg)
{
@@ -122,10 +172,11 @@ static gboolean push_compressed_buffer(GstDecoder
*decoder,
return FALSE;
}
-// TODO. Grr. Seems like a wasted alloc
-gpointer d = g_malloc(size);
-memcpy(d, data, size);
-GstBuffer *buffer = gst_buffer_new_wrapped(d, size);
+/* Reference frame_msg so it stays around until our 'deallocator' releases
it */
+spice_msg_in_ref(frame_msg);
+GstBuffer *buffer = gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_READONLY,
+data, size, 0, size,
+frame_msg,
release_msg_in);
if (gst_app_src_push_buffer(decoder-appsrc, buffer) != GST_FLOW_OK) {
SPICE_DEBUG(GStreamer error: unable to push frame of size %d, size);
@@ -195,8 +246,36 @@ static uint8_t* gst_decoder_decode_frame(GstDecoder
*decoder,
*/
release_last_frame(decoder);
+/* The pipeline may have called appsrc_need_data_cb() after we got the last
+ * output frame. This would cause us to return prematurely so reset
+ * pipeline_wait so we do wait for it to process this buffer.
+ */
+g_mutex_lock(decoder-pipeline_mutex);
+decoder-pipeline_wait = 1;
+g_mutex_unlock(decoder-pipeline_mutex);
+/* Note that it's possible for appsrc_need_data_cb() to get called between
+ * now and the pipeline wait. But this will at most cause a one frame
delay.
+ */
+
if (push_compressed_buffer(decoder, frame_msg)) {
-return pull_raw_frame(decoder);
+/* Wait for the pipeline to either produce a decoded frame, or ask
+ * for more data which means an error happened.
+ */
+g_mutex_lock(decoder-pipeline_mutex
[Spice-devel] [PATCH v2 3/7] spice-common: Add support for the VP8 and h264 video codecs.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This is identical to this patch:
http://lists.freedesktop.org/archives/spice-devel/2015-July/020984.html
spice.proto | 2 ++
1 file changed, 2 insertions(+)
diff --git a/spice.proto b/spice.proto
index 4ea1263..fa4d448 100644
--- a/spice.proto
+++ b/spice.proto
@@ -329,6 +329,8 @@ flags8 path_flags { /* TODO: C enum names changes */
enum8 video_codec_type {
MJPEG = 1,
+VP8,
+H264,
};
flags8 stream_flags {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 2/7] spice-proto: Add support for the VP8 and H264 video codecs and for advertising supported video codecs.
Clients that support multiple codecs should advertise the
SPICE_DISPLAY_CAP_MULTI_CODEC capability and one SPICE_DISPLAY_CAP_CODEC_XXX
per supported codec.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This is identical to this patch:
http://lists.freedesktop.org/archives/spice-devel/2015-July/020986.html
This should be followed by a spice-common commit that ensures we get the
right version of these headers (possibly that commit could be part of
the commit of 3/7).
spice/enums.h| 2 ++
spice/protocol.h | 4
2 files changed, 6 insertions(+)
diff --git a/spice/enums.h b/spice/enums.h
index 6a0ab0b..36b0ea3 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -139,6 +139,8 @@ typedef enum SpicePathFlags {
typedef enum SpiceVideoCodecType {
SPICE_VIDEO_CODEC_TYPE_MJPEG = 1,
+SPICE_VIDEO_CODEC_TYPE_VP8,
+SPICE_VIDEO_CODEC_TYPE_H264,
SPICE_VIDEO_CODEC_TYPE_ENUM_END
} SpiceVideoCodecType;
diff --git a/spice/protocol.h b/spice/protocol.h
index d3c5962..614dcf1 100644
--- a/spice/protocol.h
+++ b/spice/protocol.h
@@ -135,6 +135,10 @@ enum {
SPICE_DISPLAY_CAP_STREAM_REPORT,
SPICE_DISPLAY_CAP_LZ4_COMPRESSION,
SPICE_DISPLAY_CAP_PREF_COMPRESSION,
+SPICE_DISPLAY_CAP_MULTI_CODEC,
+SPICE_DISPLAY_CAP_CODEC_MJPEG,
+SPICE_DISPLAY_CAP_CODEC_VP8,
+SPICE_DISPLAY_CAP_CODEC_H264,
};
enum {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 0/7] spice-gtk: Use GStreamer to decode video
This follows up on the initial GStreamer patch series but focuses on the spice-gtk client this time around. The spice-gtk support is totally compatible with old servers so this patch series can be applied independently from server-side GStreamer video encoding support. In fact spice-gtk uses Spice's builtin MJPEG support by default, so essentially nothing will change. But it's possible to force spice-gtk to use GStreamer for MJPEG streams at build time; and if support for other codecs is added to the server, then this client will be able to decode them. As usual, one can also find these patches on GitHub. See the gst branch of the repositories below: spice: https://github.com/fgouget/spice spice-gtk: https://github.com/fgouget/spice-gtk xf86-video-qxl: https://github.com/fgouget/xf86-video-qxl spice-common: https://github.com/fgouget/spice-common spice-protocol: https://github.com/fgouget/spice-protocol (there's also 'extras' branches with more experimental/future patches for the curious) For spice-html5 and QEMU one would have to refer to the patches posted previously on spice-devel. Let me know if there are changes that are needed for inclusion. -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice 01/13] server: Refactor the Spice server video encoding so alternative implementations can be added. (take 4)
This patch simply replaces the mjpeg_encoder_xxx() call points with a
more object oriented
design.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
On Fri, 31 Jul 2015, Victor Toso wrote:
[...]
+static inline uint8_t *get_image_line(SpiceChunks *chunks, size_t *offset,
+ int *chunk_nr, int stride)
Lining up with opening parenthesis
Done.
+static int encode_mjpeg_frame(MJpegEncoder *encoder, const SpiceRect *src,
+const SpiceBitmap *image, int top_down)
Lining up with opening parenthesis
Done.
+for (i = 0; i skip_lines; i++) {
+get_image_line(chunks, offset, chunk, image_stride);
Decrease one level of identation
Done.
+if (!src_line) {
+return FALSE;
+}
I would remove the brances of above if to keep equal to the if bellow
+
+src_line += src-left * mjpeg_encoder_get_bytes_per_pixel(encoder);
+if (mjpeg_encoder_encode_scanline(encoder, src_line, stream_width)
== 0)
+return FALSE;
+}
Actually I introduced the if below too and it violates the coding
standard that says if statements should always have braces. So I've done
a pass through all my patches and it seems this file contained the only
3 incorrect if statements.
@@ -2693,18 +2693,19 @@ static void red_stop_stream(RedWorker *worker,
Stream *stream)
region_clear(stream_agent-vis_region);
region_clear(stream_agent-clip);
spice_assert(!pipe_item_is_linked(stream_agent-destroy_item));
-if (stream_agent-mjpeg_encoder
dcc-use_mjpeg_encoder_rate_control) {
-uint64_t stream_bit_rate =
mjpeg_encoder_get_bit_rate(stream_agent-mjpeg_encoder);
-
-if (stream_bit_rate dcc-streams_max_bit_rate) {
-spice_debug(old max-bit-rate=%.2f new=%.2f,
-dcc-streams_max_bit_rate / 8.0 / 1024.0 / 1024.0,
-stream_bit_rate / 8.0 / 1024.0 / 1024.0);
-dcc-streams_max_bit_rate = stream_bit_rate;
+if (stream_agent-video_encoder) {
+if (dcc-use_video_encoder_rate_control) {
+uint64_t stream_bit_rate =
stream_agent-video_encoder-get_bit_rate(stream_agent-video_encoder);
+if (stream_bit_rate dcc-streams_max_bit_rate) {
+spice_debug(old max-bit-rate=%.2f new=%.2f,
+dcc-streams_max_bit_rate / 8.0 / 1024.0 / 1024.0,
+stream_bit_rate / 8.0 / 1024.0 / 1024.0);
+dcc-streams_max_bit_rate = stream_bit_rate;
+}
}
+stream-refs++;
+red_channel_client_pipe_add(dcc-common.base,
stream_agent-destroy_item);
Here you actually changed the logic a bit, moving the strem ref and
red_channel_client_pipe_add into the if. It does seem correct but I was
wondering if stream_agent-mjpeg_encoder could be NULL before...
I have not been able to determine that.
mjpeg_encoder is set in the following locations:
* red_display_create_stream()
This is where mjpeg_encoder is initialized and in the old code this
could not fail. In future patches this may fail.
* red_display_stream_agent_stop()
* red_display_destroy_streams_agents()
Checks whether it is non-NULL and then resets it to NULL.
And mjpeg_encoder is checked against NULL in the following places.
* red_print_stream_stats()
* red_stop_stream()
* red_display_update_streams_max_latency()
But red_marshall_stream_data() assumes it is not NULL.
So clearly mjpeg_encoder is normally not NULL in red_stop_stream() and
red_print_stream_stats() otherwise they would be unable to collect the
bit rate and stream statistics. But I could not rule it out
with certainty.
Let me know if you'd prefer this chunk to go in separately.
diff --git a/server/Makefile.am b/server/Makefile.am
index 89a375c..36b6d45 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -81,7 +81,6 @@ libspice_server_la_SOURCES = \
main_channel.c \
main_channel.h \
mjpeg_encoder.c \
- mjpeg_encoder.h \
red_bitmap_utils.h \
red_channel.c \
red_channel.h \
@@ -115,6 +114,7 @@ libspice_server_la_SOURCES =\
spicevmc.c \
spice_timer_queue.c \
spice_timer_queue.h \
+ video_encoder.h \
zlib_encoder.c \
zlib_encoder.h \
spice_bitmap_utils.h\
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 9a41ef3..48042ba
[Spice-devel] [PATCH spice 01/13] server: Refactor the Spice server video encoding so alternative implementations can be added. (take 4)
This patch simply replaces the mjpeg_encoder_xxx() call points with a more
object oriented design.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Changes since take 3:
- A NULL video_encoder means that the stream creation failed so
red_stop_stream() no longer sends messages to destroy it in
that case. This avoids getting error messages in spice-gtk.
- Removed some forward declarations and made more functions static.
- Tweaked some video_encoder.h comments.
Changes since take 2:
- No change.
Changes since take 1:
- I fixed the width/height comments and they now state that they always
match src.
server/Makefile.am | 2 +-
server/mjpeg_encoder.c | 227 +++--
server/mjpeg_encoder.h | 114 -
server/red_worker.c| 173 -
server/video_encoder.h | 165 +++
5 files changed, 381 insertions(+), 300 deletions(-)
delete mode 100644 server/mjpeg_encoder.h
create mode 100644 server/video_encoder.h
diff --git a/server/Makefile.am b/server/Makefile.am
index 89a375c..36b6d45 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -81,7 +81,6 @@ libspice_server_la_SOURCES = \
main_channel.c \
main_channel.h \
mjpeg_encoder.c \
- mjpeg_encoder.h \
red_bitmap_utils.h \
red_channel.c \
red_channel.h \
@@ -115,6 +114,7 @@ libspice_server_la_SOURCES =\
spicevmc.c \
spice_timer_queue.c \
spice_timer_queue.h \
+ video_encoder.h \
zlib_encoder.c \
zlib_encoder.h \
spice_bitmap_utils.h\
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 9a41ef3..48042ba 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -20,7 +20,11 @@
#endif
#include red_common.h
-#include mjpeg_encoder.h
+
+typedef struct MJpegEncoder MJpegEncoder;
+#define VIDEO_ENCODER_T MJpegEncoder
+#include video_encoder.h
+
#include jerror.h
#include jpeglib.h
#include inttypes.h
@@ -154,6 +158,7 @@ typedef struct MJpegEncoderRateControl {
} MJpegEncoderRateControl;
struct MJpegEncoder {
+VideoEncoder base;
uint8_t *row;
uint32_t row_size;
int first_frame;
@@ -165,7 +170,7 @@ struct MJpegEncoder {
void (*pixel_converter)(uint8_t *src, uint8_t *dest);
MJpegEncoderRateControl rate_control;
-MJpegEncoderRateControlCbs cbs;
+VideoEncoderRateControlCbs cbs;
void *cbs_opaque;
/* stats */
@@ -174,11 +179,6 @@ struct MJpegEncoder {
uint32_t num_frames;
};
-static inline void mjpeg_encoder_reset_quality(MJpegEncoder *encoder,
- int quality_id,
- uint32_t fps,
- uint64_t frame_enc_size);
-static uint32_t get_max_fps(uint64_t frame_size, uint64_t bytes_per_sec);
static void mjpeg_encoder_process_server_drops(MJpegEncoder *encoder);
static uint32_t get_min_required_playback_delay(uint64_t frame_enc_size,
uint64_t byte_rate,
@@ -189,49 +189,14 @@ static inline int rate_control_is_active(MJpegEncoder*
encoder)
return encoder-cbs.get_roundtrip_ms != NULL;
}
-MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
-MJpegEncoderRateControlCbs *cbs, void *opaque)
-{
-MJpegEncoder *enc;
-
-spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
-
-enc = spice_new0(MJpegEncoder, 1);
-
-enc-first_frame = TRUE;
-enc-rate_control.byte_rate = starting_bit_rate / 8;
-enc-starting_bit_rate = starting_bit_rate;
-
-if (cbs) {
-struct timespec time;
-
-clock_gettime(CLOCK_MONOTONIC, time);
-enc-cbs = *cbs;
-enc-cbs_opaque = opaque;
-mjpeg_encoder_reset_quality(enc, MJPEG_QUALITY_SAMPLE_NUM / 2, 5, 0);
-enc-rate_control.during_quality_eval = TRUE;
-enc-rate_control.quality_eval_data.type = MJPEG_QUALITY_EVAL_TYPE_SET;
-enc-rate_control.quality_eval_data.reason =
MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
-enc-rate_control.warmup_start_time = ((uint64_t) time.tv_sec) *
10 + time.tv_nsec;
-} else {
-enc-cbs.get_roundtrip_ms = NULL;
-mjpeg_encoder_reset_quality(enc, MJPEG_LEGACY_STATIC_QUALITY_ID,
MJPEG_MAX_FPS, 0);
-}
-
-enc-cinfo.err = jpeg_std_error(enc-jerr);
-jpeg_create_compress(enc-cinfo);
-
-return enc;
-}
-
-void mjpeg_encoder_destroy(MJpegEncoder *encoder
[Spice-devel] [PATCH spice 02/13] server: Add a GStreamer 0.10 MJPEG video encoder and use it by default. (take 4)
The GStreamer video encoder supports both regular and sized streams.
It is otherwise quite basic and lacks any rate control: the bitrate is set at
startup and will not react to changes in the network conditions. Still it
should work fine on LANs.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Changes since take 3:
- Failing to create the pipeline is probably a permanent issue so if
that happens encode_frame() now returns
VIDEO_ENCODER_FRAME_UNSUPPORTED so that Spice falls back to the
non-stream way of sending screen updates.
- The video encoder will not get a zero bit rate so I removed handling
of that case. The bit rate floor has been raised to 128kbps. Going
below that does not seem very meaningful. Calculating the bit rate
ceiling has been moved to the get_bit_rate_cap() function.
- Removed some redundant field initializations and added a comment.
- Added a get_mbps() helper to simplify printing bandwidth values.
- Some reformating and reordering to group related functions.
Changes since take 2:
- I resolved the buffer timestamping issues. Appsrc is no longer a live
source (it had no reason to be) and it performs the timestamping. The
only remaining annoyance is that ffenc_mjpeg requires removing the
pipeline's default clock otherwise it gets stuck with the following
message:
ERROR ffmpeg :0:: Error, Invalid timestamp=0, last=0
I consider this to be an ffenc_mjpeg bug since none of the other
encoders (VP8, H264 and even avenc_mpjpeg in GStreamer 1.0) has this
issue. So I'm keeping the clock removal as a workaround for
ffenc_mjpeg.
- I went back to ffmpegcolorspace because it's the standard way to do
color conversion in GStreamer 0.10 (present in gst-plugins-base),
while autovideoconvert is only present in gst-plugins-bad which I
believe we don't depend on.
- I simplified the source frame copy code (push_raw_frame()). It's also
ready for adding zero-copy once GStreamer 1.0 support is added.
- I improved the autoconf GStreamer 0.10 detection: it's now possible
to require GStreamer support, or just let it be included if present.
This also paves the way for GStreamer 1.0.
- Changed set_appsrc_caps() and construct_pipeline() to return a
gboolean since they only return TRUE/FALSE.
- Fixed some formatting issues, mostly braces placement.
Changes since take 1:
- The width/height comments are now correct in the previous patch of
the series.
- Fixed the pipeline reconfiguration (for video size changes) so it
still works if we have to fall back to rebuilding the pipeline from
scratch.
- Added a comment suggesting to avoid copying the compressed buffer as
a future improvement. I think this will require changing the way the
output buffer is allocated though. So I'm leaving that for after the
basic support committed.
configure.ac | 26 +++
server/Makefile.am | 8 +
server/gstreamer_encoder.c | 451 +
server/red_worker.c| 11 +-
server/video_encoder.h | 6 +
5 files changed, 500 insertions(+), 2 deletions(-)
create mode 100644 server/gstreamer_encoder.c
diff --git a/configure.ac b/configure.ac
index 5b4caa4..ca91b5a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -80,6 +80,30 @@ AS_IF([test x$enable_opengl != xno], [
SPICE_CHECK_SMARTCARD([SMARTCARD])
AM_CONDITIONAL(SUPPORT_SMARTCARD, test x$have_smartcard = xyes)
+AC_ARG_ENABLE(gstreamer,
+ AS_HELP_STRING([--enable-gstreamer=@:@auto/yes/no@:@],
+ [Enable GStreamer 0.10 support]),
+ [],
+ [enable_gstreamer=auto])
+
+if test x$enable_gstreamer != xno; then
+PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10],
+ [enable_gstreamer=yes
+ have_gstreamer_0_10=yes],
+ [have_gstreamer_0_10=no])
+if test x$have_gstreamer_0_10 = xyes; then
+AC_SUBST(GSTREAMER_0_10_CFLAGS)
+AC_SUBST(GSTREAMER_0_10_LIBS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10 gstreamer-app-0.10])
+AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer
0.10])
+elif test x$enable_gstreamer = xyes; then
+AC_MSG_ERROR([GStreamer 0.10 support requested but not found. You may
set GSTREAMER_0_10_CFLAGS and GSTREAMER_0_10_LIBS to avoid the need to call
pkg-config.])
+fi
+else
+have_gstreamer_0_10=no
+fi
+AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$have_gstreamer_0_10 = xyes)
+
AC_ARG_ENABLE([automated_tests],
AS_HELP_STRING([--enable-automated-tests], [Enable automated
tests using spicy-screenshot (part of spice--gtk)]),,
[enable_automated_tests=no])
@@ -311,6 +335,8 @@ echo
Smartcard:${have_smartcard}
+GStreamer 0.10: ${have_gstreamer_0_10}
+
SASL
[Spice-devel] [PATCH protocol 04/13] Add support for the VP8 and H264 video codecs and for advertising supported video codecs. (take 4)
Clients that support multiple codecs should advertise the
SPICE_DISPLAY_CAP_MULTI_CODEC capability and one SPICE_DISPLAY_CAP_CODEC_XXX
per supported codec.
---
This should be followed by a spice-common commit that ensures we get the
right version of these headers (possibly that commit could be part of
the commit of 03/13).
Changes since take 3:
- None
Changes since take 2:
- This patch also adds h264.
spice/enums.h| 2 ++
spice/protocol.h | 4
2 files changed, 6 insertions(+)
diff --git a/spice/enums.h b/spice/enums.h
index 6a0ab0b..36b0ea3 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -139,6 +139,8 @@ typedef enum SpicePathFlags {
typedef enum SpiceVideoCodecType {
SPICE_VIDEO_CODEC_TYPE_MJPEG = 1,
+SPICE_VIDEO_CODEC_TYPE_VP8,
+SPICE_VIDEO_CODEC_TYPE_H264,
SPICE_VIDEO_CODEC_TYPE_ENUM_END
} SpiceVideoCodecType;
diff --git a/spice/protocol.h b/spice/protocol.h
index d3c5962..614dcf1 100644
--- a/spice/protocol.h
+++ b/spice/protocol.h
@@ -135,6 +135,10 @@ enum {
SPICE_DISPLAY_CAP_STREAM_REPORT,
SPICE_DISPLAY_CAP_LZ4_COMPRESSION,
SPICE_DISPLAY_CAP_PREF_COMPRESSION,
+SPICE_DISPLAY_CAP_MULTI_CODEC,
+SPICE_DISPLAY_CAP_CODEC_MJPEG,
+SPICE_DISPLAY_CAP_CODEC_VP8,
+SPICE_DISPLAY_CAP_CODEC_H264,
};
enum {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 05/13] server: Add VP8 support, a video codec preference list and compatibility checks with the Spice client. (take 4)
The video encoder preferences can be expressed by building a semi-colon
separated list of encoder:codec pairs. For instance 'gstreamer:vp8;spice:mjpeg'
to pick first the GStreamer VP8 video encoder first and used the original MJPEG
video encoder one as a fallback.
The server has a default preference list which can also be selected by
specifying 'auto' as the preferences list.
The client compatibility check relies on the following new capabilities:
* SPICE_DISPLAY_CAP_MULTI_CODEC which denotes a recent client that supports
multiple codecs. This capability is needed to not have to hardcode that MJPEG
is supported. This makes it possible to write clients that don't support MJPEG.
* SPICE_DISPLAY_CAP_CODEC_XXX, where XXX is a supported codec, for now MJPEG
and VP8.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Note that this patch depends on the 03/13 and 04/13 spice-common
submodule patches.
Changes since take 3:
- Proper fallback in case the server and client have no video codec
in common.
- Check whether g_get_num_processors() is available for compatibility
with glib 2.35 and older.
Changes since take 2:
- We now allow the VP8 encoder to produce P frames since this does not
increase the latency (i.e. it still lets us get the compressed frames
right away), while improving the compression ratio.
- It turns out the above also helps with CPU usage and lets us
further configure the encoder for speed.
- Only construct_pipeline() really needs to know the encoder name so
we no longer keep it in a GstEncoder field.
- Fixed some formatting issues, mostly braces placement.
Changes since take 1:
- Only set the parameters supported by the current encoder.
- reconfigure_pipeline() handling has been fixed in the previous patch.
- The video codecs list now has a more sensible default and it's
possible to explicitly request this default by specifying 'auto' in
the configuration files.
configure.ac | 4 ++
server/gstreamer_encoder.c | 66 ++---
server/mjpeg_encoder.c | 7 +-
server/red_dispatcher.c| 172 -
server/red_dispatcher.h| 8 +++
server/red_worker.c| 82 +
server/red_worker.h| 18 +
server/reds.c | 12
server/spice-server.h | 1 +
server/spice-server.syms | 1 +
server/video_encoder.h | 18 +++--
11 files changed, 343 insertions(+), 46 deletions(-)
diff --git a/configure.ac b/configure.ac
index ca91b5a..e5be699 100644
--- a/configure.ac
+++ b/configure.ac
@@ -136,6 +136,10 @@ AS_IF([test x$have_smartcard = xyes], [
PKG_CHECK_MODULES([GLIB2], [glib-2.0 = 2.22])
AS_VAR_APPEND([SPICE_REQUIRES], [ glib-2.0 = 2.22])
+AC_CHECK_LIB(glib-2.0, g_get_num_processors,
+ AC_DEFINE([HAVE_G_GET_NUMPROCESSORS], 1, [Defined if we have
g_get_num_processors()]),,
+ $GLIB2_LIBS)
+
PKG_CHECK_MODULES(PIXMAN, pixman-1 = 0.17.7)
AC_SUBST(PIXMAN_CFLAGS)
AC_SUBST(PIXMAN_LIBS)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 8a21ad0..140261e 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -190,10 +190,27 @@ static gboolean set_appsrc_caps(GstEncoder *encoder)
/* A helper for gst_encoder_encode_frame(). */
static gboolean construct_pipeline(GstEncoder *encoder, const SpiceBitmap
*bitmap)
{
+gboolean no_clock = FALSE;
+const gchar* gstenc_name;
+switch (encoder-base.codec_type)
+{
+case SPICE_VIDEO_CODEC_TYPE_MJPEG:
+gstenc_name = ffenc_mjpeg;
+no_clock = TRUE;
+break;
+case SPICE_VIDEO_CODEC_TYPE_VP8:
+gstenc_name = vp8enc;
+break;
+default:
+spice_warning(unsupported codec type %d, encoder-base.codec_type);
+return FALSE;
+}
+
GError *err = NULL;
-const gchar* desc = appsrc name=src format=2 do-timestamp=true !
ffmpegcolorspace ! ffenc_mjpeg name=encoder ! appsink name=sink;
+gchar *desc = g_strdup_printf(appsrc name=src format=2 do-timestamp=true
! ffmpegcolorspace ! %s name=encoder ! appsink name=sink, gstenc_name);
spice_debug(GStreamer pipeline: %s, desc);
encoder-pipeline = gst_parse_launch_full(desc, NULL,
GST_PARSE_FLAG_FATAL_ERRORS, err);
+g_free(desc);
if (!encoder-pipeline) {
spice_warning(GStreamer error: %s, err-message);
g_clear_error(err);
@@ -203,19 +220,36 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
encoder-gstenc = gst_bin_get_by_name(GST_BIN(encoder-pipeline),
encoder);
encoder-appsink =
GST_APP_SINK(gst_bin_get_by_name(GST_BIN(encoder-pipeline), sink));
+/* Configure the encoders for a zero-frame latency, and real-time speed */
+adjust_bit_rate(encoder);
+g_object_set(G_OBJECT(encoder-gstenc), bitrate, encoder-bit_rate,
NULL);
+if (encoder-base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8
[Spice-devel] [PATCH 00/13] Add GStreamer support for video streams (take 4)
This follows up on the previous patch series with the addition of full bit rate support. This brings the GStreamer video encoder to feature parity with the builtin Spice MJPEG one so I think it's ready to be committed. To summarize the changes since the last round: - The bit rate is automatically adjusted based on the network conditions, both down and up. - Zero-copy for the compressed output buffer. - Proper fallback in case the server and client have no video codec in common (which would only happen if the client does not support MJPEG streams). As for the previous round I'm only sending the patches needed for the Spice server to limit the size of this series. The GStreamer MJPEG encoder is fully compatible with existing clients so this should not hinder testing. I will post a new spice-gtk patch series soon but in the meantime one can fetch patches from GitHub to test the VP8 and h264 codecs. See the gst branch of the repositories below: spice: https://github.com/fgouget/spice spice-gtk: https://github.com/fgouget/spice-gtk xf86-video-qxl: https://github.com/fgouget/xf86-video-qxl spice-common: https://github.com/fgouget/spice-common spice-protocol: https://github.com/fgouget/spice-protocol (there's also 'extras' branches with more experimental/future patches for the curious) For spice-html5 and QEMU one would have to refer to the patches posted previously on spice-devel. They should still work with this series. Let me know if there are changes that are needed for inclusion. -- Francois Gouget fgou...@codeweavers.com ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH common 03/13] spice.proto: Add support for the VP8 and h264 video codecs. (take 4)
---
Changes since take 3:
- None
Changes since take 2:
- This patch also adds h264.
spice.proto | 2 ++
1 file changed, 2 insertions(+)
diff --git a/spice.proto b/spice.proto
index 4ea1263..fa4d448 100644
--- a/spice.proto
+++ b/spice.proto
@@ -329,6 +329,8 @@ flags8 path_flags { /* TODO: C enum names changes */
enum8 video_codec_type {
MJPEG = 1,
+VP8,
+H264,
};
flags8 stream_flags {
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 11/13] server: Add h264 support to the GStreamer video encoder. (take 4)
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
Having support for h264 is interesting in its own right but this shows
adding extra codecs is quite easy.
Changes since take 3:
- None.
server/gstreamer_encoder.c | 17 -
server/red_dispatcher.c| 2 ++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 2ad1890..52b3e03 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -266,6 +266,9 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
case SPICE_VIDEO_CODEC_TYPE_VP8:
gstenc_name = vp8enc;
break;
+case SPICE_VIDEO_CODEC_TYPE_H264:
+gstenc_name = x264enc;
+break;
default:
spice_warning(unsupported codec type %d, encoder-base.codec_type);
return FALSE;
@@ -322,6 +325,17 @@ static gboolean construct_pipeline(GstEncoder *encoder,
const SpiceBitmap *bitma
NULL);
break;
}
+case SPICE_VIDEO_CODEC_TYPE_H264:
+g_object_set(G_OBJECT(encoder-gstenc),
+ bitrate, encoder-bit_rate / 1024,
+ byte-stream, TRUE,
+ aud, FALSE,
+ tune, 4, /* Zero latency */
+ intra-refresh, TRUE,
+ sliced-threads, TRUE,
+ speed-preset, 1, /* ultrafast */
+ NULL);
+break;
default:
spice_warning(unknown encoder type %d, encoder-base.codec_type);
reset_pipeline(encoder);
@@ -674,7 +688,8 @@ GstEncoder *create_gstreamer_encoder(SpiceVideoCodecType
codec_type, uint64_t st
spice_assert(!cbs || (cbs cbs-get_roundtrip_ms
cbs-get_source_fps));
if (codec_type != SPICE_VIDEO_CODEC_TYPE_MJPEG
-codec_type != SPICE_VIDEO_CODEC_TYPE_VP8) {
+codec_type != SPICE_VIDEO_CODEC_TYPE_VP8
+codec_type != SPICE_VIDEO_CODEC_TYPE_H264) {
spice_warning(unsupported codec type %d, codec_type);
return NULL;
}
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index d896d00..14d3f86 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -268,12 +268,14 @@ static create_video_encoder_proc video_encoder_procs[] = {
static const EnumNames video_codec_names[] = {
{SPICE_VIDEO_CODEC_TYPE_MJPEG, mjpeg},
{SPICE_VIDEO_CODEC_TYPE_VP8, vp8},
+{SPICE_VIDEO_CODEC_TYPE_H264, h264},
{0, NULL},
};
static const EnumNames video_cap_names[] = {
{SPICE_DISPLAY_CAP_CODEC_MJPEG, mjpeg},
{SPICE_DISPLAY_CAP_CODEC_VP8, vp8},
+{SPICE_DISPLAY_CAP_CODEC_H264, h264},
{0, NULL},
};
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 09/13] server: Add GStreamer 1.0 support to the GStreamer video encoder. (take 4)
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
By default GStreamer 1.0 is used if available, otherwise GStreamer 0.10
is used and Spice is compiled without GStreamer support as a last
resort. It's possible to explicitly require a specific Gstreamer version
with --enable-gstreamer=1.0 and --enable-gstreamer=0.10, or for any
version with --enable-gstreamer=yes.
If you get an error while building the pipeline for MJPEG, then it
probably means you need the fix for this bug:
https://bugzilla.gnome.org/show_bug.cgi?id=750398
Changes since take 3:
- Check whether g_get_num_processors() is available for compatibility
with glib 2.35 and older.
- avenc_mjpeg needs the clock to be removed just like ffenc_mjpeg, its
0.10 counterpart.
- Support for the new output buffer handling.
configure.ac | 35 +++
server/Makefile.am | 12 +--
server/gstreamer_encoder.c | 84 +-
server/red_dispatcher.c| 2 +-
4 files changed, 116 insertions(+), 17 deletions(-)
diff --git a/configure.ac b/configure.ac
index e5be699..702cd49 100644
--- a/configure.ac
+++ b/configure.ac
@@ -81,14 +81,32 @@ SPICE_CHECK_SMARTCARD([SMARTCARD])
AM_CONDITIONAL(SUPPORT_SMARTCARD, test x$have_smartcard = xyes)
AC_ARG_ENABLE(gstreamer,
- AS_HELP_STRING([--enable-gstreamer=@:@auto/yes/no@:@],
- [Enable GStreamer 0.10 support]),
+ AS_HELP_STRING([--enable-gstreamer=@:@auto/0.10/1.0/yes/no@:@],
+ [Enable GStreamer support]),
[],
[enable_gstreamer=auto])
-if test x$enable_gstreamer != xno; then
+if test x$enable_gstreamer != xno test x$enable_gstreamer != x0.10;
then
+PKG_CHECK_MODULES(GSTREAMER_1_0, [gstreamer-1.0, gstreamer-app-1.0],
+ [enable_gstreamer=1.0
+ have_gstreamer_1_0=yes],
+ [have_gstreamer_1_0=no])
+if test x$have_gstreamer_1_0 = xyes; then
+AC_SUBST(GSTREAMER_1_0_CFLAGS)
+AC_SUBST(GSTREAMER_1_0_LIBS)
+AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-1.0 gstreamer-app-1.0])
+AC_DEFINE([HAVE_GSTREAMER_1_0], [1], [Define if supporting GStreamer
1.0])
+elif test x$enable_gstreamer = x1.0; then
+AC_MSG_ERROR([GStreamer 1.0 support requested but not found. You may
set GSTREAMER_1_0_CFLAGS and GSTREAMER_1_0_LIBS to avoid the need to call
pkg-config.])
+fi
+else
+have_gstreamer_1_0=no
+fi
+AM_CONDITIONAL(SUPPORT_GSTREAMER_1_0, test x$have_gstreamer_1_0 = xyes)
+
+if test x$enable_gstreamer != xno test x$enable_gstreamer != x1.0;
then
PKG_CHECK_MODULES(GSTREAMER_0_10, [gstreamer-0.10, gstreamer-app-0.10],
- [enable_gstreamer=yes
+ [enable_gstreamer=0.10
have_gstreamer_0_10=yes],
[have_gstreamer_0_10=no])
if test x$have_gstreamer_0_10 = xyes; then
@@ -96,7 +114,7 @@ if test x$enable_gstreamer != xno; then
AC_SUBST(GSTREAMER_0_10_LIBS)
AS_VAR_APPEND([SPICE_REQUIRES], [ gstreamer-0.10 gstreamer-app-0.10])
AC_DEFINE([HAVE_GSTREAMER_0_10], [1], [Define if supporting GStreamer
0.10])
-elif test x$enable_gstreamer = xyes; then
+elif test x$enable_gstreamer = x0.10; then
AC_MSG_ERROR([GStreamer 0.10 support requested but not found. You may
set GSTREAMER_0_10_CFLAGS and GSTREAMER_0_10_LIBS to avoid the need to call
pkg-config.])
fi
else
@@ -104,6 +122,11 @@ else
fi
AM_CONDITIONAL(SUPPORT_GSTREAMER_0_10, test x$have_gstreamer_0_10 = xyes)
+if test x$enable_gstreamer = xyes; then
+AC_MSG_ERROR(GStreamer support requested but not found)
+fi
+AS_IF([test x$enable_gstreamer = xauto], [enable_gstreamer=no])
+
AC_ARG_ENABLE([automated_tests],
AS_HELP_STRING([--enable-automated-tests], [Enable automated
tests using spicy-screenshot (part of spice--gtk)]),,
[enable_automated_tests=no])
@@ -339,7 +362,7 @@ echo
Smartcard:${have_smartcard}
-GStreamer 0.10: ${have_gstreamer_0_10}
+GStreamer:${enable_gstreamer}
SASL support: ${enable_sasl}
diff --git a/server/Makefile.am b/server/Makefile.am
index 4921bc3..9fb0c8e 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -12,6 +12,7 @@ AM_CPPFLAGS = \
$(SLIRP_CFLAGS) \
$(SMARTCARD_CFLAGS) \
$(GSTREAMER_0_10_CFLAGS)\
+ $(GSTREAMER_1_0_CFLAGS) \
$(SSL_CFLAGS) \
$(VISIBILITY_HIDDEN_CFLAGS) \
$(WARN_CFLAGS) \
@@ -42,7 +43,8 @@ libspice_server_la_LIBADD =
\
$(PIXMAN_LIBS
[Spice-devel] [PATCH spice 10/13] server: Avoid copying the input frame in the GStreamer encoder. (take 4)
To do so we reference the source bitmap chunks in the GStreamer buffer and rely
on the buffer's lifetime being short enough.
Note that we can only avoid copies for the first 1 Mpixels or so. That's
because Spice splits larger frames into more chunks and we can fit memory
fragments inside a GStreamer buffer. So for those we will avoid copies for the
first 3840 KB and copy the rest.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This makes it possible to avoid copying the source frame when using
GStreamer 1.0. Paradoxically we are still forced to do some copying for
the larger frames.
Here is how it works: Spice's frame buffer is composed of multiple
memory chunks. So we use GStreamer's GstMemory objects to wrap each
chunk and put them all to form the GstBuffer we pass to the pipeline.
However there's a limit to the number of memory objects that we can put
in a GstBuffer. Usually that limit is 16. Furthermore Spice splits the
frame into 256KB chunks. so this approach only works up to 4MB or
between 1 and 1.3Mpixels. For larger frames we use the zero-copy
approach for the first 15 frame chunks, and copy all the rest into the
16th memory chunk.
So on my machine, for a 720x304 video the frame copy time goes from
around 265us (line-by-line) to 5us. For a 1440x900 frame it goes from
about 1610us with the old line-by-line copy code, down to 1490us for the
new chunk-by-chunk one, and down to about 440us when minimizing copies.
The latter matches well with the ~0.3Mpixels we have to copy after the
first 1Mpixels.
To avoid these copies it will be necessary to either increase the size
of the Spice chunks or to increase the number of memory objects that
GstBuffers can hold.
Changes since take 3:
- None.
server/gstreamer_encoder.c | 107 ++---
1 file changed, 101 insertions(+), 6 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index ac8f5c0..2ad1890 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -35,6 +35,10 @@ typedef struct GstEncoder GstEncoder;
#define GSTE_DEFAULT_FPS 30
+#ifndef HAVE_GSTREAMER_0_10
+# define DO_ZERO_COPY
+#endif
+
typedef struct {
SpiceBitmapFmt spice_format;
@@ -86,6 +90,11 @@ struct GstEncoder {
/* The default video buffer */
GstVideoBuffer *default_buffer;
+#ifdef DO_ZERO_COPY
+/* Set to TRUE until GStreamer no longer needs the raw bitmap buffer. */
+gboolean needs_bitmap;
+#endif
+
/* The frame counter for GStreamer buffers */
uint32_t frame;
@@ -355,6 +364,15 @@ static void reconfigure_pipeline(GstEncoder *encoder)
}
}
+#ifdef DO_ZERO_COPY
+/* A helper for push_raw_frame() */
+static void unref_bitmap(gpointer mem)
+{
+GstEncoder *encoder = (GstEncoder*)mem;
+encoder-needs_bitmap = FALSE;
+}
+#endif
+
/* A helper for gst_encoder_encode_frame(). */
static int push_raw_frame(GstEncoder *encoder, const SpiceBitmap *bitmap,
const SpiceRect *src, int top_down)
@@ -362,15 +380,14 @@ static int push_raw_frame(GstEncoder *encoder, const
SpiceBitmap *bitmap,
const uint32_t stream_height = src-bottom - src-top;
const uint32_t stream_stride = (src-right - src-left) *
encoder-format-bpp / 8;
uint32_t len = stream_stride * stream_height;
-GstBuffer *buffer = gst_buffer_new_and_alloc(len);
#ifdef HAVE_GSTREAMER_0_10
+GstBuffer *buffer = gst_buffer_new_and_alloc(len);
uint8_t *b = GST_BUFFER_DATA(buffer);
+uint8_t *dst = b;
#else
-GstMapInfo map;
-gst_buffer_map(buffer, map, GST_MAP_WRITE);
-uint8_t *b = map.data;
+GstBuffer *buffer = gst_buffer_new();
+GstMapInfo map = { .memory = NULL };
#endif
-uint8_t *dst = b;
/* Note that we should not reorder the lines, even if top_down is false.
* It just changes the number of lines to skip at the start of the bitmap.
@@ -382,6 +399,70 @@ static int push_raw_frame(GstEncoder *encoder, const
SpiceBitmap *bitmap,
if (stream_stride == bitmap-stride) {
/* We can copy the bitmap chunk by chunk */
+#ifdef DO_ZERO_COPY
+/* We cannot control the lifetime of the bitmap but we can wrap it in
+ * the buffer anyway because:
+ * - Before returning from gst_encoder_encode_frame() we wait for the
+ * pipeline to have converted this frame into a compressed buffer.
+ * So it has to have gone through the frame at least once.
+ * - For all encoders but MJPEG, the first element of the pipeline will
+ * convert the bitmap to another image format which entails copying
+ * it. So by the time the encoder starts its work, this buffer will
+ * not be needed anymore.
+ * - The MJPEG encoder does not perform inter-frame compression and
thus
+ * does not need to keep hold of this buffer once it has processed
it.
+ */
+while (chunk_offset = chunks-chunk[chunk
[Spice-devel] [PATCH spice 12/13] server: Shape the bit rate of the GStreamer video encoders output.
The GStreamer encoders don't follow the specified bit rate very closely: they
can decide to exceed it for ten seconds or more if they consider the scene
deserves it. Such long bursts are enough to cause network congestion, resulting
in long bouts of dropped frames.
So the GStreamer video encoder now uses a virtual buffer to shape the
compressed video output and ensure we don't exceed the target bit rate for any
significant length of time, which makes it possible to use higher bit rates
overall.
It also keeps track of the encoded frame size so it can gather statistics and
call update_client_playback_delay() with accurate information and also annotate
the client report debug traces with the corresponding bit rate information.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
The bit rate control code can be split into two parts:
1. Code to turn the raw video into a compressed stream of the given bit rate.
2. Feedback code that adjusts the bit rate based on the network conditions.
In theory the GStreamer encoders implement part 1 for us but, as stated
in the commit message, in practice they are not strict enough (and most
cannot be tweaked in this respect).
It's interesting to note that if the feedback mechanism gets good,
timely information we can pretty much do without this patch: when
GStreamer exceeds the set bit rate the feedback mechanism will notice a
degradation of the network conditions and lower the target bit rate
which will lower the GStreamer's output bit rate (even if it still
exceeds the target bit rate a bit).
However this is suboptimal as it will force the feedback mechanism to
either change the bit rate more frequently (causing a GStreamer pipeline
reinitialization most of the time and degrading compression levels), or
select a lower bit rate so the network capacity is not exceeded even if
GStreamer goes 10 or 20% above the set target.
Also note that despite wanting to avoid exceeding the target bit rate,
part 1 cannot consider frames individually, if only to correctly handle
VP8's large I vs. P frame size discrepancy. Exceeding the target bit
rate for sub-second periods generally does not cause network congestion
hence the selection of a 300ms virtual buffer.
server/gstreamer_encoder.c | 287 +++--
1 file changed, 279 insertions(+), 8 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 52b3e03..4089ead 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -39,6 +39,9 @@ typedef struct GstEncoder GstEncoder;
# define DO_ZERO_COPY
#endif
+#define NANO_SECOND (10LL)
+#define MILLI_SECOND (1000LL)
+#define NANO_MS (NANO_SECOND / MILLI_SECOND)
typedef struct {
SpiceBitmapFmt spice_format;
@@ -59,6 +62,11 @@ struct GstVideoBuffer {
gboolean persistent;
};
+typedef struct {
+uint32_t mm_time;
+uint32_t size;
+} GstFrameInformation;
+
struct GstEncoder {
VideoEncoder base;
@@ -98,11 +106,71 @@ struct GstEncoder {
/* The frame counter for GStreamer buffers */
uint32_t frame;
+
+/* -- Encoded frame statistics -- */
+
+/* Should be = than FRAME_STATISTICS_COUNT. This is also used to annotate
+ * the client report debug traces with bit rate information.
+ */
+# define GSTE_HISTORY_SIZE 60
+
+/* A circular buffer containing the past encoded frames information. */
+GstFrameInformation history[GSTE_HISTORY_SIZE];
+
+/* The indices of the oldest and newest frames in the history buffer. */
+uint32_t history_first;
+uint32_t history_last;
+
+/* How many frames to take into account when computing the effective
+ * bit rate, average frame size, etc. This should be large enough so the
+ * I and P frames average out, and short enough for it to reflect the
+ * current situation.
+ */
+# define GSTE_FRAME_STATISTICS_COUNT 21
+
+/* The index of the oldest frame taken into account for the statistics. */
+uint32_t stat_first;
+
+/* Used to compute the average frame size. */
+uint64_t stat_sum;
+
+/* Tracks the maximum frame size. */
+uint32_t stat_maximum;
+
+
+/* -- Encoder bit rate control --
+ *
+ * GStreamer encoders don't follow the specified bit rate very
+ * closely. These fields are used to ensure we don't exceed the desired
+ * stream bit rate, regardless of the GStreamer encoder's output.
+ */
+
/* The bit rate target for the outgoing network stream. (bits per second)
*/
uint64_t bit_rate;
/* The minimum bit rate */
# define GSTE_MIN_BITRATE (128 * 1024)
+
+/* The bit rate control is performed using a virtual buffer to allow short
+ * term variations: bursts are allowed until the virtual buffer is full.
+ * Then frames are dropped to limit the bit rate. VBUFFER_SIZE defines the
+ * size of the virtual buffer in milliseconds worth of data
[Spice-devel] [PATCH qxl 07/13] Xspice: Add a --video-codecs option to specify which encoder:codec to use. (take 4)
---
Changes since take 3:
- None.
scripts/Xspice | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/Xspice b/scripts/Xspice
index 281535d..02875f5 100755
--- a/scripts/Xspice
+++ b/scripts/Xspice
@@ -86,6 +86,7 @@ parser.add_argument('--zlib-glz-wan-compression',
# TODO - sound support
parser.add_argument('--streaming-video', choices=['off', 'all', 'filter'],
default='filter', help='filter by default')
+parser.add_argument('--video-codecs', help=Sets a semicolon-separated list of
preferred video codecs to use. Each takes the form encoder:codec, with
spice:mjpeg being the default and other options being provided by gstreamer for
the mjpeg, vp8 and h264 codecs.)
add_boolean('--ipv4-only')
add_boolean('--ipv6-only')
parser.add_argument('--vdagent', action='store_true', dest='vdagent_enabled',
default=False, help='launch vdagent vdagentd. They provide clipboard
resolution automation')
@@ -251,7 +252,7 @@ var_args = ['port', 'tls_port', 'disable_ticketing',
'x509_key_file', 'x509_key_password',
'tls_ciphers', 'dh_file', 'password', 'image_compression',
'jpeg_wan_compression', 'zlib_glz_wan_compression',
-'streaming_video', 'deferred_fps', 'exit_on_disconnect',
+'streaming_video', 'video_codecs', 'deferred_fps', 'exit_on_disconnect',
'vdagent_enabled', 'vdagent_virtio_path', 'vdagent_uinput_path',
'vdagent_uid', 'vdagent_gid']
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH qxl 06/13] spiceqxl: Add SpiceVideoCodecs and $XSPICE_VIDEO_CODECS for specifying video codec preferences. (take 4)
---
Changes since take 3:
- None.
Changes since take 1:
- Fixed a brace placement.
examples/spiceqxl.xorg.conf.example | 7 +++
src/qxl.h | 1 +
src/qxl_driver.c| 2 ++
src/spiceqxl_spice_server.c | 15 +++
4 files changed, 25 insertions(+)
diff --git a/examples/spiceqxl.xorg.conf.example
b/examples/spiceqxl.xorg.conf.example
index d15f7f2..a82c2be 100644
--- a/examples/spiceqxl.xorg.conf.example
+++ b/examples/spiceqxl.xorg.conf.example
@@ -51,6 +51,13 @@ Section Device
# defaults to filter.
#Option SpiceStreamingVideo
+# Set video codecs to use. Provide a semicolon list of
+# codecs, in preference order. Each codec requires an encoder
+# which can be one of spice or gstreamer, and then a codec type,
+# for instance mjpeg or vp8. The default is spice:mjpeg,
+# which uses the builtin mjpeg encoder.
+#Option SpiceVideoCodecs
+
# Set zlib glz wan compression. Options are auto, never, always.
# defaults to auto.
#Option SpiceZlibGlzWanCompression
diff --git a/src/qxl.h b/src/qxl.h
index ff55604..5cc8d05 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -158,6 +158,7 @@ enum {
OPTION_SURFACE_BUFFER_SIZE,
OPTION_COMMAND_BUFFER_SIZE,
OPTION_SPICE_SMARTCARD_FILE,
+OPTION_SPICE_VIDEO_CODECS,
#endif
OPTION_COUNT,
};
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index ce0a88e..d036dac 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -155,6 +155,8 @@ const OptionInfoRec DefaultOptions[] =
CommandBufferSize,OPTV_INTEGER,
{DEFAULT_COMMAND_BUFFER_SIZE}, FALSE},
{ OPTION_SPICE_SMARTCARD_FILE,
SpiceSmartcardFile, OPTV_STRING,{0}, FALSE},
+{ OPTION_SPICE_VIDEO_CODECS,
+ SpiceVideoCodecs, OPTV_STRING,{0}, FALSE},
#endif
{ -1, NULL, OPTV_NONE, {0}, FALSE }
diff --git a/src/spiceqxl_spice_server.c b/src/spiceqxl_spice_server.c
index 14ee752..2f39561 100644
--- a/src/spiceqxl_spice_server.c
+++ b/src/spiceqxl_spice_server.c
@@ -173,6 +173,9 @@ void xspice_set_spice_server_options(OptionInfoPtr options)
const char *streaming_video =
get_str_option(options, OPTION_SPICE_STREAMING_VIDEO,
XSPICE_STREAMING_VIDEO);
+const char *video_codecs =
+get_str_option(options, OPTION_SPICE_VIDEO_CODECS,
+ XSPICE_VIDEO_CODECS);
int agent_mouse =
get_bool_option(options, OPTION_SPICE_AGENT_MOUSE,
XSPICE_AGENT_MOUSE);
@@ -295,6 +298,18 @@ void xspice_set_spice_server_options(OptionInfoPtr options)
spice_server_set_streaming_video(spice_server, streaming_video_opt);
}
+if (video_codecs) {
+#if SPICE_SERVER_VERSION = 0x000c06 /* 0.12.6 */
+if (spice_server_set_video_codecs(spice_server, video_codecs)) {
+fprintf(stderr, spice: invalid video encoder %s\n, video_codecs);
+exit(1);
+}
+#else
+fprintf(stderr, spice: video_codecs are not available (spice =
0.12.6 required)\n);
+exit(1);
+#endif
+}
+
spice_server_set_agent_mouse
(spice_server, agent_mouse);
spice_server_set_playback_compression
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice 08/13] server: Let the video encoder manage the compressed buffer and avoid copying it.
This way the video encoder is not forced to use malloc()/free().
This also allows more flexibility in how the video encoder manages the buffer
which allows for a zero-copy implementation in both video encoders.
The current implementations also ensure that there is no reallocation of the
VideoBuffer structure.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
This is a new patch which makes it possible to avoid copying the output
buffer in the GStreamer video encoder.
Before:
- Spice was handling the initial output buffer allocation, and was also
freeing it when destroying the stream.
- When calling encode_frame() Spice knew that the buffer was no
longer needed for the previous frame. So it just reused it, saving on
reallocations.
- The video encoder was responsible for reallocating the buffer when
needed. In the mjpeg_encoder case this was actually done by
the jpeg library, and thus out of the hands of the encoder itself.
- The GStreamer pipeline allocates its own output buffer which forced
the video encoder to copy it to the Spice-provided output buffer.
Now:
- The video encoders do all the output buffer allocations /
deallocations and give a pointer to the buffer to the rest of the
Spice code.
- The video encoders could assume that the buffer is no longer used by
the time a new call to encode_frame() is made. But I prefer to
decouple this a bit more. So the Spice code is reponsible for
indicating when it no longer needs a given output buffer. This means
it could hold on to more than one output buffer at a time (for
buffering or whatever).
- The video encoders optimize the case where the previous frame's
output buffer has been freed before the next encode_frame() call.
- The GStreamer video encoder can now pass the (wrapped) GStreamer
output buffer to the Spice code and thus avoid a copy. When Spice
releases the output buffer the corresponding GStreamer buffer it
released.
server/gstreamer_encoder.c | 81 ++
server/mjpeg_encoder.c | 106 +++--
server/red_worker.c| 31 ++---
server/video_encoder.h | 45 ---
4 files changed, 187 insertions(+), 76 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 140261e..f7938ae 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -26,6 +26,8 @@
#include red_common.h
+typedef struct GstVideoBuffer GstVideoBuffer;
+#define VIDEO_BUFFER_T GstVideoBuffer
typedef struct GstEncoder GstEncoder;
#define VIDEO_ENCODER_T GstEncoder
#include video_encoder.h
@@ -44,6 +46,12 @@ typedef struct {
int red_mask;
} SpiceFormatForGStreamer;
+struct GstVideoBuffer {
+VideoBuffer base;
+GstBuffer *gst_buffer;
+gboolean persistent;
+};
+
struct GstEncoder {
VideoEncoder base;
@@ -72,6 +80,9 @@ struct GstEncoder {
GstElement *gstenc;
GstAppSink *appsink;
+/* The default video buffer */
+GstVideoBuffer *default_buffer;
+
/* The frame counter for GStreamer buffers */
uint32_t frame;
@@ -83,6 +94,35 @@ struct GstEncoder {
};
+/* -- The GstVideoBuffer implementation -- */
+
+static inline GstVideoBuffer* gst_video_buffer_ref(GstVideoBuffer *buffer)
+{
+buffer-base.ref_count++;
+return buffer;
+}
+
+static void gst_video_buffer_unref(GstVideoBuffer *buffer)
+{
+if (--buffer-base.ref_count == 0) {
+gst_buffer_unref(buffer-gst_buffer);
+if (!buffer-persistent) {
+free(buffer);
+}
+}
+}
+
+static GstVideoBuffer* create_gst_video_buffer(gboolean persistent)
+{
+GstVideoBuffer *buffer = spice_new0(GstVideoBuffer, 1);
+buffer-base.ref = gst_video_buffer_ref;
+buffer-base.unref = gst_video_buffer_unref;
+buffer-persistent = persistent;
+buffer-base.ref_count = persistent ? 0 : 1;
+return buffer;
+}
+
+
/* -- Miscellaneous GstEncoder helpers -- */
static inline double get_mbps(uint64_t bit_rate)
@@ -358,24 +398,24 @@ static int push_raw_frame(GstEncoder *encoder, const
SpiceBitmap *bitmap,
}
/* A helper for gst_encoder_encode_frame(). */
-static int pull_compressed_buffer(GstEncoder *encoder,
- uint8_t **outbuf, size_t *outbuf_size,
- int *data_size)
+static int pull_compressed_buffer(GstEncoder *encoder, GstVideoBuffer **buffer)
{
-GstBuffer *buffer = gst_app_sink_pull_buffer(encoder-appsink);
-if (buffer) {
-int len = GST_BUFFER_SIZE(buffer);
-spice_assert(outbuf outbuf_size);
-if (!*outbuf || *outbuf_size len) {
-*outbuf = spice_realloc(*outbuf, len);
-*outbuf_size = len;
-}
-/* TODO Try to avoid this copy by changing the GstBuffer handling */
-memcpy(*outbuf, GST_BUFFER_DATA(buffer
[Spice-devel] [PATCH spice 13/13] server: Automatically adapt the GStreamer video encoder bit rate to the network conditions.
The video encoder uses the client reports and/or notifications of server frame
drops as its feedback mechanisms.
It uses these to figure out the lowest bit rate that causes negative feedback,
and the highest bit rate that allows a return to positive feedbacks. It then
works to narrow this range and settles on the lower end once the spread has
gone below a given threshold.
All the while it monitors the effective bit rate to ensure the target bit rate
does not grow significantly beyond what the GStreamer encoder will produce.
As soon as the network feedback indicates a significant degradation the bit
rate is lowered to minimize the risk of long freezes.
It also relies on the existing shaping of the GStreamer output bit rate to
minimize the pipeline reconfigurations.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
A word about the effective bit rate monitoring: when the video switches
from high motion scenes to a mostly static talking head the GStreamer
encoder may decide to save bits for later resulting in an effective bit
rate that is half or less of its target. As a result all the network
indicators will turn green, causing the bit rate control code to think
the target can safely be increased. But increasing the target has no
impact on the bit rate of the GStreamer output. So in such a situation
it's possible to increase the target from 4Mbps to 20Mbps while the
effective bit rate remains stuck at 2Mbps. But of course everything
crumbles when the next action scene starts: then the GStreamer encoder
will make full use of the 20Mbps, causing entwork congestion and the bit
rate control algorithm will lose time halving the target bit rate
multiple times before reaching values more in line with what the actual
network capacity. Checking that the effective bit rate corresponds to
the target before any increase avoids this issue.
The feedback mechanism mostly uses the video margin in the client stream
reports. When all is good these are consistently close to the maximum
value. So this forms a sort of buffer: when it's half empty or more the
bit rate really must be reduced.
Regarding reacting to network congestion, there is always a lag between
the congestion occuring and the video encoder being made aware of it,
and then from there to the video encoder's actions having an effect. So
it's important to react as quickly as possible so the network congestion
does not escalate to dropped frames.
- This means algorithms that operate on their own schedule, like every
20 frames, are proscribed.
- Also server-side frame drops are systematically preceded by
client-side frame drops. So server-side frame drops must be acted on
immediately.
server/gstreamer_encoder.c | 396 ++---
1 file changed, 374 insertions(+), 22 deletions(-)
diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
index 4089ead..a46af7b 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -67,6 +67,12 @@ typedef struct {
uint32_t size;
} GstFrameInformation;
+enum GstBitRateStatus {
+GSTE_BITRATE_DECREASING,
+GSTE_BITRATE_INCREASING,
+GSTE_BITRATE_STABLE,
+};
+
struct GstEncoder {
VideoEncoder base;
@@ -106,6 +112,12 @@ struct GstEncoder {
/* The frame counter for GStreamer buffers */
uint32_t frame;
+/* The GStreamer bit rate. */
+uint64_t video_bit_rate;
+
+/* Don't bother changing the GStreamer bit rate if close enough. */
+# define GSTE_VIDEO_BITRATE_MARGIN 0.05
+
/* -- Encoded frame statistics -- */
@@ -140,7 +152,7 @@ struct GstEncoder {
/* -- Encoder bit rate control --
*
- * GStreamer encoders don't follow the specified bit rate very
+ * GStreamer encoders don't follow the specified video_bit_rate very
* closely. These fields are used to ensure we don't exceed the desired
* stream bit rate, regardless of the GStreamer encoder's output.
*/
@@ -148,7 +160,7 @@ struct GstEncoder {
/* The bit rate target for the outgoing network stream. (bits per second)
*/
uint64_t bit_rate;
-/* The minimum bit rate */
+/* The minimum bit rate / bit rate increment. */
# define GSTE_MIN_BITRATE (128 * 1024)
/* The bit rate control is performed using a virtual buffer to allow short
@@ -171,6 +183,89 @@ struct GstEncoder {
/* How big of a margin to take to cover for latency jitter. */
# define GSTE_LATENCY_MARGIN 0.1
+
+
+/* -- Network bit rate control --
+ *
+ * State information for figuring out the optimal bit rate for the current
+ * network conditions.
+ */
+
+/* The mm_time of the last bit rate change. */
+uint32_t last_change;
+
+/* How much to reduce the bit rate in case of network congestion. */
+# define GSTE_BITRATE_CUT 2
+# define GSTE_BITRATE_REDUCE (4.0 / 3.0)
+
+/* Never increase the bit rate
[Spice-devel] [spice] server: spice_debug() messages don't need a trailing '\n'.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
server/red_dispatcher.c | 2 +-
server/reds.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index fb8917a..88980ce 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -445,7 +445,7 @@ void
red_dispatcher_client_monitors_config(VDAgentMonitorsConfig *monitors_confi
!now-qxl-st-qif-client_monitors_config(now-qxl,
monitors_config)) {
/* this is a normal condition, some qemu devices might not
implement it */
-spice_debug(QXLInterface::client_monitors_config failed\n);
+spice_debug(QXLInterface::client_monitors_config failed);
}
now = now-next;
}
diff --git a/server/reds.c b/server/reds.c
index d5f3d3a..552ddd8 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1003,11 +1003,11 @@ static void reds_on_main_agent_monitors_config(
msg_header = (VDAgentMessage *)cmc-buffer;
if (sizeof(VDAgentMessage) cmc-buffer_size ||
msg_header-size cmc-buffer_size - sizeof(VDAgentMessage)) {
-spice_debug(not enough data yet. %d\n, cmc-buffer_size);
+spice_debug(not enough data yet. %d, cmc-buffer_size);
return;
}
monitors_config = (VDAgentMonitorsConfig *)(cmc-buffer +
sizeof(*msg_header));
-spice_debug(%s: %d\n, __func__, monitors_config-num_of_monitors);
+spice_debug(%s: %d, __func__, monitors_config-num_of_monitors);
red_dispatcher_client_monitors_config(monitors_config);
reds_client_monitors_config_cleanup();
}
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice 1/2] server: Don't reset the latency before showing it in the invalid net test error message.
Signed-off-by: Francois Gouget fgou...@codeweavers.com
---
server/main_channel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/server/main_channel.c b/server/main_channel.c
index e7df3f9..f1b38af 100644
--- a/server/main_channel.c
+++ b/server/main_channel.c
@@ -969,11 +969,11 @@ static int main_channel_handle_parsed(RedChannelClient
*rcc, uint32_t size, uint
mcc-net_test_id = 0;
if (roundtrip = mcc-latency) {
// probably high load on client or server result with
incorrect values
+spice_printerr(net test: invalid values, latency % PRIu64
+roundtrip % PRIu64 . assuming high
+bandwidth, mcc-latency, roundtrip);
mcc-latency = 0;
mcc-net_test_stage = NET_TEST_STAGE_INVALID;
-spice_printerr(net test: invalid values, latency % PRIu64
-roundtrip % PRIu64 . assuming high
- bandwidth, mcc-latency, roundtrip);
red_channel_client_start_connectivity_monitoring(mcc-base,
CLIENT_CONNECTIVITY_TIMEOUT);
break;
--
2.1.4
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice 2/2] server: Weakly try to get a better latency value for the bandwidth test.
Signed-off-by: Francois Gouget fgou...@codeweavers.com --- NET_TEST_WARMUP_BYTES is 0 so the warmup ping is the same as the one we use to measure the latency. Even if it was not, the actual latency would be the MIN() of both anyway so we might as well use both roundtrip times to ward off latency jitter a bit. server/main_channel.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/main_channel.c b/server/main_channel.c index f1b38af..0df1751 100644 --- a/server/main_channel.c +++ b/server/main_channel.c @@ -959,11 +959,12 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint case NET_TEST_STAGE_WARMUP: mcc-net_test_id++; mcc-net_test_stage = NET_TEST_STAGE_LATENCY; +mcc-latency = roundtrip; break; case NET_TEST_STAGE_LATENCY: mcc-net_test_id++; mcc-net_test_stage = NET_TEST_STAGE_RATE; -mcc-latency = roundtrip; +mcc-latency = MIN(mcc-latency, roundtrip); break; case NET_TEST_STAGE_RATE: mcc-net_test_id = 0; -- 2.1.4 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v6 08/26] server: Add VP8 support to the GStreamer encoder
On Thu, 22 Oct 2015, Christophe Fergeau wrote:
> On Wed, Oct 14, 2015 at 05:32:03PM +0200, Francois Gouget wrote:
> > The Spice server administrator can specify the preferred encoder and
> > codec preferences to optimize for CPU or bandwidth usage. Preferences
> > are described in a semi-colon separated list of encoder:codec pairs.
> > For instance 'gstreamer:vp8;spice:mjpeg' to pick first the GStreamer
> > VP8 video encoder and the original MJPEG video encoder as a fallback.
> > The server has a default preference list which can also be selected by
> > specifying 'auto' as the preferences list.
>
> Note: All this paragraph describes a very different thing than what the
> shortlog says "server: Add VP8 support to the GStreamer encoder". Imo
> they are 2 distinct things, the addition of
> spice_server_set_video_codecs() and the addition of vp8 support to the
> gstreamer encoder, ie they should be (at least) 2 different commits. It
> might even make sense to move the client capability checks to a 3rd
> commit.
These are technically independent but conceptually they all depend on
each other:
* Adding VP8 support without a way to request using VP8 instead of
MJPEG would be equivalent to adding dead code. Furthermore if
compiling in GStreamer support forced use of VP8 then that server
would be incompatible with most clients so checking the client caps
is really necessary.
* Committing a way to pick the encoder and codec when the only
codec is MJPEG does not make sense either. This could degenerate to
code that only lets the administrator pick the encoder (i.e. builting
or gstreamer), but the next patch would have to add support for the
codec and would essentially be just as big.
* Committing the CAPS checking code first could work, though it would
be somewhat pointless and would not even shave a couple of lines off
this one.
> When adding use of SPICE_DISPLAY_CAP_MULTI_CODEC , I would raise the
> spice-protocol requirement in configure.ac to 0.12.11 (and do a
> post-release bump in spice-protocol git).
I have bumped the requirement to 0.12.11 in configure.ac but I believe
the post-release bump in spice-protocol git has to be left to you.
> > /* Start playing */
> > spice_debug("setting state to PLAYING");
> > @@ -225,6 +274,12 @@ static gboolean construct_pipeline(SpiceGstEncoder
> > *encoder,
> > /* A helper for gst_encoder_encode_frame(). */
> > static void reconfigure_pipeline(SpiceGstEncoder *encoder)
> > {
> > +if (encoder->base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8) {
> > +/* vp8enc gets confused if we try to reconfigure the pipeline */
> > +reset_pipeline(encoder);
> > +return;
> > +}
>
> Any bug reference for this one? Just add it to the comment if there is
> such a bug, if not, that's fine.
vp8enc ignores our attempt to change the appsrc caps so that when it
gets a buffer of the wrong size we get stuck. Given that VP8 performs
inter-frame compression I cannot totally blame it for not liking the
video format to change mid-stream. So while I'd prefer it if it could
deal with this case, I'm not sure this warrants a vp8enc bug report. In
any case this requires a workaround for now. I tried to make the comment
more explicit.
Here are the corresponding log traces:
(/usr/bin/Xorg:26752): Spice-Debug **:
gstreamer_encoder.c:415:gst_encoder_encode_frame: video format change: width 0
-> 640, height 0 -> 360, format 0 -> 9
[...]
(/usr/bin/Xorg:26752): Spice-Debug **:
gstreamer_encoder.c:415:gst_encoder_encode_frame: video format change: width
640 -> 764, height 360 -> 448, format 9 -> 9
[...]
0:00:00.343781738 26752 0x7f9768544050 DEBUG basetransform
gstbasetransform.c:624:gst_base_transform_transform_size: asked to
transform size 1369088 for caps video/x-raw-rgb, bpp=(int)32, depth=(int)24,
width=(int)640, height=(int)360, endianness=(int)4321, red_mask=(int)65280,
green_mask=(int)16711680, blue_mask=(int)-16777216, framerate=(fraction)30/1 to
size for caps video/x-raw-yuv, format=(fourcc)I420, width=(int)640,
height=(int)360, framerate=(fraction)30/1 in direction SINK
** (Xorg:26752): WARNING **: ffmpegcsp0: size 1369088 is not a multiple of unit
size 921600
See also:
http://lists.freedesktop.org/archives/spice-devel/2015-August/021274.html
> > @@ -1349,10 +1349,12 @@ static void mjpeg_encoder_get_stats(VideoEncoder
> > *video_encoder,
> > stats->avg_quality = (double)encoder->avg_quality /
> > encoder->num_frames;
> > }
> >
> > -VideoEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
> > +VideoEncoder *mjpeg_encoder_new(SpiceVideoCodecType codec_type,
> > +uint64_t start
Re: [Spice-devel] [PATCH v6 16/26] server: Give up after a while if GStreamer cannot handle the video
On Thu, 22 Oct 2015, Christophe Fergeau wrote:
[...]
> > @@ -1136,6 +1157,13 @@ static int gst_encoder_encode_frame(VideoEncoder
> > *video_encoder,
> > if (rc == VIDEO_ENCODER_FRAME_ENCODE_DONE) {
> > rc = pull_compressed_buffer(encoder, video_buffer);
> > #ifdef DO_ZERO_COPY
> > +if (rc != VIDEO_ENCODER_FRAME_ENCODE_DONE) {
> > +/* The input buffer will be stuck in the pipeline, preventing
> > later
> > + * ones from being processed. So reset the pipeline.
> > + */
> > +reset_pipeline(encoder);
> > +encoder->errors++;
>
> Maybe increase the errors in pull_compressed_buffer() ?
All the encoding errors eventually bubble up to
gst_encoder_encode_frame() which is where they are caught, tallied and
dealt with. So I think it makes more sense to keep that line here.
--
Francois Gouget <fgou...@codeweavers.com>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v6 04/26] server: Hide the MJPEG encoder internals from red_worker.c
On Wed, 21 Oct 2015, Christophe Fergeau wrote: > ACK. > > Since the changes up to now are useful cleanups regardless of the > addition of gstreamer, I suggest we land the patches up to this one > right now, hopefully this will save some rebasing pain (haven't looked > at rest of the patches yet, I'll do that now). Thanks! There's a couple others like this further down the patch series for spice-gtk: * [PATCH v6 20/26] spice-gtk: Prefix the configure audio GStreamer variables with GSTAUDIO_ http://lists.freedesktop.org/archives/spice-devel/2015-October/022402.html * [PATCH v6 21/26] spice-gtk: Enable adding alternative video decoders This one actually received some comments and I split it in two and sent the updated patches as attachments to my reply. The first part, p21a.diff, is pretty short and would qualify as being independent from the GStreamer rework. http://lists.freedesktop.org/archives/spice-devel/2015-October/022472.html http://lists.freedesktop.org/archives/spice-devel/attachments/20151016/d2e54112/attachment.diff But sadly I have not gotten feedback on the modified patches yet so I don't know if they are really fit to go. -- Francois Gouget <fgou...@codeweavers.com> ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [client 07/11] build-sys: Allow simultaneous support for Pulse and GStreamer audio
On Fri, 6 Nov 2015, Christophe Fergeau wrote: [...] > > -AC_ARG_WITH([audio], > > - AS_HELP_STRING([--with-audio=@<:@gstreamer/pulse/auto/no@:>@], [Select > > audio backend @<:@default=auto@:>@]), > > +AC_ARG_ENABLE([pulse], > > + AS_HELP_STRING([--enable-pulse=@<:@yes/auto/no@:>@], [Enable the > > PulseAudio backend @<:@default=auto@:>@]), > > Imo it would be a less disruptive change if we changed '--with-audio=auto' to > enable both GStreamer and PulseAudio if the needed .pc files are > available. Removing --with-audio and replacing it with > --enable-pulseaudio/--enable-gstreamer means anyone using --with-audio > will need to update its build scripts. The drawback of --with-audio=auto is that it makes it impossible to require having support for both PulseAudio and GStreamer. That is unlike './configure --enable-pulse --enable-gstaudio' it will not print an error if one of them is not available. (and something like --with-audio=pulse,gstreamer feels wrong and would be very non standard) Would keeping --with-audio as a temporary frontend for the two enable options be ok? It could print a warning to remind developers it's deprecated? -- Francois Gouget <fgou...@codeweavers.com> ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [common 03/11] build-sys: Add SPICE_CHECK_GSTREAMER_ELEMENTS()
On Wed, 4 Nov 2015, Christophe Fergeau wrote:
[...]
> > +AC_DEFUN([SPICE_CHECK_GSTREAMER_ELEMENTS], [
> > +AS_IF([test "x$1" != x],
> > + [missing=""
> > + for element in $3
> > + do
> > + AS_VAR_PUSHDEF([cache_var],[spice_cv_prog_${1}_${element}])dnl
> > + AC_CACHE_CHECK([for the $element GStreamer element], cache_var,
> > + [found=no
> > + "$1" $element >/dev/null 2>/dev/null &&
> > found=yes
> > + eval "cache_var=$found"])
> > + AS_VAR_COPY(res, cache_var)
>
> On el6, this needs a fallback definition:
> +m4_ifndef([AS_VAR_COPY],
> + [m4_define([AS_VAR_COPY],
> + [AS_LITERAL_IF([$1[]$2], [$1=$$2], [eval $1=\$$2])])])
>
> I can add that before pushing. Looks good otherwise (though my m4-foo is
> weaker than what you are doing in this patch ;)
Thanks. That would be nice.
--
Francois Gouget <fgou...@codeweavers.com>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice 10/11] build-sys: Use AC_MSG_NOTICE()
On Tue, 3 Nov 2015, Francois Gouget wrote:
> Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
> ---
> configure.ac | 15 ---
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> This patch is independent from all other patches in this series.
Is anything blocking this patch?
> diff --git a/configure.ac b/configure.ac
> index dfb967b..ad76467 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -289,7 +289,7 @@ docs/manual/Makefile
> ])
>
> dnl
> ==
> -echo "
> +AC_MSG_NOTICE([
>
> Spice $VERSION
> ==
> @@ -300,23 +300,16 @@ echo "
> python: ${PYTHON}
>
> OpenGL: ${enable_opengl}
> -
> LZ4 support: ${enable_lz4}
> -
> Smartcard:${have_smartcard}
> -
> SASL support: ${enable_sasl}
> -
> Automated tests: ${enable_automated_tests}
> -
> Manual: ${have_asciidoc}
> -"
> +
> +Now type 'make' to build $PACKAGE
> +])
>
> if test x"$arch_warn" != x; then
> AC_MSG_WARN([$arch_warn])
> echo ""
> fi
> -
> -echo \
> -"Now type 'make' to build $PACKAGE
> -"
> --
> 2.6.1
--
Francois Gouget <fgou...@codeweavers.com>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [client 04/11] build-sys: Remove some dead configure.ac DBus code
On Tue, 3 Nov 2015, Francois Gouget wrote: > Signed-off-by: Francois Gouget <fgou...@codeweavers.com> > --- > configure.ac | 4 > 1 file changed, 4 deletions(-) > > This patch does not depend on any other patch in this series. Anything blocking this? (there must be a curse...) > diff --git a/configure.ac b/configure.ac > index 5790a37..7033cbb 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -671,10 +671,6 @@ have_dbus=no > if test "x$enable_dbus" != "xno"; then >AC_DEFINE([USE_GDBUS], [1], [Define if supporting gdbus]) >have_dbus=yes > - > - if test "x$enable_dbus" = "xyes" && test "x$have_dbus" = "xno"; then > -AC_MSG_ERROR([D-Bus support explicitly requested, but some required > packages are not available]) > - fi > fi > > SPICE_CHECK_LZ4 > -- > 2.6.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > -- Francois Gouget <fgou...@codeweavers.com> ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [common 01/11] build-sys: Add the SPICE_WARNING() and SPICE_PRINT_MESSAGES m4 macros
On Tue, 3 Nov 2015, Francois Gouget wrote: > A call to SPICE_WARNING() anywhere in the configure file results in the > warning being printed at the end of the configure run where it will be > be visible. This makes it possible to keep the SPICE_WARNING() calls > together with the related feature checks instead of having to put a > separate AC_MSG_WARN() call near the end. This version has the AS_VAR_APPEND() fallback. Is it ok? > > Signed-off-by: Francois Gouget <fgou...@codeweavers.com> > --- > m4/spice-deps.m4 | 21 + > 1 file changed, 21 insertions(+) > > For reference, see also: > http://lists.freedesktop.org/archives/spice-devel/2015-November/023009.html > > diff --git a/m4/spice-deps.m4 b/m4/spice-deps.m4 > index 0f90cec..57d5b78 100644 > --- a/m4/spice-deps.m4 > +++ b/m4/spice-deps.m4 > @@ -1,3 +1,24 @@ > +# For autoconf < 2.63 > +m4_ifndef([AS_VAR_APPEND], > + AC_DEFUN([AS_VAR_APPEND], $1=$$1$2)) > + > +# SPICE_WARNING(warning) > +# SPICE_PRINT_MESSAGES > +# -- > +# Collect warnings and print them at the end so they are clearly visible. > +# - > +AC_DEFUN([SPICE_WARNING],AS_VAR_APPEND([spice_warnings],["|$1"])) > +AC_DEFUN([SPICE_PRINT_MESSAGES],[ > +ac_save_IFS="$IFS" > +IFS="|" > +for msg in $spice_warnings; do > +IFS="$ac_save_IFS" > +AS_VAR_IF([msg],[],,[AC_MSG_WARN([$msg]); echo >&2]) > +done > +IFS="$ac_save_IFS" > +]) > + > + > # SPICE_CHECK_SYSDEPS() > # - > # Checks for header files and library functions needed by spice-common. > -- > 2.6.1 -- Francois Gouget <fgou...@codeweavers.com> ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice v3] server: Provide a framerate estimate based on the initial frames
This way the video encoder can actually count on a real estimate when
it is initializing.
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
server/display-channel.h | 1 +
server/red_worker.c | 18 ++
server/stream.h | 1 +
3 files changed, 16 insertions(+), 4 deletions(-)
I sent this patch in June:
http://lists.freedesktop.org/archives/spice-devel/2015-June/thread.html
Then again a week ago:
http://lists.freedesktop.org/archives/spice-devel/2015-November/thread.html
But never got any feedback on it. What's up???
diff --git a/server/display-channel.h b/server/display-channel.h
index c3dcc29..24f9da5 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -152,6 +152,7 @@ struct Drawable {
Ring glz_ring;
red_time_t creation_time;
+red_time_t first_frame_time;
int frames_count;
int gradual_frames_count;
int last_gradual_frame;
diff --git a/server/red_worker.c b/server/red_worker.c
index 9673288..b141a1e 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -984,6 +984,7 @@ static void
display_stream_trace_add_drawable(DisplayChannel *display, Drawable
trace = >items_trace[display->next_item_trace++ &
ITEMS_TRACE_MASK];
trace->time = item->creation_time;
+trace->first_frame_time = item->first_frame_time;
trace->frames_count = item->frames_count;
trace->gradual_frames_count = item->gradual_frames_count;
trace->last_gradual_frame = item->last_gradual_frame;
@@ -1772,7 +1773,12 @@ static void display_channel_create_stream(DisplayChannel
*display, Drawable *dra
SpiceBitmap *bitmap = >red_drawable->u.copy.src_bitmap->u.bitmap;
stream->top_down = !!(bitmap->flags & SPICE_BITMAP_FLAGS_TOP_DOWN);
drawable->stream = stream;
-stream->input_fps = MAX_FPS;
+/* Provide an fps estimate the video encoder can use when initializing
+ * based on the frames that lead to the creation of the stream. Round to
+ * the nearest integer, for instance 24 for 23.976.
+ */
+uint64_t duration = drawable->creation_time - drawable->first_frame_time;
+stream->input_fps = ((uint64_t)drawable->frames_count * 1000 * 1000 * 1000
+ duration / 2) / duration;
stream->num_input_frames = 0;
stream->input_fps_start_time = drawable->creation_time;
display->streams_size_total += stream->width * stream->height;
@@ -1780,10 +1786,10 @@ static void
display_channel_create_stream(DisplayChannel *display, Drawable *dra
FOREACH_DCC(display, dcc_ring_item, next, dcc) {
dcc_create_stream(dcc, stream);
}
-spice_debug("stream %d %dx%d (%d, %d) (%d, %d)",
+spice_debug("stream %d %dx%d (%d, %d) (%d, %d) %u fps",
(int)(stream - display->streams_buf), stream->width,
stream->height, stream->dest_area.left, stream->dest_area.top,
-stream->dest_area.right, stream->dest_area.bottom);
+stream->dest_area.right, stream->dest_area.bottom,
stream->input_fps);
return;
}
@@ -2009,11 +2015,13 @@ static int is_stream_start(Drawable *drawable)
// returns whether a stream was created
static int display_channel_stream_add_frame(DisplayChannel *display,
Drawable *frame_drawable,
+red_time_t first_frame_time,
int frames_count,
int gradual_frames_count,
int last_gradual_frame)
{
update_copy_graduality(display, frame_drawable);
+frame_drawable->first_frame_time = first_frame_time;
frame_drawable->frames_count = frames_count + 1;
frame_drawable->gradual_frames_count = gradual_frames_count;
@@ -2074,6 +2082,7 @@ static void
display_channel_stream_maintenance(DisplayChannel *display,
FALSE);
if (is_next_frame != STREAM_FRAME_NONE) {
display_channel_stream_add_frame(display, candidate,
+ prev->first_frame_time,
prev->frames_count,
prev->gradual_frames_count,
prev->last_gradual_frame);
@@ -2235,6 +2244,7 @@ static void red_use_stream_trace(DisplayChannel *display,
Drawable *drawable)
>dest_area, trace->time, NULL,
FALSE) !=
STREAM_FRAME_NONE) {
if (display_channel_stream_add_frame(display, drawable,
+ trace->first_frame_time,
trace->frames_
[Spice-devel] [spice] build-sys: Use AC_MSG_NOTICE()
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
configure.ac | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
I submitted this very simple patch twice before as part of patch series
while noting it was independent of the rest of the series:
http://lists.freedesktop.org/archives/spice-devel/2015-October/022911.html
http://lists.freedesktop.org/archives/spice-devel/2015-November/023057.html
Asked if something was wrong with it there:
http://lists.freedesktop.org/archives/spice-devel/2015-November/023368.html
One of the series it was in sort of got acked there:
http://lists.freedesktop.org/archives/spice-devel/2015-November/023144.html
Let's see if it fares any better when presented all on its own.
diff --git a/configure.ac b/configure.ac
index dfb967b..ad76467 100644
--- a/configure.ac
+++ b/configure.ac
@@ -289,7 +289,7 @@ docs/manual/Makefile
])
dnl ==
-echo "
+AC_MSG_NOTICE([
Spice $VERSION
==
@@ -300,23 +300,16 @@ echo "
python: ${PYTHON}
OpenGL: ${enable_opengl}
-
LZ4 support: ${enable_lz4}
-
Smartcard:${have_smartcard}
-
SASL support: ${enable_sasl}
-
Automated tests: ${enable_automated_tests}
-
Manual: ${have_asciidoc}
-"
+
+Now type 'make' to build $PACKAGE
+])
if test x"$arch_warn" != x; then
AC_MSG_WARN([$arch_warn])
echo ""
fi
-
-echo \
-"Now type 'make' to build $PACKAGE
-"
--
2.6.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice] server: Don't check the 'this' mjpeg_encoder pointer
mjpeg_encoder_get_stats() was the only function to check it.
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
server/mjpeg_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index c8253a7..84a0078 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -1333,7 +1333,7 @@ uint64_t mjpeg_encoder_get_bit_rate(MJpegEncoder *encoder)
void mjpeg_encoder_get_stats(MJpegEncoder *encoder, MJpegEncoderStats *stats)
{
-spice_assert(encoder != NULL && stats != NULL);
+spice_assert(stats != NULL);
stats->starting_bit_rate = encoder->starting_bit_rate;
stats->cur_bit_rate = mjpeg_encoder_get_bit_rate(encoder);
stats->avg_quality = (double)encoder->avg_quality / encoder->num_frames;
--
2.6.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 1/3] vdagent: Group the client and server functions together
This will simplify selectively disabling the server-side code.
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
src/udscs.c | 478 ++--
src/udscs.h | 128 +---
2 files changed, 314 insertions(+), 292 deletions(-)
I separated the reordering from the addition of the #ifdefs as suggested
by Victor Toso:
http://lists.freedesktop.org/archives/spice-devel/2015-November/023457.html
diff --git a/src/udscs.c b/src/udscs.c
index 288aca2..732db33 100644
--- a/src/udscs.c
+++ b/src/udscs.c
@@ -66,86 +66,6 @@ struct udscs_connection {
struct udscs_connection *prev;
};
-struct udscs_server {
-int fd;
-const char * const *type_to_string;
-int no_types;
-int debug;
-struct udscs_connection connections_head;
-udscs_connect_callback connect_callback;
-udscs_read_callback read_callback;
-udscs_disconnect_callback disconnect_callback;
-};
-
-static void udscs_do_write(struct udscs_connection **connp);
-static void udscs_do_read(struct udscs_connection **connp);
-
-
-struct udscs_server *udscs_create_server(const char *socketname,
-udscs_connect_callback connect_callback,
-udscs_read_callback read_callback,
-udscs_disconnect_callback disconnect_callback,
-const char * const type_to_string[], int no_types, int debug)
-{
-int c;
-struct sockaddr_un address;
-struct udscs_server *server;
-
-server = calloc(1, sizeof(*server));
-if (!server)
-return NULL;
-
-server->type_to_string = type_to_string;
-server->no_types = no_types;
-server->debug = debug;
-
-server->fd = socket(PF_UNIX, SOCK_STREAM, 0);
-if (server->fd == -1) {
-syslog(LOG_ERR, "creating unix domain socket: %m");
-free(server);
-return NULL;
-}
-
-address.sun_family = AF_UNIX;
-snprintf(address.sun_path, sizeof(address.sun_path), "%s", socketname);
-c = bind(server->fd, (struct sockaddr *), sizeof(address));
-if (c != 0) {
-syslog(LOG_ERR, "bind %s: %m", socketname);
-free(server);
-return NULL;
-}
-
-c = listen(server->fd, 5);
-if (c != 0) {
-syslog(LOG_ERR, "listen: %m");
-free(server);
-return NULL;
-}
-
-server->connect_callback = connect_callback;
-server->read_callback = read_callback;
-server->disconnect_callback = disconnect_callback;
-
-return server;
-}
-
-void udscs_destroy_server(struct udscs_server *server)
-{
-struct udscs_connection *conn, *next_conn;
-
-if (!server)
-return;
-
-conn = server->connections_head.next;
-while (conn) {
-next_conn = conn->next;
-udscs_destroy_connection();
-conn = next_conn;
-}
-close(server->fd);
-free(server);
-}
-
struct udscs_connection *udscs_connect(const char *socketname,
udscs_read_callback read_callback,
udscs_disconnect_callback disconnect_callback,
@@ -225,131 +145,17 @@ void udscs_destroy_connection(struct udscs_connection
**connp)
*connp = NULL;
}
-struct ucred udscs_get_peer_cred(struct udscs_connection *conn)
-{
-return conn->peer_cred;
-}
-
-int udscs_server_fill_fds(struct udscs_server *server, fd_set *readfds,
-fd_set *writefds)
+void udscs_set_user_data(struct udscs_connection *conn, void *data)
{
-struct udscs_connection *conn;
-int nfds = server->fd + 1;
-
-if (!server)
-return -1;
-
-FD_SET(server->fd, readfds);
-
-conn = server->connections_head.next;
-while (conn) {
-int conn_nfds = udscs_client_fill_fds(conn, readfds, writefds);
-if (conn_nfds > nfds)
-nfds = conn_nfds;
-
-conn = conn->next;
-}
-
-return nfds;
+conn->user_data = data;
}
-int udscs_client_fill_fds(struct udscs_connection *conn, fd_set *readfds,
-fd_set *writefds)
+void *udscs_get_user_data(struct udscs_connection *conn)
{
if (!conn)
-return -1;
-
-FD_SET(conn->fd, readfds);
-if (conn->write_buf)
-FD_SET(conn->fd, writefds);
-
-return conn->fd + 1;
-}
-
-static void udscs_server_accept(struct udscs_server *server) {
-struct udscs_connection *new_conn, *conn;
-struct sockaddr_un address;
-socklen_t length = sizeof(address);
-int r, fd;
-
-fd = accept(server->fd, (struct sockaddr *), );
-if (fd == -1) {
-if (errno == EINTR)
-return;
-syslog(LOG_ERR, "accept: %m");
-return;
-}
-
-new_conn = calloc(1, sizeof(*conn));
-if (!new_conn) {
-syslog(LOG_ERR, "out of memory, disconnecting new client");
-close(fd);
-return;
-}
-
-new_conn->fd = fd;
-new_conn->type_to_string = server->type_to_string;
-new_conn->no_types = server->no_types;
-new_conn->debug
[Spice-devel] [PATCH v2 3/3] vdagent: Disable the server-side udscs code for vdagent
vdagent only needs the client-side API. Signed-off-by: Francois Gouget <fgou...@codeweavers.com> --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 8c55b43..7def506 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4,7 +4,7 @@ NULL = bin_PROGRAMS = src/spice-vdagent sbin_PROGRAMS = src/spice-vdagentd -src_spice_vdagent_CFLAGS = $(X_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) $(ALSA_CFLAGS) +src_spice_vdagent_CFLAGS = $(X_CFLAGS) $(SPICE_CFLAGS) $(GLIB2_CFLAGS) $(ALSA_CFLAGS) -DUDSCS_NO_SERVER src_spice_vdagent_LDADD = $(X_LIBS) $(SPICE_LIBS) $(GLIB2_LIBS) $(ALSA_LIBS) src_spice_vdagent_SOURCES = src/vdagent.c \ src/vdagent-x11.c \ -- 2.6.2 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v2 2/3] vdagent: Allow disabling the server-side udscs support
To do so define UDSCS_NO_SERVER.
This simplifies reuse in client-only scenarios that don't need peer
credential support for instance.
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
src/udscs.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/udscs.c b/src/udscs.c
index 732db33..334d54a 100644
--- a/src/udscs.c
+++ b/src/udscs.c
@@ -46,8 +46,10 @@ struct udscs_connection {
const char * const *type_to_string;
int no_types;
int debug;
-struct ucred peer_cred;
void *user_data;
+#ifndef UDSCS_NO_SERVER
+struct ucred peer_cred;
+#endif
/* Read stuff, single buffer, separate header and data buffer */
int header_read;
@@ -350,6 +352,8 @@ int udscs_client_fill_fds(struct udscs_connection *conn,
fd_set *readfds,
}
+#ifndef UDSCS_NO_SERVER
+
/* -- Server-side implementation -- */
struct udscs_server {
@@ -563,3 +567,5 @@ int udscs_server_for_all_clients(struct udscs_server
*server,
}
return r;
}
+
+#endif
--
2.6.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice] server: Remove the display_channel_attach_stream() prototype
It is unused.
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
server/display-channel.h | 3 ---
1 file changed, 3 deletions(-)
It was added in this commit:
commit 4987df8e67f48cde13d39f4e81909f672ae33e29
Author: Marc-André Lureau <marcandre.lur...@gmail.com>
Date: Tue Nov 10 13:21:28 2015 +
worker: move stream to display channel
Acked-by: Jonathon Jongsma <jjong...@redhat.com>
So unless there was some ulterior motive it should not be there.
diff --git a/server/display-channel.h b/server/display-channel.h
index c3dcc29..a9ae40a 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -358,9 +358,6 @@ typedef struct SurfaceCreateItem {
void display_channel_set_stream_video
(DisplayChannel *display,
int
stream_video);
-void display_channel_attach_stream
(DisplayChannel *display,
- Drawable
*drawable,
- Stream
*stream);
intdisplay_channel_get_streams_timeout
(DisplayChannel *display);
void display_channel_compress_stats_print (const
DisplayChannel *display);
void display_channel_compress_stats_reset
(DisplayChannel *display);
--
2.6.2___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] vdagent: VDP_LAST_PORT is redundant with VDP_END_PORT. Remove it
VDP_LAST_PORT is equal to VDP_SERVER_PORT while VDP_END_PORT (defined
in spice-protocol) is VDP_SERVER_PORT + 1 so the code needs to be
adjusted to account for this difference.
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
src/vdagent-virtio-port.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
Adjusted the commit log as suggested by Christophe Fergeau:
http://lists.freedesktop.org/archives/spice-devel/2015-November/023297.html
diff --git a/src/vdagent-virtio-port.c b/src/vdagent-virtio-port.c
index b04d55b..722f3ba 100644
--- a/src/vdagent-virtio-port.c
+++ b/src/vdagent-virtio-port.c
@@ -31,7 +31,6 @@
#include "vdagent-virtio-port.h"
-#define VDP_LAST_PORT VDP_SERVER_PORT
struct vdagent_virtio_port_buf {
uint8_t *buf;
@@ -63,7 +62,7 @@ struct vdagent_virtio_port {
uint8_t chunk_data[VD_AGENT_MAX_DATA_SIZE];
/* Per chunk port data */
-struct vdagent_virtio_port_chunk_port_data port_data[VDP_LAST_PORT + 1];
+struct vdagent_virtio_port_chunk_port_data port_data[VDP_END_PORT];
/* Writes are stored in a linked list of buffers, with both the header
+ data for a single message in 1 buffer. */
@@ -142,7 +141,7 @@ void vdagent_virtio_port_destroy(struct vdagent_virtio_port
**vportp)
wbuf = next_wbuf;
}
-for (i = 0; i <= VDP_LAST_PORT; i++) {
+for (i = 0; i < VDP_END_PORT; i++) {
free(vport->port_data[i].message_data);
}
@@ -287,7 +286,7 @@ void vdagent_virtio_port_flush(struct vdagent_virtio_port
**vportp)
void vdagent_virtio_port_reset(struct vdagent_virtio_port *vport, int port)
{
-if (port > VDP_LAST_PORT) {
+if (port >= VDP_END_PORT) {
syslog(LOG_ERR, "vdagent_virtio_port_reset port out of range");
return;
}
@@ -427,7 +426,7 @@ static void vdagent_virtio_port_do_read(struct
vdagent_virtio_port **vportp)
vdagent_virtio_port_destroy(vportp);
return;
}
-if (vport->chunk_header.port > VDP_LAST_PORT) {
+if (vport->chunk_header.port >= VDP_END_PORT) {
syslog(LOG_ERR, "chunk port %u out of range",
vport->chunk_header.port);
vdagent_virtio_port_destroy(vportp);
--
2.6.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 1/3] vdagent: Group the client and server functions together
On Fri, 13 Nov 2015, Victor Toso wrote: [...] > PS: Now it seems to be correctly threaded in patchwork, thanks! > https://patchwork.freedesktop.org/series/745/ Is there a reference to Spice's patchwork page somewhere? -- Francois Gouget <fgou...@codeweavers.com> ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice] server: Don't check the 'this' mjpeg_encoder pointer
On Fri, 13 Nov 2015, Frediano Ziglio wrote:
[...]
> > void mjpeg_encoder_get_stats(MJpegEncoder *encoder, MJpegEncoderStats
> > *stats)
> > {
> > -spice_assert(encoder != NULL && stats != NULL);
> > +spice_assert(stats != NULL);
> > stats->starting_bit_rate = encoder->starting_bit_rate;
> > stats->cur_bit_rate = mjpeg_encoder_get_bit_rate(encoder);
> > stats->avg_quality = (double)encoder->avg_quality /
> > encoder->num_frames;
> > --
> > 2.6.2
>
> Personally I think that these kind of checks are not helping that much and
> I would agree. A NULL pointer is a bug but removing the test cause a core
> on modern systems so adding it just slow down the execution and increase code
> size. For the same reason however even the check for stats could be removed
> like many other tests for NULL pointers.
>
> It's quite question of style subject to personal opinions.
Yes, it's hard to know what to do when writing new code these days:
1) Follow surrounding code and thus use spice_assert(), in particular to
document the function's prerequisites.
2) Aknowledge that the server being a library it should not crash
the application (which I'm totally on board with, having had this
problem with libav), and use the spice_return_if_fail() instead of
spice_assert().
3) Still use spice_assert() but only for cases where the function would
have crashed anyway. I guess the advantage is that it makes the cause
of the crash clear without the need for debugging information in the
binary.
4) A mix of spice_assert() (point 3) for cases where Spice has no way
to return an error to the caller and would otherwise crash, and
spice_return_if_fail() (point 2) for other cases.
5) Consider all of this to drag down performance and use none of them.
For mjpeg_encoder_get_stats() specifically, I think in the end I would
settle on either:
* No check: the function is three lines long, it's painfully obvious
neither parameter should be NULL. Plus why single out the NULL
pointer case when a use after free is probably more likely?
* Or a single spice_return_if_fail(stat != NULL) which may be more in
line with current policy (don't return statistics if not given a
place to put them).
--
Francois Gouget <fgou...@codeweavers.com>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice v3] server: Provide a framerate estimate based on the initial frames
On Fri, 13 Nov 2015, Frediano Ziglio wrote:
[...]
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index c3dcc29..24f9da5 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -152,6 +152,7 @@ struct Drawable {
> > Ring glz_ring;
> >
> > red_time_t creation_time;
> > +red_time_t first_frame_time;
> > int frames_count;
> > int gradual_frames_count;
> > int last_gradual_frame;
>
> This here looks weird. ItemTrace should trace/record the frames and have a
> first and count.
> I think however is more of a design problem than something related to your
> patch.
> There are too many fields related to stream in Drawable.
[...]
> is not clear why the time keep updating with new frames.
The video stream detection code is pretty complex and undocumented (and
buggy too). But my understanding is that a video stream is created only
if at least twenty image 'blits' of the same size and type arrive, each
within a maximum time interval from the previous one.
So the code was only keeping track of the current frame's creation time,
and that of the previous one to check for the 'maximum time interval'
condition. But basing a framerate estimate on just one frame interval
would not be very precise. So to get a proper estimate I need to also
keep track of the timestamp for the first frame in the series, so I can
compute an average over the first 20 frames (which represents 0.67s at
30fps so long enough to smooth over scheduling delays). Hence the need
for the first_frame_time field.
Also the code does not keep the twenty frames in memory. My rough
understanding and recollection of it is that as soon as it's done with a
frame it frees it and only keeps the essential data into an ItemTrace
structure. So to preserve the time of the first frame it's necessary to
copy the first_frame_time field from frame to frame, and back and forth
between ItemTrace and Drawable structures.
[...]
> > +/* Provide an fps estimate the video encoder can use when initializing
> > + * based on the frames that lead to the creation of the stream. Round
> > to
> > + * the nearest integer, for instance 24 for 23.976.
> > + */
> > +uint64_t duration = drawable->creation_time -
> > drawable->first_frame_time;
> > +stream->input_fps = ((uint64_t)drawable->frames_count * 1000 * 1000 *
> > 1000 + duration / 2) / duration;
>
> Just to be 100% sure I would check for duration == 0 and limit input_fps to
> MAX_FPS.
Sure. Will do.
> Which kind of test did you do?
Playing video with mplayer, totem and Flash (YouTube), with and without
SpiceDeferredFPS and EnableSurfaces. The fps estimates were always
within 1fps of the actual framerate.
--
Francois Gouget <fgou...@codeweavers.com>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice] server: Don't check the 'this' mjpeg_encoder pointer
On Mon, 16 Nov 2015, Christophe Fergeau wrote: > On Fri, Nov 13, 2015 at 04:14:17PM +0100, Francois Gouget wrote: > > mjpeg_encoder_get_stats() was the only function to check it. > > Could you trigger this assert in a somehow legit case? Or is this just > for consistency? It's for consistency and also as a way to figure out what to do for the GStreamer encoder without having to send the whole series for each iteration. -- Francois Gouget <fgou...@codeweavers.com> ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice v4] server: Provide a framerate estimate based on the initial frames
This way the video encoder can actually count on a real estimate when
it is initializing.
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
server/display-channel.h | 1 +
server/red_worker.c | 22 ++
server/stream.h | 1 +
3 files changed, 20 insertions(+), 4 deletions(-)
Check that duration is not 0 and that we don't exceed MAX_FPS.
diff --git a/server/display-channel.h b/server/display-channel.h
index a9ae40a..23d7a05 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -152,6 +152,7 @@ struct Drawable {
Ring glz_ring;
red_time_t creation_time;
+red_time_t first_frame_time;
int frames_count;
int gradual_frames_count;
int last_gradual_frame;
diff --git a/server/red_worker.c b/server/red_worker.c
index 165e4c0..23834d3 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -984,6 +984,7 @@ static void
display_stream_trace_add_drawable(DisplayChannel *display, Drawable
trace = >items_trace[display->next_item_trace++ &
ITEMS_TRACE_MASK];
trace->time = item->creation_time;
+trace->first_frame_time = item->first_frame_time;
trace->frames_count = item->frames_count;
trace->gradual_frames_count = item->gradual_frames_count;
trace->last_gradual_frame = item->last_gradual_frame;
@@ -1772,7 +1773,16 @@ static void display_channel_create_stream(DisplayChannel
*display, Drawable *dra
SpiceBitmap *bitmap = >red_drawable->u.copy.src_bitmap->u.bitmap;
stream->top_down = !!(bitmap->flags & SPICE_BITMAP_FLAGS_TOP_DOWN);
drawable->stream = stream;
-stream->input_fps = MAX_FPS;
+/* Provide an fps estimate the video encoder can use when initializing
+ * based on the frames that lead to the creation of the stream. Round to
+ * the nearest integer, for instance 24 for 23.976.
+ */
+uint64_t duration = drawable->creation_time - drawable->first_frame_time;
+if (duration > (uint64_t)drawable->frames_count * 1000 * 1000 * 1000 /
MAX_FPS) {
+stream->input_fps = ((uint64_t)drawable->frames_count * 1000 * 1000 *
1000 + duration / 2) / duration;
+} else {
+stream->input_fps = MAX_FPS;
+}
stream->num_input_frames = 0;
stream->input_fps_start_time = drawable->creation_time;
display->streams_size_total += stream->width * stream->height;
@@ -1780,10 +1790,10 @@ static void
display_channel_create_stream(DisplayChannel *display, Drawable *dra
FOREACH_DCC(display, dcc_ring_item, next, dcc) {
dcc_create_stream(dcc, stream);
}
-spice_debug("stream %d %dx%d (%d, %d) (%d, %d)",
+spice_debug("stream %d %dx%d (%d, %d) (%d, %d) %u fps",
(int)(stream - display->streams_buf), stream->width,
stream->height, stream->dest_area.left, stream->dest_area.top,
-stream->dest_area.right, stream->dest_area.bottom);
+stream->dest_area.right, stream->dest_area.bottom,
stream->input_fps);
return;
}
@@ -2009,11 +2019,13 @@ static int is_stream_start(Drawable *drawable)
// returns whether a stream was created
static int display_channel_stream_add_frame(DisplayChannel *display,
Drawable *frame_drawable,
+red_time_t first_frame_time,
int frames_count,
int gradual_frames_count,
int last_gradual_frame)
{
update_copy_graduality(display, frame_drawable);
+frame_drawable->first_frame_time = first_frame_time;
frame_drawable->frames_count = frames_count + 1;
frame_drawable->gradual_frames_count = gradual_frames_count;
@@ -2074,6 +2086,7 @@ static void
display_channel_stream_maintenance(DisplayChannel *display,
FALSE);
if (is_next_frame != STREAM_FRAME_NONE) {
display_channel_stream_add_frame(display, candidate,
+ prev->first_frame_time,
prev->frames_count,
prev->gradual_frames_count,
prev->last_gradual_frame);
@@ -2235,6 +2248,7 @@ static void red_use_stream_trace(DisplayChannel *display,
Drawable *drawable)
>dest_area, trace->time, NULL,
FALSE) !=
STREAM_FRAME_NONE) {
if (display_channel_stream_add_frame(display, drawable,
+ trace->first_frame_time,
trace->frames_count,
[Spice-devel] server: Include stdint.h for int64_t
This fixes a compilation error with gcc 4.4 on RHEL 6. Signed-off-by: Francois Gouget <fgou...@codeweavers.com> --- server/utils.h | 1 + 1 file changed, 1 insertion(+) diff --git a/server/utils.h b/server/utils.h index 1ebc32f..cae03d4 100644 --- a/server/utils.h +++ b/server/utils.h @@ -19,6 +19,7 @@ # define UTILS_H_ #include +#include typedef int64_t red_time_t; -- 2.6.2 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] server: Duplicate typedef definitions are not allowed in C99
This fixes some compilation errors with gcc 4.4.7 on RHEL 6.
Signed-off-by: Francois Gouget <fgou...@codeweavers.com>
---
server/red_channel.h| 1 -
server/red_dispatcher.h | 1 -
server/red_worker.c | 4 ++--
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/server/red_channel.h b/server/red_channel.h
index eda4436..4deecb6 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -128,7 +128,6 @@ typedef struct OutgoingHandler {
/* Red Channel interface */
-typedef struct RedsStream RedsStream;
typedef struct RedChannel RedChannel;
typedef struct RedChannelClient RedChannelClient;
typedef struct RedClient RedClient;
diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
index 02337b8..fef067c 100644
--- a/server/red_dispatcher.h
+++ b/server/red_dispatcher.h
@@ -23,7 +23,6 @@
#include "red_channel.h"
typedef struct RedDispatcher RedDispatcher;
-typedef struct RedChannelClient RedChannelClient;
typedef struct AsyncCommand AsyncCommand;
diff --git a/server/red_worker.c b/server/red_worker.c
index 9673288..165e4c0 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -233,7 +233,7 @@ typedef struct RedSurface {
QXLReleaseInfoExt create, destroy;
} RedSurface;
-typedef struct RedWorker {
+struct RedWorker {
pthread_t thread;
clockid_t clockid;
QXLInstance *qxl;
@@ -290,7 +290,7 @@ typedef struct RedWorker {
int driver_cap_monitors_config;
FILE *record_fd;
-} RedWorker;
+};
typedef enum {
BITMAP_DATA_TYPE_INVALID,
--
2.6.2
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
