[Spice-devel] [spice v1 10/10] dcc: handle preferred video codec message

[Victor Toso]

Hi,

On Mon, Oct 24, 2016 at 11:21:24AM -0400, Frediano Ziglio wrote:
> > 
> > From: Victor Toso <me at victortoso.com>
> > 
> > SPICE_MSGC_DISPLAY_PREFERRED_VIDEO_CODEC_TYPE can be used to set the
> > preferred order of video codecs to be used based in client's
> > preference.
> > 
> > A new function is introduced in reds.c to update its video codec:
> > reds_set_video_codecs_from_client_preferred_msg() - it only update the
> > rank values of existing video codecs.
> > 
> > It is important to note that this message will not _immediately_
> > change a ongoing streaming; It might change the video codec on newly
> > created streams.
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  server/dcc.c        | 13 +++++++++++++
> >  server/red-worker.c |  1 +
> >  server/reds.c       | 35 +++++++++++++++++++++++++++++++++++
> >  server/reds.h       |  3 +++
> >  4 files changed, 52 insertions(+)
> > 
> > diff --git a/server/dcc.c b/server/dcc.c
> > index 5c05c3b..c86f8db 100644
> > --- a/server/dcc.c
> > +++ b/server/dcc.c
> > @@ -1097,6 +1097,16 @@ static int
> > dcc_handle_preferred_compression(DisplayChannelClient *dcc,
> >      return TRUE;
> >  }
> >  
> > +static int dcc_handle_preferred_video_codec_type(DisplayChannelClient *dcc,
> > +         SpiceMsgcDisplayPreferredVideoCodecType *msg)
> > +{
> > +    RedsState *reds;
> > +
> > +    reds =
> > red_channel_get_server(red_channel_client_get_channel(&dcc->parent));
> > +    reds_set_video_codecs_from_client_preferred_msg(reds, msg->codec_ranks,
> > msg->num_of_codecs);
> > +    return TRUE;
> > +}
> > +
> >  static int dcc_handle_gl_draw_done(DisplayChannelClient *dcc)
> >  {
> >      DisplayChannel *display = DCC_TO_DC(dcc);
> > @@ -1127,6 +1137,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/red-worker.c b/server/red-worker.c
> > index 40e58f2..8e8be5b 100644
> > --- a/server/red-worker.c
> > +++ b/server/red-worker.c
> > @@ -1384,6 +1384,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> >      red_channel_register_client_cbs(channel, client_display_cbs,
> >      dispatcher);
> >      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);
> >      reds_register_channel(reds, channel);
> >  
> > diff --git a/server/reds.c b/server/reds.c
> > index 6dbc010..28acddd 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -3649,6 +3649,41 @@ static void reds_set_video_codecs(RedsState *reds,
> > GArray *video_codecs)
> >      reds->config->video_codecs = video_codecs;
> >  }
> >  
> > +void reds_set_video_codecs_from_client_preferred_msg(RedsState *reds,
> > +        SpiceVideoCodecPreferredRank *codec_ranks, guint32 codec_ranks_len)
> > +{
> > +    gint i;
> > +    GArray *video_codecs;
> > +
> > +    /* Dup the current array */
> > +    video_codecs = g_array_new(FALSE, FALSE, sizeof(RedVideoCodec));
> > +    for (i = 0; i < reds->config->video_codecs->len; i++) {
> > +        RedVideoCodec codec = g_array_index(reds->config->video_codecs,
> > RedVideoCodec, i);
> > +
> > +        /* Only one type of codec should be set as preferred - reset to
> > sofware decoder */
> > +        if (codec.rank == SPICE_VIDEO_CODEC_RANK_PREFERRED)
> > +            codec.rank = SPICE_VIDEO_CODEC_RANK_SOFTWARE_DECODER;
> > +
> > +        g_array_append_val(video_codecs, codec);
> > +    }
> > +
> > +    /* Update the rank based in the information given by client */
> > +    for (i = 0; i < codec_ranks_len; i++) {
> > +        gint j;
> > +
> > +        for (j = 0; j < video_codecs->len; j++) {
> > +            RedVideoCodec *codec = &g_array_index(video_codecs,
> > RedVideoCodec, j);
> > +
> > +            if (codec_ranks[i].type != codec->type)
> > +                continue;
> > +
> > +            codec->rank = codec_ranks[i].rank;
> > +        }
> > +    }
> > +    reds_set_video_codecs(reds, video_codecs);
> > +    reds_on_vc_change(reds);
> > +}
> > +
>
> This assume that all the clients have the same ranking.
> If a client which support ranking connect before a client without
> ranking the second connection is affected by the first.

That's true, buggy.

> The ranking should be done by client.

Yes, I'll check how to change that.


> Also this is not changing the order of the codecs so server will pick
> up the codec not taking into account the ranking.

The order was done in a previous patch. Currently
reds_set_video_codecs() does the sort.

Cheers,
  toso

> 
> >  static void reds_set_video_codecs_from_string(RedsState *reds, const char
> >  *codecs)
> >  {
> >      char *encoder_name, *codec_name, *codec_rank;
> > diff --git a/server/reds.h b/server/reds.h
> > index cd62fc1..e3afe42 100644
> > --- a/server/reds.h
> > +++ b/server/reds.h
> > @@ -94,6 +94,9 @@ void reds_on_main_channel_migrate(RedsState *reds,
> > MainChannelClient *mcc);
> >  void reds_set_client_mm_time_latency(RedsState *reds, RedClient *client,
> >  uint32_t latency);
> >  uint32_t reds_get_streaming_video(const RedsState *reds);
> >  GArray* reds_get_video_codecs(const RedsState *reds);
> > +void reds_set_video_codecs_from_client_preferred_msg(RedsState *reds,
> > +
> > SpiceVideoCodecPreferredRank
> > *codec_ranks,
> > +                                                     guint32
> > codec_ranks_len);
> >  spice_wan_compression_t reds_get_jpeg_state(const RedsState *reds);
> >  spice_wan_compression_t reds_get_zlib_glz_state(const RedsState *reds);
> >  SpiceCoreInterfaceInternal* reds_get_core_interface(RedsState *reds);
> 
> Frediano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161024/f0d95663/attachment-0001.sig>