dcc: Create a stream associated with gl_draw for non-gl clients (v5)

For non-gl/remote clients, if there is no stream associated with the DisplayChannel, then we create a new stream. Otherwise, we just update the current stream's timestamp. v2: (suggestions and fixups from Frediano) - Moved the gl_draw_stream object from DCC to DC - Moved the stream initialization code from display_channel_create_stream() into a separate function that is reused when creating gl_draw_stream v3: - Create a new primary surface whenever a new stream gets created v4: - Use nullptr instead of NULL and true instead of TRUE (Frediano) - Create the stream as part of gl scanout instead of gl draw operation so that if would be easily possible to obtain key params such as stride, flags, etc - Store key params such as fd, flags, stride, etc in the stream so that we do not have to look at scanout again v5: (Frediano) - No need to pass scanout object to create_gl_draw_stream as it is always NULL - Don't compute the stream's last_time if drawable is valid - Let the default input_fps be MAX_FPS - Use uint32_t type for stride - Make sure that a newly created stream is provided to all connected clients - When the scanout's drm_dma_buf_fd is associated with a stream, take an additional reference on the fd to ensure that it is not closed when the stream might still be using it Cc: Frediano Ziglio <freddy77@gmail.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Cc: Dongwon Kim <dongwon.kim@intel.com> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
2025-12-26 14:41:25 +00:00 · 2023-03-15 13:28:33 -07:00 · 2023-03-15 13:28:33 -07:00 · 810fa998a1
commit 810fa998a1
parent 52202cc827
5 changed files with 187 additions and 32 deletions
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@ -499,13 +499,18 @@ RedPipeItemPtr dcc_gl_scanout_item_new(RedChannelClient *rcc, void *data, int nu
    if (dcc->is_gl_client()) {
        return red::make_shared<RedGlScanoutUnixItem>();
    } else if (rcc->test_remote_cap(SPICE_DISPLAY_CAP_MULTI_CODEC)) {
-        return RedPipeItemPtr();
+        if (!display_channel_create_gl_draw_stream(DCC_TO_DC(dcc))) {
+            red_channel_warning(rcc->get_channel(),
+                                "Cannot create GL stream");
+            rcc->disconnect();
+        }
    } else {
        red_channel_warning(rcc->get_channel(),
                            "Client does not support GL scanout or multiple codecs");
        rcc->disconnect();
-        return RedPipeItemPtr();
    }
+
+    return RedPipeItemPtr();
 }

 XXX_CAST(RedChannelClient, DisplayChannelClient, DISPLAY_CHANNEL_CLIENT);
@ -523,6 +528,15 @@ RedPipeItemPtr dcc_gl_draw_item_new(RedChannelClient *rcc, void *data, int num)
        return RedPipeItemPtr();
    }

+    if (!dcc->is_gl_client()) {
+        if (!display_channel_update_gl_draw_stream(dcc, draw)) {
+            red_channel_warning(rcc->get_channel(),
+                                "Cannot update GL stream");
+            rcc->disconnect();
+            return RedPipeItemPtr();
+        }
+    }
+
    dcc->priv->gl_draw_ongoing = TRUE;
    auto item = red::make_shared<RedGlDrawItem>();
    item->draw = *draw;
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@ -110,6 +110,7 @@ struct DisplayChannelPrivate
    uint32_t stream_count;
    std::array<VideoStream, NUM_STREAMS> streams_buf;
    VideoStream *free_streams;
+    VideoStream *gl_draw_stream;
    Ring streams;
    std::array<ItemTrace, NUM_TRACE_ITEMS> items_trace;
    uint32_t next_item_trace;
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@ -2172,6 +2172,7 @@ display_channel_new(RedsState *reds, QXLInstance *qxl,
                                         video_codecs, n_surfaces);
    if (display) {
        display_channel_set_stream_video(display.get(), stream_video);
+        display->priv->gl_draw_stream = nullptr;
    }
    return display;
 }
--- a/server/video-stream.cpp
+++ b/server/video-stream.cpp
@ -20,6 +20,7 @@
 #include "display-channel-private.h"
 #include "main-channel-client.h"
 #include "red-client.h"
+#include "red-qxl.h"

 #define FPS_TEST_INTERVAL 1
 #define FOREACH_STREAMS(display, item)                  \
@ -115,6 +116,11 @@ 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->drm_dma_buf_fd > 0) {
+            close(stream->drm_dma_buf_fd);
+            stream->drm_dma_buf_fd = -1;
+        }
    }
    display->priv->streams_size_total -= stream->width * stream->height;
    ring_remove(&stream->link);
@ -366,7 +372,53 @@ static VideoStream *display_channel_stream_try_new(DisplayChannel *display)
    return stream;
 }

-static void display_channel_create_stream(DisplayChannel *display, Drawable *drawable)
+static void display_channel_init_stream(DisplayChannel *display,
+                                        VideoStream *stream,
+                                        Drawable *drawable,
+                                        SpiceRect* src_rect)
+{
+    SpiceBitmap *bitmap;
+    uint64_t duration;
+
+    ring_add(&display->priv->streams, &stream->link);
+    stream->current = drawable;
+    stream->last_time = drawable ? drawable->creation_time :
+                        spice_get_monotonic_time_ns();
+    stream->width = src_rect->right - src_rect->left;
+    stream->height = src_rect->bottom - src_rect->top;
+    stream->dest_area = *src_rect;
+    stream->refs = 1;
+    stream->input_fps = MAX_FPS;
+    stream->num_input_frames = 0;
+    stream->input_fps_start_time = stream->last_time;
+    display->priv->streams_size_total += stream->width * stream->height;
+    display->priv->stream_count++;
+
+    if (drawable) {
+        drawable->stream = stream;
+        stream->dest_area = drawable->red_drawable->bbox;
+        bitmap = &drawable->red_drawable->u.copy.src_bitmap->u.bitmap;
+        stream->top_down = !!(bitmap->flags & SPICE_BITMAP_FLAGS_TOP_DOWN);
+
+        /* Provide an fps estimate the video encoder can use when initializing
+         * based on the frames that lead to the creation of the stream. Round
+         * to the nearest integer, for instance 24 for 23.976.
+         */
+        duration = drawable->creation_time - drawable->first_frame_time;
+        if (duration > NSEC_PER_SEC * drawable->frames_count / MAX_FPS) {
+            stream->input_fps = (NSEC_PER_SEC * drawable->frames_count + duration / 2) / duration;
+        }
+    }
+
+    spice_debug("stream %d %dx%d (%d, %d) (%d, %d) %u fps",
+                display_channel_get_video_stream_id(display, stream), stream->width,
+                stream->height, stream->dest_area.left, stream->dest_area.top,
+                stream->dest_area.right, stream->dest_area.bottom,
+                stream->input_fps);
+}
+
+static void display_channel_create_stream(DisplayChannel *display,
+                                          Drawable *drawable)
 {
    DisplayChannelClient *dcc;
    VideoStream *stream;
@ -381,38 +433,120 @@ static void display_channel_create_stream(DisplayChannel *display, Drawable *dra
    spice_assert(drawable->red_drawable->type == QXL_DRAW_COPY);
    src_rect = &drawable->red_drawable->u.copy.src_area;

-    ring_add(&display->priv->streams, &stream->link);
-    stream->current = drawable;
-    stream->last_time = drawable->creation_time;
-    stream->width = src_rect->right - src_rect->left;
-    stream->height = src_rect->bottom - src_rect->top;
-    stream->dest_area = drawable->red_drawable->bbox;
-    stream->refs = 1;
-    SpiceBitmap *bitmap = &drawable->red_drawable->u.copy.src_bitmap->u.bitmap;
-    stream->top_down = !!(bitmap->flags & SPICE_BITMAP_FLAGS_TOP_DOWN);
-    drawable->stream = stream;
-    /* Provide an fps estimate the video encoder can use when initializing
-     * based on the frames that lead to the creation of the stream. Round to
-     * the nearest integer, for instance 24 for 23.976.
-     */
-    uint64_t duration = drawable->creation_time - drawable->first_frame_time;
-    if (duration > NSEC_PER_SEC * drawable->frames_count / MAX_FPS) {
-        stream->input_fps = (NSEC_PER_SEC * drawable->frames_count + duration / 2) / duration;
-    } else {
-        stream->input_fps = MAX_FPS;
-    }
-    stream->num_input_frames = 0;
-    stream->input_fps_start_time = drawable->creation_time;
-    display->priv->streams_size_total += stream->width * stream->height;
-    display->priv->stream_count++;
+    display_channel_init_stream(display, stream, drawable, src_rect);
    FOREACH_DCC(display, dcc) {
        dcc_create_stream(dcc, stream);
    }
-    spice_debug("stream %d %dx%d (%d, %d) (%d, %d) %u fps",
-                display_channel_get_video_stream_id(display, stream), stream->width,
-                stream->height, stream->dest_area.left, stream->dest_area.top,
-                stream->dest_area.right, stream->dest_area.bottom,
-                stream->input_fps);
+}
+
+bool display_channel_create_gl_draw_stream(DisplayChannel *display)
+{
+    DisplayChannelClient *dcc;
+    QXLInstance* qxl = display->priv->qxl;
+    SpiceMsgDisplayGlScanoutUnix *scanout = red_qxl_get_gl_scanout(qxl);
+    VideoStream *stream;
+    bool ret = false;
+
+    /* If the scanout is NULL, it could mean the scanout is explicitly
+     * (or temporarily) disabled, which is not an error condition.
+     */
+    if (!scanout) {
+        return true;
+    }
+    SpiceRect dest_area = {
+        .left = 0,
+        .top = 0,
+        .right = scanout->width,
+        .bottom = scanout->height
+    };
+
+    /* Let stream take a reference on the fd, so that the fd cannot be
+     * closed while a gl draw is happening. Ideally, this would not happen
+     * as a new gl scanout is not issued until the previous gl draw is
+     * completed but there could be corner cases such as when multiple
+     * new clients are connecting simultaneously.
+     */
+    int fd = dup(scanout->drm_dma_buf_fd);
+    if (fd < 0) {
+        goto err;
+    }
+
+    if (display->priv->surfaces[0]) {
+        display_channel_surface_id_unref(display, 0);
+    }
+    if (!display_channel_create_surface(display, 0, scanout->width,
+                                        scanout->height, scanout->stride,
+                                        SPICE_SURFACE_FMT_32_xRGB,
+                                        nullptr, true, true)) {
+        close(fd);
+        goto err;
+    }
+
+    display_channel_set_monitors_config_to_primary(display);
+    display_channel_push_monitors_config(display);
+    display->pipes_add_empty_msg(SPICE_MSG_DISPLAY_MARK);
+    display->push();
+
+    if (!(stream = display_channel_stream_try_new(display))) {
+        close(fd);
+        goto err;
+    }
+
+    ret = true;
+    display_channel_init_stream(display, stream, nullptr, &dest_area);
+    stream->top_down = (scanout->flags & SPICE_GL_SCANOUT_FLAGS_Y0TOP) ? 0 : 1;
+    stream->stride = scanout->stride;
+    stream->drm_dma_buf_fd = fd;
+    /* This is the upper bound; it should be possible to stream at 60 FPS
+     * with a hardware based encoder.
+     */
+    stream->input_fps = MAX_FPS * 2;
+
+    display->priv->gl_draw_stream = stream;
+    FOREACH_DCC(display, dcc) {
+        dcc_create_stream(dcc, stream);
+    }
+err:
+    red_qxl_put_gl_scanout(qxl, scanout);
+    return ret;
+}
+
+bool display_channel_update_gl_draw_stream(DisplayChannelClient *dcc,
+                                           const SpiceMsgDisplayGlDraw *draw)
+{
+    DisplayChannel *display = DCC_TO_DC(dcc);
+    VideoStream *stream = display->priv->gl_draw_stream;
+    SpiceRect dest_area = {
+        .left = draw->x,
+        .top = draw->y,
+        .right = draw->w,
+        .bottom = draw->h
+    };
+
+    if (stream) {
+        stream->width = draw->w;
+        stream->height = draw->h;
+        stream->dest_area = dest_area;
+        stream->last_time = spice_get_monotonic_time_ns();
+    } else {
+        if (!display_channel_create_gl_draw_stream(display)) {
+            return false;
+        }
+        stream = display->priv->gl_draw_stream;
+    }
+
+    int stream_id = display_channel_get_video_stream_id(display, stream);
+    VideoStreamAgent *agent = dcc_get_video_stream_agent(dcc, stream_id);
+
+    /* Just a sanity check to make sure that an encoder was created */
+    if (!agent->video_encoder) {
+        dcc_create_stream(dcc, stream);
+        if (!agent->video_encoder) {
+            return false;
+        }
+    }
+
+    return true;
 }

 // returns whether a stream was created
--- a/server/video-stream.h
+++ b/server/video-stream.h
@ -113,6 +113,8 @@ struct VideoStream {
    red_time_t last_time;
    int width;
    int height;
+    uint32_t stride;
+    int drm_dma_buf_fd;
    SpiceRect dest_area;
    int top_down;
    VideoStream *next;
@ -124,6 +126,9 @@ struct VideoStream {
 };

 void display_channel_init_video_streams(DisplayChannel *display);
+bool display_channel_create_gl_draw_stream(DisplayChannel *display);
+bool display_channel_update_gl_draw_stream(DisplayChannelClient *dcc,
+                                           const SpiceMsgDisplayGlDraw *draw);
 void video_stream_stop(DisplayChannel *display, VideoStream *stream);
 void video_stream_trace_update(DisplayChannel *display, Drawable *drawable);
 void video_stream_maintenance(DisplayChannel *display, Drawable *candidate,