From 82bd40dcbcc3601da755678778f033bd9a30286d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Thu, 4 May 2023 12:31:41 +0200 Subject: [PATCH] screen-cast/src: Never dequeue pw_buffer's we refuse to record to The DMA buffer paths vs MemFd paths differ slightly in when content is recorded. This was in some places done by trying to record but bail if the dequeued buffer had the wrong type. This is problematic for two reasons: we'd update the timestamp even if we refused to record, making the follow-up attempt fail, and we'd dequeue and queue buffers that didn't get any content, meaning the receiving end would see empty buffers potentially with only cursor updates. Fix this by keeping track if a stream is DMA buffer able or not, and don't attempt to record at all in the places we would previously require DMA buffers. This avoids both issues: we don't dequeue/queue pw_buffers that we refuse to record to, and we won't update the recorded timestamp when we didn't intend to record to begin with. Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/2783 Part-of: --- .../meta-screen-cast-monitor-stream-src.c | 25 ++++++++++++------- src/backends/meta-screen-cast-stream-src.c | 22 ++++++++++------ src/backends/meta-screen-cast-stream-src.h | 3 ++- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/backends/meta-screen-cast-monitor-stream-src.c b/src/backends/meta-screen-cast-monitor-stream-src.c index efb458067e..073a4d101f 100644 --- a/src/backends/meta-screen-cast-monitor-stream-src.c +++ b/src/backends/meta-screen-cast-monitor-stream-src.c @@ -158,8 +158,8 @@ stage_painted (MetaStage *stage, MetaScreenCastMonitorStreamSrc *monitor_src = META_SCREEN_CAST_MONITOR_STREAM_SRC (user_data); MetaScreenCastStreamSrc *src = META_SCREEN_CAST_STREAM_SRC (monitor_src); - MetaScreenCastRecordResult record_result; - MetaScreenCastRecordFlag flags; + MetaScreenCastRecordResult record_result = + META_SCREEN_CAST_RECORD_RESULT_RECORDED_NOTHING; int64_t presentation_time_us; if (monitor_src->maybe_record_idle_id) @@ -168,12 +168,16 @@ stage_painted (MetaStage *stage, if (!clutter_frame_get_target_presentation_time (frame, &presentation_time_us)) presentation_time_us = g_get_monotonic_time (); - flags = META_SCREEN_CAST_RECORD_FLAG_DMABUF_ONLY; - record_result = - meta_screen_cast_stream_src_maybe_record_frame_with_timestamp (src, - flags, - NULL, - presentation_time_us); + if (meta_screen_cast_stream_src_uses_dma_bufs (src)) + { + MetaScreenCastRecordFlag flags = META_SCREEN_CAST_RECORD_FLAG_NONE; + + record_result = + meta_screen_cast_stream_src_maybe_record_frame_with_timestamp (src, + flags, + NULL, + presentation_time_us); + } if (!(record_result & META_SCREEN_CAST_RECORD_RESULT_RECORDED_FRAME)) { @@ -200,13 +204,16 @@ before_stage_painted (MetaStage *stage, if (monitor_src->maybe_record_idle_id) return; + if (!meta_screen_cast_stream_src_uses_dma_bufs (src)) + return; + if (!clutter_stage_view_peek_scanout (view)) return; if (!clutter_frame_get_target_presentation_time (frame, &presentation_time_us)) presentation_time_us = g_get_monotonic_time (); - flags = META_SCREEN_CAST_RECORD_FLAG_DMABUF_ONLY; + flags = META_SCREEN_CAST_RECORD_FLAG_NONE; meta_screen_cast_stream_src_maybe_record_frame_with_timestamp (src, flags, NULL, diff --git a/src/backends/meta-screen-cast-stream-src.c b/src/backends/meta-screen-cast-stream-src.c index 91a8afab47..94fc222e43 100644 --- a/src/backends/meta-screen-cast-stream-src.c +++ b/src/backends/meta-screen-cast-stream-src.c @@ -107,6 +107,7 @@ typedef struct _MetaScreenCastStreamSrcPrivate int64_t last_frame_timestamp_us; guint follow_up_frame_source_id; + gboolean uses_dma_bufs; GHashTable *dmabuf_handles; cairo_region_t *redraw_clip; @@ -513,15 +514,9 @@ do_record_frame (MetaScreenCastStreamSrc *src, { MetaScreenCastStreamSrcPrivate *priv = meta_screen_cast_stream_src_get_instance_private (src); - gboolean dmabuf_only; - dmabuf_only = flags & META_SCREEN_CAST_RECORD_FLAG_DMABUF_ONLY; - if (dmabuf_only && spa_buffer->datas[0].type != SPA_DATA_DmaBuf) - return FALSE; - - if (!dmabuf_only && - (spa_buffer->datas[0].data || - spa_buffer->datas[0].type == SPA_DATA_MemFd)) + if (spa_buffer->datas[0].data || + spa_buffer->datas[0].type == SPA_DATA_MemFd) { int width = priv->video_format.size.width; int height = priv->video_format.size.height; @@ -1058,6 +1053,8 @@ on_stream_add_buffer (void *data, dmabuf_handle = NULL; } + priv->uses_dma_bufs = !!dmabuf_handle; + if (dmabuf_handle) { meta_topic (META_DEBUG_SCREEN_CAST, @@ -1595,3 +1592,12 @@ meta_screen_cast_stream_src_class_init (MetaScreenCastStreamSrcClass *klass) NULL, NULL, NULL, G_TYPE_NONE, 0); } + +gboolean +meta_screen_cast_stream_src_uses_dma_bufs (MetaScreenCastStreamSrc *src) +{ + MetaScreenCastStreamSrcPrivate *priv = + meta_screen_cast_stream_src_get_instance_private (src); + + return priv->uses_dma_bufs; +} diff --git a/src/backends/meta-screen-cast-stream-src.h b/src/backends/meta-screen-cast-stream-src.h index 63058f2c35..a15ca54f15 100644 --- a/src/backends/meta-screen-cast-stream-src.h +++ b/src/backends/meta-screen-cast-stream-src.h @@ -41,7 +41,6 @@ typedef enum _MetaScreenCastRecordFlag { META_SCREEN_CAST_RECORD_FLAG_NONE = 0, META_SCREEN_CAST_RECORD_FLAG_CURSOR_ONLY = 1 << 0, - META_SCREEN_CAST_RECORD_FLAG_DMABUF_ONLY = 1 << 1, } MetaScreenCastRecordFlag; typedef enum _MetaScreenCastRecordResult @@ -132,4 +131,6 @@ void meta_screen_cast_stream_src_set_cursor_sprite_metadata (MetaScreenCastStrea float scale, MetaMonitorTransform transform); +gboolean meta_screen_cast_stream_src_uses_dma_bufs (MetaScreenCastStreamSrc *src); + #endif /* META_SCREEN_CAST_STREAM_SRC_H */ -- GitLab