[Spice-devel] [spice v8] dcc: handle preferred video codec message
Frediano Ziglio
fziglio at redhat.com
Mon Feb 20 12:25:07 UTC 2017
>
> [0] SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE
>
> This message provides a list of video codecs based on client's order
> of preference.
>
> We duplicate the video codecs array from reds.c and sort it using the
> order of codecs as reference.
>
> This message will not change an ongoing streaming but it could change
> newly created streams depending the rank value of each video codec
> that can be set by spice_server_set_video_codecs()
>
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
> server/dcc-private.h | 5 ++
> server/dcc.c | 126
> +++++++++++++++++++++++++++++++++++++++++++++++
> server/dcc.h | 1 +
> server/display-channel.c | 2 +
> server/stream.c | 5 +-
> 5 files changed, 138 insertions(+), 1 deletion(-)
>
> diff --git a/server/dcc-private.h b/server/dcc-private.h
> index 64b32a7..dd54c70 100644
> --- a/server/dcc-private.h
> +++ b/server/dcc-private.h
> @@ -51,6 +51,11 @@ struct DisplayChannelClientPrivate
> int num_pixmap_cache_items;
> } send_data;
>
> + /* Host prefererred video-codec order sorted with client preferred */
prefererred -> preferred
> + GArray *preferred_video_codecs;
> + /* Last client preferred video-codec order */
> + GArray *client_preferred_video_codecs;
> +
> uint8_t surface_client_created[NUM_SURFACES];
> QRegion surface_client_lossy_region[NUM_SURFACES];
>
> diff --git a/server/dcc.c b/server/dcc.c
> index afe37b1..9271ea5 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -39,6 +39,8 @@ enum
> PROP_ZLIB_GLZ_STATE
> };
>
> +static void on_display_video_codecs_update(GObject *gobject, GParamSpec
> *pspec, gpointer user_data);
> +
> static void
> display_channel_client_get_property(GObject *object,
> guint property_id,
> @@ -99,12 +101,19 @@ display_channel_client_constructed(GObject *object)
> dcc_init_stream_agents(self);
>
> image_encoders_init(&self->priv->encoders,
> &DCC_TO_DC(self)->priv->encoder_shared_data);
> +
> + g_signal_connect(DCC_TO_DC(self), "notify::video-codecs",
> + G_CALLBACK(on_display_video_codecs_update), self);
> }
>
> static void
> display_channel_client_finalize(GObject *object)
> {
> DisplayChannelClient *self = DISPLAY_CHANNEL_CLIENT(object);
> +
> + g_signal_handlers_disconnect_by_func(DCC_TO_DC(self),
> on_display_video_codecs_update, self);
> + g_clear_pointer(&self->priv->preferred_video_codecs, g_array_unref);
> + g_clear_pointer(&self->priv->client_preferred_video_codecs,
> g_array_unref);
> g_free(self->priv);
>
> G_OBJECT_CLASS(display_channel_client_parent_class)->finalize(object);
> @@ -1105,6 +1114,120 @@ static int
> dcc_handle_preferred_compression(DisplayChannelClient *dcc,
> return TRUE;
> }
>
> +
> +/* TODO: Client preference should only be considered when host has
> video-codecs
> + * with the same priority value. At the moment, the video-codec GArray will
> be
> + * sorted following only the client's preference (@user_data)
> + *
> + * example:
> + * host encoding preference: gstreamer:mjpeg;gstreamer:vp8;gstreamer:h264
> + * client decoding preference: h264, vp9, mjpeg
> + * result: gstreamer:h264;gstreamer:mjpeg;gstreamer:vp8
> + */
> +static gint sort_video_codecs_by_client_preference(gconstpointer a_pointer,
> + gconstpointer b_pointer,
> + gpointer user_data)
> +{
> + guint 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;
style, always use brackets
> +
> + if (b->type == g_array_index(client_pref, gint, i))
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +static void dcc_update_preferred_video_codecs(DisplayChannelClient *dcc)
> +{
> + guint i;
> + RedsState *reds;
> + GArray *video_codecs, *server_codecs;
> + GString *msg;
> +
> + reds =
> red_channel_get_server(red_channel_client_get_channel(&dcc->parent));
> + server_codecs = reds_get_video_codecs(reds);
This is not thread safe, don't call reds_get_video_codecs from here,
use display_channel_get_video_codecs.
> + spice_return_if_fail(server_codecs != NULL);
> +
> + /* Copy current host preference */
> + video_codecs = g_array_new(FALSE, FALSE, sizeof(RedVideoCodec));
> + for (i = 0; i < server_codecs->len; i++) {
> + RedVideoCodec codec = g_array_index(server_codecs, RedVideoCodec,
> i);
> + g_array_append_val(video_codecs, codec);
> + }
> +
> + /* Sort the copy of current host preference based on client's preference
> */
> + g_array_sort_with_data(video_codecs,
> sort_video_codecs_by_client_preference,
> + dcc->priv->client_preferred_video_codecs);
> + 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:");
> + 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);
> + }
> + spice_info("%s", msg->str);
Maybe spice_debug is more appropriate here.
spice_info is for users.
> + g_string_free(msg, TRUE);
> +}
> +
> +static void on_display_video_codecs_update(GObject *gobject,
> + GParamSpec *pspec, gpointer user_data)
> +{
> + DisplayChannelClient *dcc = DISPLAY_CHANNEL_CLIENT(user_data);
> +
> + /* Only worry about video-codecs update if client has sent
> + * SPICE_MSGC_DISPLAY_PREFERRED_COMPRESSION */
SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE ?
> + if (dcc->priv->client_preferred_video_codecs == NULL)
> + return;
> +
style
> + /* New host preference */
> + dcc_update_preferred_video_codecs(dcc);
> +}
> +
> +static int dcc_handle_preferred_video_codec_type(DisplayChannelClient *dcc,
> + SpiceMsgcDisplayPreferredVideoCodecType *msg)
> +{
> + gint i;
> + GArray *client_codecs;
> +
> + g_return_val_if_fail(msg->num_of_codecs > 0, TRUE);
> + g_return_val_if_fail(msg->num_of_codecs <
> SPICE_VIDEO_CODEC_TYPE_ENUM_END, TRUE);
> +
This could happen only with duplicate cases.
> + client_codecs = g_array_new(FALSE, FALSE, sizeof(gint));
> + for (i = 0; i < msg->num_of_codecs; i++) {
> + gint video_codec = msg->codecs[i];
> +
> + if (video_codec < SPICE_VIDEO_CODEC_TYPE_MJPEG ||
> + video_codec >= SPICE_VIDEO_CODEC_TYPE_ENUM_END) {
> + spice_warning("Client has sent invalid video-codec (value %d at
> index %d). "
> + "Ignoring preferred video-codec message entirely",
> + video_codec, i);
> + g_array_unref(client_codecs);
> + return TRUE;
> + }
> +
> + g_array_append_val(client_codecs, video_codec);
> + }
but here duplicate cases are not handled.
> + g_clear_pointer(&dcc->priv->client_preferred_video_codecs,
> g_array_unref);
> + dcc->priv->client_preferred_video_codecs = client_codecs;
> +
> + /* New client preference */
> + dcc_update_preferred_video_codecs(dcc);
> + return TRUE;
> +}
> +
> +GArray *dcc_get_preferred_video_codecs(DisplayChannelClient *dcc)
> +{
> + return dcc->priv->preferred_video_codecs;
> +}
> +
> static int dcc_handle_gl_draw_done(DisplayChannelClient *dcc)
> {
> DisplayChannel *display = DCC_TO_DC(dcc);
> @@ -1135,6 +1258,9 @@ int dcc_handle_message(RedChannelClient *rcc, uint32_t
> size, uint16_t type, void
> (SpiceMsgcDisplayPreferredCompression *)msg);
> case SPICE_MSGC_DISPLAY_GL_DRAW_DONE:
> return dcc_handle_gl_draw_done(dcc);
> + case SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE:
> + return dcc_handle_preferred_video_codec_type(dcc,
> + (SpiceMsgcDisplayPreferredVideoCodecType *)msg);
> default:
> return red_channel_client_handle_message(rcc, size, type, msg);
> }
> diff --git a/server/dcc.h b/server/dcc.h
> index 6fb468d..8e7b863 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -204,6 +204,7 @@ uint64_t dcc_get_max_stream_bit_rate(DisplayChannelClient
> *dcc);
> void dcc_set_max_stream_bit_rate(DisplayChannelClient *dcc, uint64_t rate);
> int dcc_config_socket(RedChannelClient *rcc);
> gboolean dcc_is_low_bandwidth(DisplayChannelClient *dcc);
> +GArray *dcc_get_preferred_video_codecs(DisplayChannelClient *dcc);
>
> G_END_DECLS
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 7d3c6e4..de33ef7 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -214,6 +214,7 @@ void display_channel_set_video_codecs(DisplayChannel
> *display, GArray *video_cod
>
> g_clear_pointer(&display->priv->video_codecs, g_array_unref);
> display->priv->video_codecs = g_array_ref(video_codecs);
> + g_object_notify(G_OBJECT(display), "video-codecs");
> }
>
> GArray *display_channel_get_video_codecs(DisplayChannel *display)
> @@ -2061,6 +2062,7 @@ display_channel_constructed(GObject *object)
>
> red_channel_set_cap(channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
> red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION);
> + red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE);
> red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
> }
>
> diff --git a/server/stream.c b/server/stream.c
> index e2cd66e..faa33d6 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -693,7 +693,10 @@ static VideoEncoder*
> dcc_create_video_encoder(DisplayChannelClient *dcc,
> int i;
> GArray *video_codecs;
>
> - video_codecs = display_channel_get_video_codecs(display);
> + video_codecs = dcc_get_preferred_video_codecs(dcc);
> + if (video_codecs == NULL)
> + video_codecs = display_channel_get_video_codecs(display);
> +
This if IMO should go to dcc_get_preferred_video_codecs.
> for (i = 0; i < video_codecs->len; i++) {
> RedVideoCodec* video_codec = &g_array_index (video_codecs,
> RedVideoCodec, i);
>
Frediano
More information about the Spice-devel
mailing list