Re: [spice v10] dcc: handle preferred video codec message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> From: Victor Toso <me@xxxxxxxxxxxxxx>
> 
> [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@xxxxxxxxxx>
> ---
>  server/dcc-private.h     |   6 +++
>  server/dcc.c             | 122
>  +++++++++++++++++++++++++++++++++++++++++++++++
>  server/dcc.h             |   1 +
>  server/display-channel.c |   2 +
>  server/stream.c          |   3 +-
>  5 files changed, 132 insertions(+), 2 deletions(-)
> 
> diff --git a/server/dcc-private.h b/server/dcc-private.h
> index 64b32a7..242af10 100644
> --- a/server/dcc-private.h
> +++ b/server/dcc-private.h
> @@ -51,6 +51,12 @@ struct DisplayChannelClientPrivate
>          int num_pixmap_cache_items;
>      } send_data;
>  
> +    /* Host preferred video-codec order sorted with client preferred */
> +    GArray *preferred_video_codecs;
> +    /* Array with SPICE_VIDEO_CODEC_TYPE_ENUM_END elements, with the client
> +     * preference order (index) as value */
> +    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 373dad9..bdd13db 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,116 @@ 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)
> +{
> +    const RedVideoCodec *a = a_pointer;
> +    const RedVideoCodec *b = b_pointer;
> +    GArray *client_pref = user_data;
> +
> +    return (g_array_index(client_pref, gint, b->type) -
> +            g_array_index(client_pref, gint, a->type));
> +}
> +
> +static void dcc_update_preferred_video_codecs(DisplayChannelClient *dcc)
> +{
> +    guint i;
> +    GArray *video_codecs, *server_codecs;
> +    GString *msg;
> +
> +    server_codecs = display_channel_get_video_codecs(DCC_TO_DC(dcc));
> +    spice_return_if_fail(server_codecs != NULL);
> +
> +    /* Copy current host preference */
> +    video_codecs = g_array_sized_new(FALSE, FALSE, sizeof(RedVideoCodec),
> server_codecs->len);
> +    g_array_append_vals(video_codecs, server_codecs->data,
> server_codecs->len);
> +
> +    /* 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_debug("%s", msg->str);
> +    g_string_free(msg, TRUE);
> +}
> +
> +static void on_display_video_codecs_update(GObject *gobject,
> +        GParamSpec *pspec, gpointer user_data)

Arguments are not indented as usual (here and below).

> +{
> +    DisplayChannelClient *dcc = DISPLAY_CHANNEL_CLIENT(user_data);
> +
> +    /* Only worry about video-codecs update if client has sent
> +     * SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE */
> +    if (dcc->priv->client_preferred_video_codecs == NULL) {
> +        return;
> +    }
> +
> +    /* New host preference */
> +    dcc_update_preferred_video_codecs(dcc);
> +}
> +
> +static int dcc_handle_preferred_video_codec_type(DisplayChannelClient *dcc,
> +         SpiceMsgcDisplayPreferredVideoCodecType *msg)
> +{
> +    gint i, len;
> +    gint indexes[SPICE_VIDEO_CODEC_TYPE_ENUM_END] = { 0 };
> +    GArray *client;
> +
> +    g_return_val_if_fail(msg->num_of_codecs > 0, TRUE);
> +
> +    for (len = 0, 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_debug("Client has sent unknow video-codec (value %d at
> index %d). "
> +                        "Ignoring as server can't handle it",
> +                         video_codec, i);
> +            continue;
> +        }
> +
> +        if (indexes[video_codec] != 0)
> +            continue;
> +

Missing brackets.

> +        len++;
> +        indexes[video_codec] = len;
> +    }

Previously the codecs not in the preferred list got last positions,
now they get first ones.
Initializing the indexes array to SPICE_VIDEO_CODEC_TYPE_ENUM_END
(or other max value) would sort them as before (which I think should
be the right order).

> +    client = g_array_sized_new(FALSE, FALSE, sizeof(gint),
> SPICE_VIDEO_CODEC_TYPE_ENUM_END);
> +    g_array_append_vals(client, indexes, SPICE_VIDEO_CODEC_TYPE_ENUM_END);
> +
> +    g_clear_pointer(&dcc->priv->client_preferred_video_codecs,
> g_array_unref);
> +    dcc->priv->client_preferred_video_codecs = client;
> +
> +    /* New client preference */
> +    dcc_update_preferred_video_codecs(dcc);
> +    return TRUE;
> +}
> +
> +GArray *dcc_get_preferred_video_codecs_for_encoding(DisplayChannelClient
> *dcc)
> +{
> +    if (dcc->priv->preferred_video_codecs != NULL) {
> +        return dcc->priv->preferred_video_codecs;
> +    }
> +    return display_channel_get_video_codecs(DCC_TO_DC(dcc));
> +}
> +
>  static int dcc_handle_gl_draw_done(DisplayChannelClient *dcc)
>  {
>      DisplayChannel *display = DCC_TO_DC(dcc);
> @@ -1135,6 +1254,9 @@ int dcc_handle_message(RedChannelClient *rcc, uint16_t
> type, uint32_t size, 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, type, size, msg);
>      }
> diff --git a/server/dcc.h b/server/dcc.h
> index ebdbb8d..37bd3e9 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_for_encoding(DisplayChannelClient
> *dcc);
>  

Is the _for_encoding suffix not too much?

>  G_END_DECLS
>  
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 288969c..85df19d 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)
> @@ -2057,6 +2058,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..f1ec560 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -687,13 +687,12 @@ static VideoEncoder*
> dcc_create_video_encoder(DisplayChannelClient *dcc,
>                                                uint64_t starting_bit_rate,
>                                                VideoEncoderRateControlCbs
>                                                *cbs)
>  {
> -    DisplayChannel *display = DCC_TO_DC(dcc);
>      RedChannelClient *rcc = RED_CHANNEL_CLIENT(dcc);
>      int client_has_multi_codec = red_channel_client_test_remote_cap(rcc,
>      SPICE_DISPLAY_CAP_MULTI_CODEC);
>      int i;
>      GArray *video_codecs;
>  
> -    video_codecs = display_channel_get_video_codecs(display);
> +    video_codecs = dcc_get_preferred_video_codecs_for_encoding(dcc);
>      for (i = 0; i < video_codecs->len; i++) {
>          RedVideoCodec* video_codec = &g_array_index (video_codecs,
>          RedVideoCodec, i);
>  

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]