Hello Uri,
On Thu, Jun 27, 2019 at 11:04 AM Uri Lublin <uril@xxxxxxxxxx> wrote:
>
> On 6/27/19 11:40 AM, Frediano Ziglio wrote:
> >>
> >> Interrupt/restart 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)
>
> Hi,
>
> This patch only stops the video stream.
> A new stream will be started automatically according to spice rules
> (video-stream detection).
indeed, I wasn't sure about the wording, because the purpose of the
patch is to trigger a stream restart, it's not to stop it, so I don't
know which word to put in the commit title! (I'll add your sentence in
the body anyway to clarify)
> >> ---
> >> server/dcc.c | 2 ++
> >> server/display-channel.c | 2 ++
> >> server/video-stream.c | 5 +++++
> >> 3 files changed, 9 insertions(+)
> >>
> >> diff --git a/server/dcc.c b/server/dcc.c
> >> index a94e27e8..0deeed88 100644
> >> --- a/server/dcc.c
> >> +++ b/server/dcc.c
> >> @@ -1201,6 +1201,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 071c0140..4b8e6e90 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)
> >> diff --git a/server/video-stream.c b/server/video-stream.c
> >> index 4ac25af8..f227713b 100644
> >> --- a/server/video-stream.c
> >> +++ b/server/video-stream.c
> >> @@ -925,6 +925,11 @@ void video_stream_detach_and_stop(DisplayChannel
> >> *display)
> >> RingItem *stream_item;
> >>
> >> spice_debug("trace");
> >> +
> >> + if (!ring_is_initialized(&display->priv->streams)) {
> >> + return;
> >> + }
> >> +
> >
> > If this happens I would consider a program error and I would abort().
> >
> >> while ((stream_item = ring_get_head(&display->priv->streams))) {
>
> When the ring is empty, ring_get_head returns NULL, and the
> while loop exits immediately. So this case is already covered.
> (It also aborts (spice_assert) if the the ring is "not initialized")
yes, it was a bug that the stream was uninitialized at this stage;
hence ring_is_empty was aborting the execution
I will let video_stream_detach_and_stop untouched
thanks,
Kevin
> >> VideoStream *stream = SPICE_CONTAINEROF(stream_item, VideoStream,
> >> link);
> >>
> >
> > Frediano
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel