[PATCH v6 2/5] dcc: Create a stream associated with gl_draw for non-gl clients (v5)
Vivek Kasireddy
vivek.kasireddy at intel.com
Mon Mar 4 10:45:44 UTC 2024
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 at gmail.com>
Cc: Gerd Hoffmann <kraxel at redhat.com>
Cc: Marc-André Lureau <marcandre.lureau at redhat.com>
Cc: Dongwon Kim <dongwon.kim at intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
---
server/dcc.cpp | 18 ++-
server/display-channel-private.h | 1 +
server/display-channel.cpp | 1 +
server/video-stream.cpp | 196 ++++++++++++++++++++++++++-----
server/video-stream.h | 5 +
5 files changed, 188 insertions(+), 33 deletions(-)
diff --git a/server/dcc.cpp b/server/dcc.cpp
index ca73470a..ace2fdbb 100644
--- 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;
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 04ac2c0d..e0693f54 100644
--- 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;
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 4bd0cf41..53a4724d 100644
--- 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;
}
diff --git a/server/video-stream.cpp b/server/video-stream.cpp
index 056d0c31..29143c24 100644
--- 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,48 +372,44 @@ 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)
{
- DisplayChannelClient *dcc;
- VideoStream *stream;
- SpiceRect* src_rect;
-
- spice_assert(!drawable->stream);
-
- if (!(stream = display_channel_stream_try_new(display))) {
- return;
- }
-
- spice_assert(drawable->red_drawable->type == QXL_DRAW_COPY);
- src_rect = &drawable->red_drawable->u.copy.src_area;
+ SpiceBitmap *bitmap;
+ uint64_t duration;
ring_add(&display->priv->streams, &stream->link);
stream->current = drawable;
- stream->last_time = drawable->creation_time;
+ 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 = drawable->red_drawable->bbox;
+ stream->dest_area = *src_rect;
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->input_fps = MAX_FPS;
stream->num_input_frames = 0;
- stream->input_fps_start_time = drawable->creation_time;
+ stream->input_fps_start_time = stream->last_time;
display->priv->streams_size_total += stream->width * stream->height;
display->priv->stream_count++;
- FOREACH_DCC(display, dcc) {
- dcc_create_stream(dcc, stream);
+
+ 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,
@@ -415,6 +417,138 @@ static void display_channel_create_stream(DisplayChannel *display, Drawable *dra
stream->input_fps);
}
+static void display_channel_create_stream(DisplayChannel *display,
+ Drawable *drawable)
+{
+ DisplayChannelClient *dcc;
+ VideoStream *stream;
+ SpiceRect* src_rect;
+
+ spice_assert(!drawable->stream);
+
+ if (!(stream = display_channel_stream_try_new(display))) {
+ return;
+ }
+
+ spice_assert(drawable->red_drawable->type == QXL_DRAW_COPY);
+ src_rect = &drawable->red_drawable->u.copy.src_area;
+
+ display_channel_init_stream(display, stream, drawable, src_rect);
+ FOREACH_DCC(display, dcc) {
+ dcc_create_stream(dcc, stream);
+ }
+}
+
+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
static bool video_stream_add_frame(DisplayChannel *display,
Drawable *frame_drawable,
diff --git a/server/video-stream.h b/server/video-stream.h
index 23b44ff5..d1f5667b 100644
--- 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,
--
2.43.0
More information about the Spice-devel
mailing list