[Spice-devel] [RFC spice-server] reds: add support to priority for video codecs
Victor Toso
victortoso at redhat.com
Fri Jan 6 14:45:23 UTC 2017
From: Victor Toso <me at victortoso.com>
This patch implements a new value to the preference introduced in
497fcbb0a315b034ba keeping it backwards compatible. The new value is
the priority, which is an unsigned integer and should be set as last
argument. e.g: encoder:codec:rank
Video codecs will now be ordered by its priority.
The logic around the priority is the following:
* Default priority value is 1;
* Higher the priority means higher preference for encoding;
* Priority of value 0 disables encoding with this particular codec;
* Priority should be a single digit (0-9)
Signed-off-by: Victor Toso <victortoso at redhat.com>
---
server/dcc.c | 29 +++++++++++++++--------------
server/reds.c | 17 +++++++++++++++--
server/tests/test-codecs-parsing.c | 5 +++++
server/video-encoder.h | 1 +
4 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/server/dcc.c b/server/dcc.c
index 62c6b45..460d02b 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -1114,28 +1114,29 @@ static int dcc_handle_preferred_compression(DisplayChannelClient *dcc,
return TRUE;
}
-
-/* TODO: Client preference should only considered when host has video-codecs
- * with the same priority value. At the moment, server's video-codec order
- * is sorted following client's preference. */
static gint sort_video_codecs_by_client_preference(gconstpointer a_pointer,
gconstpointer b_pointer,
gpointer user_data)
{
- gint i;
const RedVideoCodec *a = a_pointer;
const RedVideoCodec *b = b_pointer;
- GArray *client_pref = user_data;
- for (i = 0; i < client_pref->len; i++) {
- if (a->type == g_array_index(client_pref, gint, i))
- return -1;
+ /* Client preference is only considered when host has video-codecs with the
+ * same priority value */
+ if (a->priority == b->priority) {
+ gint i;
+ GArray *client_pref = user_data;
- if (b->type == g_array_index(client_pref, gint, i))
- return 1;
+ for (i = 0; i < client_pref->len; i++) {
+ if (a->type == g_array_index(client_pref, gint, i))
+ return -1;
+
+ if (b->type == g_array_index(client_pref, gint, i))
+ return 1;
+ }
}
- return 0;
+ return (a->priority - b->priority);
}
static void dcc_update_preferred_video_codecs(DisplayChannelClient *dcc)
@@ -1163,10 +1164,10 @@ static void dcc_update_preferred_video_codecs(DisplayChannelClient *dcc)
g_clear_pointer(&dcc->priv->preferred_video_codecs, g_array_unref);
dcc->priv->preferred_video_codecs = video_codecs;
- msg = g_string_new("Preferred video-codecs:");
+ msg = g_string_new("Preferred video-codecs (type-priority):");
for (i = 0; i < video_codecs->len; i++) {
RedVideoCodec codec = g_array_index(video_codecs, RedVideoCodec, i);
- g_string_append_printf(msg, " %d", codec.type);
+ g_string_append_printf(msg, " %d-%d", codec.type, codec.priority);
}
spice_info("%s", msg->str);
g_string_free(msg, TRUE);
diff --git a/server/reds.c b/server/reds.c
index 26a8b51..c16dfce 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3512,6 +3512,14 @@ static const int video_codec_caps[] = {
};
+static gint sort_video_codecs_on_priority(gconstpointer a_pointer, gconstpointer b_pointer)
+{
+ const RedVideoCodec* a = a_pointer;
+ const RedVideoCodec* b = b_pointer;
+
+ return (b->priority - a->priority);
+}
+
/*
* @codecs should be described in a semi-colon separated list of encoder:codec.
* e.g: "gstreamer:vp8;spice:mjpeg"
@@ -3531,7 +3539,7 @@ static void reds_set_video_codecs_from_string(RedsState *reds, const char *codec
video_codecs = g_array_new(FALSE, FALSE, sizeof(RedVideoCodec));
preferences = g_strsplit(codecs, ";", -1);
for (i = 0; preferences[i] != NULL; i++) {
- const gchar *encoder_name, *codec_name;
+ const gchar *encoder_name, *codec_name, *codec_priority;
uint32_t encoder_index, codec_index;
gchar **pref;
@@ -3539,7 +3547,7 @@ static void reds_set_video_codecs_from_string(RedsState *reds, const char *codec
if (g_str_equal(preferences[i], ""))
continue;
- pref = g_strsplit(preferences[i], ":", 2);
+ pref = g_strsplit(preferences[i], ":", 3);
if (pref[0] == NULL || pref[1] == NULL) {
spice_warning("spice: invalid encoder:codec value at '%s'", preferences[i]);
@@ -3549,6 +3557,7 @@ static void reds_set_video_codecs_from_string(RedsState *reds, const char *codec
encoder_name = pref[0];
codec_name = pref[1];
+ codec_priority = pref[2];
if (!get_name_index(video_encoder_names, encoder_name, &encoder_index)){
spice_warning("spice: unknown video encoder %s", encoder_name);
@@ -3561,9 +3570,11 @@ static void reds_set_video_codecs_from_string(RedsState *reds, const char *codec
} else {
RedVideoCodec new_codec;
+
new_codec.create = video_encoder_procs[encoder_index];
new_codec.type = video_codec_names[codec_index].id;
new_codec.cap = video_codec_caps[codec_index];
+ new_codec.priority = (codec_priority != NULL) ? atoi(codec_priority) : 1;
g_array_append_val(video_codecs, new_codec);
}
g_strfreev(pref);
@@ -3933,6 +3944,8 @@ static void reds_set_video_codecs(RedsState *reds, GArray *video_codecs)
spice_return_if_fail(video_codecs != NULL);
+ g_array_sort(video_codecs, sort_video_codecs_on_priority);
+
reds->config->video_codecs = video_codecs;
}
diff --git a/server/tests/test-codecs-parsing.c b/server/tests/test-codecs-parsing.c
index 5af2e5d..ad74890 100644
--- a/server/tests/test-codecs-parsing.c
+++ b/server/tests/test-codecs-parsing.c
@@ -29,12 +29,17 @@ static void codecs_good(void)
guint i;
const gchar *codecs[] = {
"spice:mjpeg",
+ "spice:mjpeg:1",
+ "spice:mjpeg:9",
"spice:mjpeg;;;",
"spice:mjpeg;;spice:mjpeg;;;",
+ "spice:mjpeg:1;;spice:mjpeg:2;;;",
";;spice:mjpeg;;spice:mjpeg;;;",
#if defined(HAVE_GSTREAMER_1_0) || defined(HAVE_GSTREAMER_0_10)
"gstreamer:mjpeg;gstreamer:h264;gstreamer:vp8;",
+ "gstreamer:mjpeg;gstreamer:h264:0;gstreamer:vp8:9;",
";;spice:mjpeg;;gstreamer:mjpeg;gstreamer:h264;gstreamer:vp8;",
+ ";;spice:mjpeg:0;;gstreamer:mjpeg:0;gstreamer:h264:1;gstreamer:vp8:0;",
#endif
};
diff --git a/server/video-encoder.h b/server/video-encoder.h
index a4cd2b3..8b62bc9 100644
--- a/server/video-encoder.h
+++ b/server/video-encoder.h
@@ -210,6 +210,7 @@ VideoEncoder* gstreamer_encoder_new(SpiceVideoCodecType codec_type,
typedef struct RedVideoCodec {
new_video_encoder_t create;
SpiceVideoCodecType type;
+ uint8_t priority;
uint32_t cap;
} RedVideoCodec;
--
2.9.3
More information about the Spice-devel
mailing list