Clients that support multiple codecs should advertise the SPICE_DISPLAY_CAP_MULTI_CODEC
capability and one SPICE_DISPLAY_CAP_CODEC_XXX per supported codec.
---
On Fri, 31 Jul 2015, Victor Toso wrote:
[...]
> > +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)
>
> Personally, I would prefer glib-compat.{c,h}. It is easier to track and
> drop stuff when bumping glib version.
I think it's better to check for the needed features so alternative
implementations (potentially non-glib ones) can be used if missing.
> > static gboolean construct_pipeline(GstEncoder *encoder, const SpiceBitmap *bitmap)
> > {
> > + gboolean no_clock = FALSE;
>
> This no_clock will be removed in this series again using
> encoder->base.codec_type -- I would say we could make that happen in
> this patch.
Done.
> > static void reconfigure_pipeline(GstEncoder *encoder)
> > {
> > - if (gst_element_set_state(encoder->pipeline, GST_STATE_PAUSED) == GST_STATE_CHANGE_FAILURE ||
> > - !set_appsrc_caps(encoder) ||
> > - gst_element_set_state(encoder->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE) {
> > + if (encoder->base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8) {
> > + /* vp8enc gets confused if we try to reconfigure the pipeline */
>
> What do you mean by vp8enc getting confused?
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.
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:<ffmpegcsp0> 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
> > +MJpegEncoder *create_mjpeg_encoder(SpiceVideoCodecType codec_type,
> > + uint64_t starting_bit_rate,
> > VideoEncoderRateControlCbs *cbs,
> > void *cbs_opaque)
> > {
> > spice_assert(!cbs || (cbs && cbs->get_roundtrip_ms && cbs->get_source_fps));
> > + if (codec_type != SPICE_VIDEO_CODEC_TYPE_MJPEG) {
> > + return NULL;
> > + }
>
> Shouldn't be spice_assert too? It shouldn't call create_mjpeg_encoder
> with codec_type != SPICE_VIDEO_CODEC_TYPE_MJPEG, should it?
Yes, that would be better. Done.
> > +/* Expected string: encoder:codec;encoder:codec */
> > +static const char* parse_video_codecs(const char *codecs, char **encoder,
> > + char **codec)
> > {
> > - RendererInfo *inf = renderers_info;
> > - while (inf->name) {
> > - if (strcmp(name, inf->name) == 0) {
> > - return inf;
> > + const char* e;
> > + while (*codecs == ';') {
> > + codecs++;
> > + }
> > + if (!*codecs) {
> > + return NULL;
> > + }
> > + e = codecs;
> > + while (isalnum(*codecs) || *codecs == '_') {
>
> Why not using sscanf (like we do on the client to parse URI) or even
> strtok?
That's a good idea. Done.
> > +int red_dispatcher_set_video_codecs(const char *codecs)
> > {
[...]
> > + 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;
>
> So, if one of the options are wrong, this makes everything wrong.
> I'd go for a spice_warning to show the error but use continue instead of
> return FALSE...
I modified this code so if a video:codec specification is wrong we print
a warning and skip to what follows the next semi-colon.
> > + if (num_video_codecs < 0) {
> > + red_dispatcher_set_video_codecs("auto");
>
> "auto" is user option, internally we should use VIDEO_ENCODER_DEFAULT_PREFERENCE
-> Done.
> > +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;
> > +}
>
> Looks good, I think part of this could go to gst-0.10 patch so the
> rework between the series would be reduced like the ifdef below that
> you have included and now removed.
I don't think much can go in the gst-0.10 patch. That patch only
supports MJPEG so it would not make sense to introduce all the codec
configuration code. I also don't think it would make sense to introduce
a user-visible option to make it possible to choose between the
GStreamer and Spice video encoders, only to then change its syntax
completely. Also initialization cannot fail so the error handling code
would not make sense either. I did add
red_display_create_video_encoder() but I think that's as far as makes
sense.
> > +SPICE_GNUC_VISIBLE int spice_server_set_video_codecs(SpiceServer *s, const char* video_codecs)
> > +{
> > + spice_assert(reds == s);
> > +
> > + if (!red_dispatcher_set_video_codecs(video_codecs)) {
>
> We may have a warning/debug here.
I have modified red_dispatcher_set_video_codecs() so it issues the right
warning for each case. So it's not necessary to issue a warning here.
> > -/* Instantiates the builtin MJPEG video encoder.
> > +/* Instantiates the a video encoder for the specified codec.
>
> s/the a/the
Done.
Here is the updated patch below.
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 a1df2fd..8e61441 100644
--- a/server/gstreamer_encoder.c
+++ b/server/gstreamer_encoder.c
@@ -110,6 +110,8 @@ static void reset_pipeline(GstEncoder *encoder)
gst_element_set_state(encoder->pipeline, GST_STATE_NULL);
gst_caps_unref(encoder->src_caps);
+ encoder->src_caps = NULL;
+
gst_object_unref(encoder->appsrc);
gst_object_unref(encoder->gstenc);
gst_object_unref(encoder->appsink);
@@ -168,9 +170,12 @@ static SpiceFormatForGStreamer *map_format(SpiceBitmapFmt format)
return NULL;
}
-static gboolean set_appsrc_caps(GstEncoder *encoder)
+static void set_appsrc_caps(GstEncoder *encoder)
{
- GstCaps *new_caps = gst_caps_new_simple("video/x-raw-rgb",
+ if (encoder->src_caps) {
+ gst_caps_unref(encoder->src_caps);
+ }
+ encoder->src_caps = gst_caps_new_simple("video/x-raw-rgb",
"bpp", G_TYPE_INT, encoder->format->bpp,
"depth", G_TYPE_INT, encoder->format->depth,
"endianness", G_TYPE_INT, encoder->format->endianness,
@@ -181,49 +186,73 @@ static gboolean set_appsrc_caps(GstEncoder *encoder)
"height", G_TYPE_INT, encoder->height,
"framerate", GST_TYPE_FRACTION, get_source_fps(encoder), 1,
NULL);
- if (!new_caps) {
- spice_warning("GStreamer error: could not create the source caps");
- return FALSE;
- }
- g_object_set(G_OBJECT(encoder->appsrc), "caps", new_caps, NULL);
- if (encoder->src_caps) {
- gst_caps_unref(encoder->src_caps);
- }
- encoder->src_caps = new_caps;
- return TRUE;
+ g_object_set(G_OBJECT(encoder->appsrc), "caps", encoder->src_caps, NULL);
}
/* A helper for gst_encoder_encode_frame(). */
static gboolean construct_pipeline(GstEncoder *encoder, const SpiceBitmap *bitmap)
{
+ const gchar* gstenc_name;
+ switch (encoder->base.codec_type)
+ {
+ case SPICE_VIDEO_CODEC_TYPE_MJPEG:
+ gstenc_name = "ffenc_mjpeg";
+ 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);
- if (!encoder->pipeline) {
+ g_free(desc);
+ if (!encoder->pipeline || err) {
spice_warning("GStreamer error: %s", err->message);
g_clear_error(&err);
+ if (encoder->pipeline) {
+ gst_object_unref(encoder->pipeline);
+ encoder->pipeline = NULL;
+ }
return FALSE;
}
encoder->appsrc = GST_APP_SRC(gst_bin_get_by_name(GST_BIN(encoder->pipeline), "src"));
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 source caps */
- encoder->src_caps = NULL;
- if (!set_appsrc_caps(encoder)) {
- spice_warning("could not set the GStreamer source caps");
- reset_pipeline(encoder);
- return FALSE;
- }
-
- /* Set the encoder initial bit rate */
+ /* 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) {
+ /* 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),
+ "resize-allowed", TRUE, /* for very low bit rates */
+ "mode", 1, /* CBR */
+ "max-latency", 0, /* zero-frame latency */
+ "error-resilient", TRUE, /* for client frame drops */
+ "speed", 7, /* ultrafast */
+ "threads", core_count - 1,
+ NULL);
+ }
+
+ /* Set the source caps */
+ set_appsrc_caps(encoder);
/* Start playing */
- spice_debug("removing the pipeline clock");
- gst_pipeline_use_clock(GST_PIPELINE(encoder->pipeline), NULL);
+ if (encoder->base.codec_type == SPICE_VIDEO_CODEC_TYPE_MJPEG) {
+ /* See https://bugzilla.gnome.org/show_bug.cgi?id=753257 */
+ 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) == GST_STATE_CHANGE_FAILURE) {
spice_warning("GStreamer error: unable to set the pipeline to the playing state");
@@ -237,11 +266,20 @@ static gboolean construct_pipeline(GstEncoder *encoder, const SpiceBitmap *bitma
/* A helper for gst_encoder_encode_frame(). */
static void reconfigure_pipeline(GstEncoder *encoder)
{
- if (gst_element_set_state(encoder->pipeline, GST_STATE_PAUSED) == GST_STATE_CHANGE_FAILURE ||
- !set_appsrc_caps(encoder) ||
- gst_element_set_state(encoder->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE) {
- spice_debug("GStreamer error: the pipeline reconfiguration failed, rebuilding it instead");
- reset_pipeline(encoder); /* we can rebuild it... */
+ if (encoder->base.codec_type == SPICE_VIDEO_CODEC_TYPE_VP8) {
+ /* vp8enc gets confused if we try to reconfigure the pipeline */
+ reset_pipeline(encoder);
+ return;
+ }
+
+ if (gst_element_set_state(encoder->pipeline, GST_STATE_PAUSED) == GST_STATE_CHANGE_FAILURE) {
+ spice_debug("GStreamer error: could not pause the pipeline, rebuilding it instead");
+ reset_pipeline(encoder);
+ }
+ set_appsrc_caps(encoder);
+ if (gst_element_set_state(encoder->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE) {
+ spice_debug("GStreamer error: could not restart the pipeline, rebuilding it instead");
+ reset_pipeline(encoder);
}
}
@@ -447,11 +485,19 @@ static void gst_encoder_get_stats(GstEncoder *encoder, VideoEncoderStats *stats)
}
}
-GstEncoder *create_gstreamer_encoder(uint64_t starting_bit_rate, VideoEncoderRateControlCbs *cbs, void *cbs_opaque)
+GstEncoder *create_gstreamer_encoder(SpiceVideoCodecType codec_type,
+ uint64_t starting_bit_rate,
+ VideoEncoderRateControlCbs *cbs,
+ void *cbs_opaque)
{
GstEncoder *encoder;
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) {
+ spice_warning("unsupported codec type %d", codec_type);
+ return NULL;
+ }
gst_init(NULL, NULL);
@@ -462,6 +508,7 @@ GstEncoder *create_gstreamer_encoder(uint64_t starting_bit_rate, VideoEncoderRat
encoder->base.notify_server_frame_drop = &gst_encoder_notify_server_frame_drop;
encoder->base.get_bit_rate = &gst_encoder_get_bit_rate;
encoder->base.get_stats = &gst_encoder_get_stats;
+ encoder->base.codec_type = codec_type;
if (cbs) {
encoder->cbs = *cbs;
diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 2a2ebd0..f0d8019 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -1335,10 +1335,12 @@ static void mjpeg_encoder_get_stats(MJpegEncoder *encoder, VideoEncoderStats *st
stats->avg_quality = (double)encoder->avg_quality / encoder->num_frames;
}
-MJpegEncoder *create_mjpeg_encoder(uint64_t starting_bit_rate,
+MJpegEncoder *create_mjpeg_encoder(SpiceVideoCodecType codec_type,
+ uint64_t starting_bit_rate,
VideoEncoderRateControlCbs *cbs,
void *cbs_opaque)
{
+ spice_assert(codec_type == SPICE_VIDEO_CODEC_TYPE_MJPEG);
spice_assert(!cbs || (cbs && cbs->get_roundtrip_ms && cbs->get_source_fps));
MJpegEncoder *encoder = spice_new0(MJpegEncoder, 1);
@@ -1349,6 +1351,7 @@ MJpegEncoder *create_mjpeg_encoder(uint64_t starting_bit_rate,
encoder->base.notify_server_frame_drop = &mjpeg_encoder_notify_server_frame_drop;
encoder->base.get_bit_rate = &mjpeg_encoder_get_bit_rate;
encoder->base.get_stats = &mjpeg_encoder_get_stats;
+ encoder->base.codec_type = codec_type;
encoder->first_frame = TRUE;
encoder->rate_control.byte_rate = starting_bit_rate / 8;
encoder->starting_bit_rate = starting_bit_rate;
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index 113848a..acceb53 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -27,6 +27,7 @@
#include <sys/socket.h>
#include <signal.h>
#include <inttypes.h>
+#include <ctype.h>
#include <spice/qxl_dev.h>
#include "common/quic.h"
@@ -205,43 +206,152 @@ static void red_dispatcher_cursor_migrate(RedChannelClient *rcc)
&payload);
}
-typedef struct RendererInfo {
- int id;
+typedef struct {
+ uint32_t id;
const char *name;
-} RendererInfo;
+} EnumNames;
-static RendererInfo renderers_info[] = {
+static int name_to_enum(const EnumNames names[], const char *string, uint32_t *id)
+{
+ if (string) {
+ int i = 0;
+ while (names[i].name) {
+ if (strcmp(string, names[i].name) == 0) {
+ *id = names[i].id;
+ return TRUE;
+ }
+ i++;
+ }
+ }
+ return FALSE;
+}
+
+static const EnumNames renderer_names[] = {
{RED_RENDERER_SW, "sw"},
#ifdef USE_OPENGL
{RED_RENDERER_OGL_PBUF, "oglpbuf"},
{RED_RENDERER_OGL_PIXMAP, "oglpixmap"},
#endif
- {RED_RENDERER_INVALID, NULL},
+ {0, NULL},
};
static uint32_t renderers[RED_MAX_RENDERERS];
static uint32_t num_renderers = 0;
-static RendererInfo *find_renderer(const char *name)
+int red_dispatcher_add_renderer(const char *name)
{
- RendererInfo *inf = renderers_info;
- while (inf->name) {
- if (strcmp(name, inf->name) == 0) {
- return inf;
- }
- inf++;
+ uint32_t renderer;
+
+ if (num_renderers == RED_MAX_RENDERERS ||
+ !name_to_enum(renderer_names, name, &renderer)) {
+ return FALSE;
}
- return NULL;
+ renderers[num_renderers++] = renderer;
+ return TRUE;
}
-int red_dispatcher_add_renderer(const char *name)
+static const EnumNames video_encoder_names[] = {
+ {0, "spice"},
+ {1, "gstreamer"},
+ {0, NULL},
+};
+
+static create_video_encoder_proc video_encoder_procs[] = {
+ &create_mjpeg_encoder,
+#ifdef HAVE_GSTREAMER_0_10
+ &create_gstreamer_encoder,
+#else
+ NULL,
+#endif
+};
+
+static const EnumNames video_codec_names[] = {
+ {SPICE_VIDEO_CODEC_TYPE_MJPEG, "mjpeg"},
+ {SPICE_VIDEO_CODEC_TYPE_VP8, "vp8"},
+ {0, NULL},
+};
+
+static const EnumNames video_cap_names[] = {
+ {SPICE_DISPLAY_CAP_CODEC_MJPEG, "mjpeg"},
+ {SPICE_DISPLAY_CAP_CODEC_VP8, "vp8"},
+ {0, NULL},
+};
+
+
+static RedVideoCodec video_codecs[RED_MAX_VIDEO_CODECS];
+static int num_video_codecs = -1;
+
+/* Expected string: encoder:codec;encoder:codec */
+static const char* parse_video_codecs(const char *codecs, char **encoder,
+ char **codec)
{
- RendererInfo *inf;
+ while (*codecs == ';') {
+ codecs++;
+ }
+ if (!*codecs) {
+ return NULL;
+ }
+ int n;
+ *encoder = *codec = NULL;
+ if (sscanf(codecs, "%m[0-9a-zA-Z_]:%m[0-9a-zA-Z_]%n", encoder, codec, &n) != 2) {
+ spice_warning("spice: invalid encoder:codec value at %s", codecs);
+ return strchr(codecs, ';');
+ }
+ return codecs + n;
+}
- if (num_renderers == RED_MAX_RENDERERS || !(inf = find_renderer(name))) {
- return FALSE;
+int red_dispatcher_set_video_codecs(const char *codecs)
+{
+ uint32_t encoder;
+ SpiceVideoCodecType type;
+ uint32_t cap;
+ char *encoder_name, *codec_name;
+ static RedVideoCodec new_codecs[RED_MAX_VIDEO_CODECS];
+ int count;
+ const char* c;
+
+ 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 skip = FALSE;
+ if (!encoder_name || !codec_name) {
+ skip = TRUE;
+
+ } else if (!name_to_enum(video_encoder_names, encoder_name, &encoder)) {
+ spice_warning("spice: unknown video encoder %s", encoder_name);
+ skip = TRUE;
+
+ } else if (!name_to_enum(video_codec_names, codec_name, &type) ||
+ !name_to_enum(video_cap_names, codec_name, &cap)) {
+ spice_warning("spice: unknown video codec %s", codec_name);
+ skip = TRUE;
+
+ } else if (!video_encoder_procs[encoder]) {
+ spice_warning("spice: unsupported video encoder %s", encoder_name);
+ skip = TRUE;
+ }
+
+ free(encoder_name);
+ free(codec_name);
+ if (skip) {
+ continue;
+ }
+
+ if (count == RED_MAX_VIDEO_CODECS) {
+ spice_warning("spice: cannot add more than %d video codec preferences", count);
+ break;
+ }
+ 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;
}
@@ -785,6 +895,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;
@@ -1092,6 +1218,11 @@ 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(VIDEO_ENCODER_DEFAULT_PREFERENCE);
+ }
+ 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 320b7e3..82aed9f 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 c739d4f..b32ccf6 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
@@ -993,6 +994,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;
@@ -3080,21 +3083,45 @@ 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(uint64_t starting_bit_rate,
+static VideoEncoder* red_display_create_video_encoder(DisplayChannelClient *dcc,
+ uint64_t starting_bit_rate,
VideoEncoderRateControlCbs *cbs,
void *cbs_opaque)
{
-#ifdef HAVE_GSTREAMER_0_10
- VideoEncoder* video_encoder = create_gstreamer_encoder(starting_bit_rate, cbs, cbs_opaque);
- if (video_encoder) {
- return video_encoder;
+ 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;
+ }
}
-#endif
- /* Use the builtin MJPEG video encoder as a fallback */
- return create_mjpeg_encoder(starting_bit_rate, cbs, cbs_opaque);
+
+ /* 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 *stream)
+static int red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
{
StreamAgent *agent = &dcc->stream_agents[get_stream_id(dcc->common.worker, stream)];
@@ -3120,10 +3147,15 @@ static void red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
video_cbs.update_client_playback_delay = red_stream_update_client_playback_latency;
initial_bit_rate = red_stream_get_initial_bit_rate(dcc, stream);
- agent->video_encoder = red_display_create_video_encoder(initial_bit_rate, &video_cbs, agent);
+ agent->video_encoder = red_display_create_video_encoder(dcc, initial_bit_rate, &video_cbs, agent);
} else {
- agent->video_encoder = red_display_create_video_encoder(0, NULL, NULL);
+ agent->video_encoder = red_display_create_video_encoder(dcc, 0, NULL, NULL);
}
+ if (agent->video_encoder == NULL) {
+ stream->refs--;
+ return FALSE;
+ }
+
red_channel_client_pipe_add(&dcc->common.base, &agent->create_item);
if (red_channel_client_test_remote_cap(&dcc->common.base, SPICE_DISPLAY_CAP_STREAM_REPORT)) {
@@ -3141,6 +3173,7 @@ static void red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
agent->stats.start = stream->current->red_drawable->mm_time;
}
#endif
+ return TRUE;
}
static void red_create_stream(RedWorker *worker, Drawable *drawable)
@@ -3175,7 +3208,12 @@ static void red_create_stream(RedWorker *worker, Drawable *drawable)
worker->streams_size_total += stream->width * stream->height;
worker->stream_count++;
WORKER_FOREACH_DCC_SAFE(worker, dcc_ring_item, next, dcc) {
- red_display_create_stream(dcc, stream);
+ if (!red_display_create_stream(dcc, stream)) {
+ drawable->stream = NULL;
+ stream->current = NULL;
+ red_stop_stream(worker, stream);
+ return;
+ }
}
spice_debug("stream %d %dx%d (%d, %d) (%d, %d)", (int)(stream - worker->streams_buf), stream->width,
stream->height, stream->dest_area.left, stream->dest_area.top,
@@ -3190,7 +3228,10 @@ static void red_disply_start_streams(DisplayChannelClient *dcc)
while ((item = ring_next(ring, item))) {
Stream *stream = SPICE_CONTAINEROF(item, Stream, link);
- red_display_create_stream(dcc, stream);
+ if (!red_display_create_stream(dcc, stream)) {
+ red_stop_stream(dcc->common.worker, stream);
+ return;
+ }
}
}
@@ -8937,7 +8978,7 @@ static void red_display_marshall_stream_start(RedChannelClient *rcc,
stream_create.surface_id = 0;
stream_create.id = get_stream_id(dcc->common.worker, stream);
stream_create.flags = stream->top_down ? SPICE_STREAM_FLAGS_TOP_DOWN : 0;
- stream_create.codec_type = SPICE_VIDEO_CODEC_TYPE_MJPEG;
+ stream_create.codec_type = agent->video_encoder->codec_type;
stream_create.src_width = stream->width;
stream_create.src_height = stream->height;
@@ -11754,6 +11795,15 @@ void handle_dev_set_streaming_video(void *opaque, void *payload)
}
}
+void handle_dev_set_video_codecs(void *opaque, void *payload)
+{
+ RedWorkerMessageSetVideoCodecs *msg = payload;
+ RedWorker *worker = opaque;
+
+ worker->num_video_codecs = msg->num_video_codecs;
+ memcpy(worker->video_codecs, msg->video_codecs, sizeof(worker->video_codecs));
+}
+
void handle_dev_set_mouse_mode(void *opaque, void *payload)
{
RedWorkerMessageSetMouseMode *msg = payload;
@@ -12060,6 +12110,8 @@ static void red_init(RedWorker *worker, WorkerInitData *init_data)
worker->jpeg_state = init_data->jpeg_state;
worker->zlib_glz_state = init_data->zlib_glz_state;
worker->streaming_video = init_data->streaming_video;
+ worker->num_video_codecs = init_data->num_video_codecs;
+ memcpy(worker->video_codecs, init_data->video_codecs, sizeof(worker->video_codecs));
worker->driver_cap_monitors_config = 0;
ring_init(&worker->current_list);
image_cache_init(&worker->image_cache);
diff --git a/server/red_worker.h b/server/red_worker.h
index c34369a..c13d225 100644
--- a/server/red_worker.h
+++ b/server/red_worker.h
@@ -21,6 +21,7 @@
#include <unistd.h>
#include <errno.h>
#include "red_common.h"
+#include "video_encoder.h"
enum {
RED_WORKER_PENDING_WAKEUP,
@@ -69,6 +70,7 @@ enum {
RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC,
RED_WORKER_MESSAGE_DRIVER_UNLOAD,
+ RED_WORKER_MESSAGE_SET_VIDEO_CODECS,
RED_WORKER_MESSAGE_COUNT // LAST
};
@@ -84,6 +86,20 @@ enum {
RED_RENDERER_OGL_PIXMAP,
};
+#define RED_MAX_VIDEO_CODECS 8
+
+typedef struct RedVideoCodec {
+ create_video_encoder_proc create;
+ SpiceVideoCodecType type;
+ uint32_t cap;
+} RedVideoCodec;
+
+enum {
+ SPICE_STREAMING_INVALID,
+ SPICE_STREAMING_SPICE,
+ SPICE_STREAMING_GSTREAMER
+};
+
typedef struct RedDispatcher RedDispatcher;
typedef struct WorkerInitData {
@@ -96,6 +112,8 @@ typedef struct WorkerInitData {
spice_wan_compression_t jpeg_state;
spice_wan_compression_t zlib_glz_state;
int streaming_video;
+ uint32_t num_video_codecs;
+ RedVideoCodec video_codecs[RED_MAX_VIDEO_CODECS];
uint32_t num_memslots;
uint32_t num_memslots_groups;
uint8_t memslot_gen_bits;
diff --git a/server/reds.c b/server/reds.c
index cec28b8..b83e57a 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3682,6 +3682,18 @@ SPICE_GNUC_VISIBLE int spice_server_set_streaming_video(SpiceServer *s, int valu
return 0;
}
+SPICE_GNUC_VISIBLE int spice_server_set_video_codecs(SpiceServer *s, const char* video_codecs)
+{
+ spice_assert(reds == s);
+
+ if (!red_dispatcher_set_video_codecs(video_codecs)) {
+ return -1;
+ }
+
+ red_dispatcher_on_vc_change();
+ return 0;
+}
+
SPICE_GNUC_VISIBLE int spice_server_set_playback_compression(SpiceServer *s, int enable)
{
spice_assert(reds == s);
diff --git a/server/spice-server.h b/server/spice-server.h
index c2ff61d..198dcf0 100644
--- a/server/spice-server.h
+++ b/server/spice-server.h
@@ -107,6 +107,7 @@ enum {
};
int spice_server_set_streaming_video(SpiceServer *s, int value);
+int spice_server_set_video_codecs(SpiceServer *s, const char* video_codecs);
int spice_server_set_playback_compression(SpiceServer *s, int enable);
int spice_server_set_agent_mouse(SpiceServer *s, int enable);
int spice_server_set_agent_copypaste(SpiceServer *s, int enable);
diff --git a/server/spice-server.syms b/server/spice-server.syms
index 3c7d945..295ee59 100644
--- a/server/spice-server.syms
+++ b/server/spice-server.syms
@@ -32,6 +32,7 @@ global:
spice_server_set_playback_compression;
spice_server_set_port;
spice_server_set_streaming_video;
+ spice_server_set_video_codecs;
spice_server_set_ticket;
spice_server_set_tls;
spice_server_set_zlib_glz_compression;
diff --git a/server/video_encoder.h b/server/video_encoder.h
index 64af9ae..ac96589 100644
--- a/server/video_encoder.h
+++ b/server/video_encoder.h
@@ -122,6 +122,9 @@ struct VideoEncoder {
* statistics.
*/
void (*get_stats)(VIDEO_ENCODER_T *encoder, VideoEncoderStats *stats);
+
+ /* The codec being used by the video encoder */
+ SpiceVideoCodecType codec_type;
};
@@ -149,23 +152,28 @@ typedef struct VideoEncoderRateControlCbs {
} VideoEncoderRateControlCbs;
-/* Instantiates the builtin MJPEG video encoder.
+/* Instantiates the video encoder for the specified codec.
*
- * @starting_bit_rate: An initial estimate of the available stream bit rate .
+ * @codec_type: The codec to use.
+ * @starting_bit_rate: An initial estimate of the available stream bit rate.
* @bit_rate_control: True if the client supports rate control.
* @cbs: A set of callback methods to be used for rate control.
* @cbs_opaque: A pointer to be passed to the rate control callbacks.
* @return: A pointer to a structure implementing the VideoEncoder
* methods.
*/
-typedef VIDEO_ENCODER_T* (*create_video_encoder_proc)(uint64_t starting_bit_rate, VideoEncoderRateControlCbs *cbs, void *cbs_opaque);
+typedef VIDEO_ENCODER_T* (*create_video_encoder_proc)(SpiceVideoCodecType codec_type, uint64_t starting_bit_rate, VideoEncoderRateControlCbs *cbs, void *cbs_opaque);
-VIDEO_ENCODER_T* create_mjpeg_encoder(uint64_t starting_bit_rate,
+VIDEO_ENCODER_T* create_mjpeg_encoder(SpiceVideoCodecType codec_type,
+ uint64_t starting_bit_rate,
VideoEncoderRateControlCbs *cbs,
void *cbs_opaque);
-VIDEO_ENCODER_T* create_gstreamer_encoder(uint64_t starting_bit_rate,
+VIDEO_ENCODER_T* create_gstreamer_encoder(SpiceVideoCodecType codec_type,
+ uint64_t starting_bit_rate,
VideoEncoderRateControlCbs *cbs,
void *cbs_opaque);
+#define VIDEO_ENCODER_DEFAULT_PREFERENCE "spice:mjpeg;gstreamer:mjpeg;gstreamer:vp8"
+
#endif
--
Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel