[spice-server] streaming: Restart streams on video-codec changes

Submitted by Kevin Pouget on July 4, 2019, 8:08 a.m.

Details

Message ID 20190704080841.21403-1-kpouget@redhat.com
State Accepted
Headers show
Series "streaming: Restart streams on video-codec changes" ( rev: 2 ) in Spice

Not browsing as part of any series.

Commit Message

Kevin Pouget July 4, 2019, 8:08 a.m.
Interrupt the video streams when the user changes the preferred
video-codecs (dcc_handle_preferred_video_codec_type) or when the host
admin updates the list of video-codecs allowed
(display_channel_set_video_codecs).

The video streaming will be automatically restarted by spice
video-detection rules.
---
 server/dcc.c             | 2 ++
 server/display-channel.c | 2 ++
 2 files changed, 4 insertions(+)

Patch hide | download patch | download mbox

diff --git a/server/dcc.c b/server/dcc.c
index 71d09b77..86893ffe 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -1198,6 +1198,8 @@  static int dcc_handle_preferred_video_codec_type(DisplayChannelClient *dcc,
 
     /* New client preference */
     dcc_update_preferred_video_codecs(dcc);
+    video_stream_detach_and_stop(DCC_TO_DC(dcc));
+
     return TRUE;
 }
 
diff --git a/server/display-channel.c b/server/display-channel.c
index 4677c261..75266598 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -255,6 +255,8 @@  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");
+
+    video_stream_detach_and_stop(display);
 }
 
 GArray *display_channel_get_video_codecs(DisplayChannel *display)

Comments


> 
> Interrupt the video streams when the user changes the preferred
> video-codecs (dcc_handle_preferred_video_codec_type) or when the host
> admin updates the list of video-codecs allowed
> (display_channel_set_video_codecs).
> 
> The video streaming will be automatically restarted by spice
> video-detection rules.

I suppose it would be more smart to check if the used codec is still
fine and also if a single client wants to change the list of codecs
it would be good to check all clients. On the other hand the list
of codecs is not supposed to be changed much and the support for
multiple clients is something never been production ready and disabled
(only an experimental feature with plenty of bugs)

Acked

Frediano

> ---
>  server/dcc.c             | 2 ++
>  server/display-channel.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index 71d09b77..86893ffe 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -1198,6 +1198,8 @@ static int
> dcc_handle_preferred_video_codec_type(DisplayChannelClient *dcc,
>  
>      /* New client preference */
>      dcc_update_preferred_video_codecs(dcc);
> +    video_stream_detach_and_stop(DCC_TO_DC(dcc));
> +
>      return TRUE;
>  }
>  
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 4677c261..75266598 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -255,6 +255,8 @@ 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");
> +
> +    video_stream_detach_and_stop(display);
>  }
>  
>  GArray *display_channel_get_video_codecs(DisplayChannel *display)

On Thu, Jul 11, 2019 at 10:35 AM Frediano Ziglio <fziglio@redhat.com> wrote:
>
> >
> > Interrupt the video streams when the user changes the preferred
> > video-codecs (dcc_handle_preferred_video_codec_type) or when the host
> > admin updates the list of video-codecs allowed
> > (display_channel_set_video_codecs).
> >
> > The video streaming will be automatically restarted by spice
> > video-detection rules.
>
> I suppose it would be more smart to check if the used codec is still
> fine and also if a single client wants to change the list of codecs
> it would be good to check all clients. On the other hand the list
> of codecs is not supposed to be changed much and the support for
> multiple clients is something never been production ready and disabled
> (only an experimental feature with plenty of bugs)

for my understanding, how is the multi-client supposed to work? they
all see the same screen/outputs, or each of them have a different
seat, fully independent?
in the first case, we should look for the video codecs that are
supported by all clients,
in the second case, I guess the video encoding should (in theory) be
independent for each of the outputs?

but indeed it could have been worth checking if the video codec to use
actually changed, I'll look at it


> Acked
>
> Frediano
>
> > ---
> >  server/dcc.c             | 2 ++
> >  server/display-channel.c | 2 ++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/server/dcc.c b/server/dcc.c
> > index 71d09b77..86893ffe 100644
> > --- a/server/dcc.c
> > +++ b/server/dcc.c
> > @@ -1198,6 +1198,8 @@ static int
> > dcc_handle_preferred_video_codec_type(DisplayChannelClient *dcc,
> >
> >      /* New client preference */
> >      dcc_update_preferred_video_codecs(dcc);
> > +    video_stream_detach_and_stop(DCC_TO_DC(dcc));
> > +
> >      return TRUE;
> >  }
> >
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index 4677c261..75266598 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -255,6 +255,8 @@ 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");
> > +
> > +    video_stream_detach_and_stop(display);
> >  }
> >
> >  GArray *display_channel_get_video_codecs(DisplayChannel *display)