Il giorno gio 16 mar 2023 alle ore 06:05 Vivek Kasireddy
<vivek.kasireddy@xxxxxxxxx> ha scritto:
>
> Before stopping a stream, we need to ensure that there is no ongoing
> gl_draw operation; otherwise, we may not get another opportunity to
> create a new stream if the current one gets stopped. And, once the
> stream is stopped, we need to clear the gl_draw_stream pointer
> associated with the relevant client (dcc).
>
> Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> Cc: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> Cc: Dongwon Kim <dongwon.kim@xxxxxxxxx>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> ---
> server/video-stream.cpp | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/server/video-stream.cpp b/server/video-stream.cpp
> index 03a7d68d..b7f3d2c5 100644
> --- a/server/video-stream.cpp
> +++ b/server/video-stream.cpp
> @@ -18,6 +18,7 @@
>
> #include "video-stream.h"
> #include "display-channel-private.h"
> +#include "dcc-private.h"
This smells bad from design, it loose the "private" meaning.
> #include "main-channel-client.h"
> #include "red-client.h"
>
> @@ -115,6 +116,10 @@ void video_stream_stop(DisplayChannel *display, VideoStream *stream)
> }
> dcc->pipe_add(video_stream_destroy_item_new(stream_agent));
> video_stream_agent_stats_print(stream_agent);
> +
> + if (stream == dcc->priv->gl_draw_stream) {
> + dcc->priv->gl_draw_stream = nullptr;
It looks like a "weak" pointer. Why is that? Maybe the bound between
stream and time expiration? Maybe for that specific stream time
expiration does not make much sense?
> + }
> }
> display->priv->streams_size_total -= stream->width * stream->height;
> ring_remove(&stream->link);
> @@ -974,6 +979,19 @@ void video_stream_detach_and_stop(DisplayChannel *display)
> }
> }
>
> +static bool is_stream_gl_draw(VideoStream *stream, DisplayChannel *display)
> +{
> + DisplayChannelClient *dcc;
> +
> + FOREACH_DCC(display, dcc) {
> + if (stream == dcc->priv->gl_draw_stream &&
> + dcc->priv->gl_draw_ongoing) {
> + return TRUE;
Minor: true
> + }
> + }
> + return FALSE;
Minor: false
Maybe use a boolean flag in VideoStream instead ?
> +}
> +
> void video_stream_timeout(DisplayChannel *display)
> {
> Ring *ring = &display->priv->streams;
> @@ -984,7 +1002,8 @@ void video_stream_timeout(DisplayChannel *display)
> while (item) {
> VideoStream *stream = SPICE_CONTAINEROF(item, VideoStream, link);
> item = ring_next(ring, item);
> - if (now >= (stream->last_time + RED_STREAM_TIMEOUT)) {
> + if (now >= (stream->last_time + RED_STREAM_TIMEOUT) &&
> + !is_stream_gl_draw(stream, display)) {
> detach_video_stream_gracefully(display, stream, nullptr);
> video_stream_stop(display, stream);
> }
Frediano