[Spice-devel] [spice-server v6 9/9] (RFC) reds: add support to priority for video codecs

Victor Toso victortoso at redhat.com
Tue Jan 31 11:08:22 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 d20edee..084e6e9 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3527,6 +3527,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"
@@ -3546,7 +3554,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;
 
@@ -3555,7 +3563,7 @@ static void reds_set_video_codecs_from_string(RedsState *reds, const char *codec
             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]);
@@ -3565,6 +3573,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);
@@ -3577,9 +3586,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);
@@ -3949,6 +3960,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 16fab24..259421c 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