From cfbd6b0884db2eab893831e7bde5cfe640fe52db Mon Sep 17 00:00:00 2001 From: Jan Grulich Date: Fri, 10 May 2024 16:05:03 +0200 Subject: [PATCH] Video capture PipeWire: drop corrupted PipeWire buffers Use SPA_CHUNK_FLAG_CORRUPTED and SPA_META_HEADER_FLAG_CORRUPTED flags to determine corrupted buffers or corrupted buffer data. We used to only rely on compositors setting chunk->size, but this doesn't make sense for dmabufs where they have to make up arbitrary values. It also looks this is not reliable and can cause glitches as we end up processing corrupted buffers. Bug: webrtc:338232699 Change-Id: Ida0c6a5e7a37e19598c6d5884726200f81b94962 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349881 Commit-Queue: Mark Foltz Reviewed-by: Mark Foltz Reviewed-by: Alexander Cooper Cr-Commit-Position: refs/heads/main@{#42292} --- .../linux/wayland/shared_screencast_stream.cc | 28 ++++++++++++++- .../linux/wayland/shared_screencast_stream.h | 3 ++ .../shared_screencast_stream_unittest.cc | 34 ++++++++++++++++++- .../test/test_screencast_stream_provider.cc | 29 ++++++++++++---- .../test/test_screencast_stream_provider.h | 4 ++- 5 files changed, 88 insertions(+), 10 deletions(-) diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc index 6998d655c2..4e042ef79e 100644 --- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc +++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc @@ -323,6 +323,19 @@ void SharedScreenCastStreamPrivate::OnStreamProcess(void* data) { return; } + struct spa_meta_header* header = + static_cast(spa_buffer_find_meta_data( + buffer->buffer, SPA_META_Header, sizeof(*header))); + if (header && (header->flags & SPA_META_HEADER_FLAG_CORRUPTED)) { + RTC_LOG(LS_INFO) << "Dropping corrupted buffer"; + if (that->observer_) { + that->observer_->OnBufferCorruptedMetadata(); + } + // Queue buffer for reuse; it will not be processed further. + pw_stream_queue_buffer(that->pw_stream_, buffer); + return; + } + that->ProcessBuffer(buffer); pw_stream_queue_buffer(that->pw_stream_, buffer); @@ -709,7 +722,20 @@ void SharedScreenCastStreamPrivate::ProcessBuffer(pw_buffer* buffer) { } } - if (spa_buffer->datas[0].chunk->size == 0) { + if (spa_buffer->datas[0].chunk->flags & SPA_CHUNK_FLAG_CORRUPTED) { + RTC_LOG(LS_INFO) << "Dropping buffer with corrupted or missing data"; + if (observer_) { + observer_->OnBufferCorruptedData(); + } + return; + } + + if (spa_buffer->datas[0].type == SPA_DATA_MemFd && + spa_buffer->datas[0].chunk->size == 0) { + RTC_LOG(LS_INFO) << "Dropping buffer with empty data"; + if (observer_) { + observer_->OnEmptyBuffer(); + } return; } diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.h b/modules/desktop_capture/linux/wayland/shared_screencast_stream.h index f57e22cb69..1b00e27476 100644 --- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.h +++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.h @@ -35,6 +35,9 @@ class RTC_EXPORT SharedScreenCastStream virtual void OnCursorShapeChanged() = 0; virtual void OnDesktopFrameChanged() = 0; virtual void OnFailedToProcessBuffer() = 0; + virtual void OnBufferCorruptedMetadata() = 0; + virtual void OnBufferCorruptedData() = 0; + virtual void OnEmptyBuffer() = 0; virtual void OnStreamConfigured() = 0; virtual void OnFrameRateChanged(uint32_t frame_rate) = 0; diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream_unittest.cc b/modules/desktop_capture/linux/wayland/shared_screencast_stream_unittest.cc index 6a72edd025..72ce01eb26 100644 --- a/modules/desktop_capture/linux/wayland/shared_screencast_stream_unittest.cc +++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream_unittest.cc @@ -55,6 +55,9 @@ class PipeWireStreamTest : public ::testing::Test, MOCK_METHOD(void, OnCursorShapeChanged, (), (override)); MOCK_METHOD(void, OnDesktopFrameChanged, (), (override)); MOCK_METHOD(void, OnFailedToProcessBuffer, (), (override)); + MOCK_METHOD(void, OnBufferCorruptedMetadata, (), (override)); + MOCK_METHOD(void, OnBufferCorruptedData, (), (override)); + MOCK_METHOD(void, OnEmptyBuffer, (), (override)); MOCK_METHOD(void, OnStreamConfigured, (), (override)); MOCK_METHOD(void, OnFrameRateChanged, (uint32_t), (override)); @@ -103,8 +106,9 @@ TEST_F(PipeWireStreamTest, TestPipeWire) { waitStartStreamingEvent.Wait(kShortWait); rtc::Event frameRetrievedEvent; - EXPECT_CALL(*this, OnFrameRecorded).Times(3); + EXPECT_CALL(*this, OnFrameRecorded).Times(6); EXPECT_CALL(*this, OnDesktopFrameChanged) + .Times(3) .WillRepeatedly([&frameRetrievedEvent] { frameRetrievedEvent.Set(); }); // Record a frame in FakePipeWireStream @@ -156,6 +160,34 @@ TEST_F(PipeWireStreamTest, TestPipeWire) { frameRetrievedEvent.Wait(kShortWait); EXPECT_EQ(RgbaColor(frame->data()), blue_color); + // Check we don't process faulty buffers + rtc::Event corruptedMetadataFrameEvent; + EXPECT_CALL(*this, OnBufferCorruptedMetadata) + .WillOnce([&corruptedMetadataFrameEvent] { + corruptedMetadataFrameEvent.Set(); + }); + + test_screencast_stream_provider_->RecordFrame( + blue_color, TestScreenCastStreamProvider::CorruptedMetadata); + corruptedMetadataFrameEvent.Wait(kShortWait); + + rtc::Event corruptedDataFrameEvent; + EXPECT_CALL(*this, OnBufferCorruptedData) + .WillOnce([&corruptedDataFrameEvent] { corruptedDataFrameEvent.Set(); }); + + test_screencast_stream_provider_->RecordFrame( + blue_color, TestScreenCastStreamProvider::CorruptedData); + corruptedDataFrameEvent.Wait(kShortWait); + + rtc::Event emptyFrameEvent; + EXPECT_CALL(*this, OnEmptyBuffer).WillOnce([&emptyFrameEvent] { + emptyFrameEvent.Set(); + }); + + test_screencast_stream_provider_->RecordFrame( + blue_color, TestScreenCastStreamProvider::EmptyData); + emptyFrameEvent.Wait(kShortWait); + // Update stream parameters. EXPECT_CALL(*this, OnFrameRateChanged(0)) .Times(1) diff --git a/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.cc b/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.cc index ee5c17e7d7..10551047a9 100644 --- a/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.cc +++ b/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.cc @@ -131,7 +131,8 @@ TestScreenCastStreamProvider::~TestScreenCastStreamProvider() { } } -void TestScreenCastStreamProvider::RecordFrame(RgbaColor rgba_color) { +void TestScreenCastStreamProvider::RecordFrame(RgbaColor rgba_color, + FrameDefect frame_defect) { const char* error; if (pw_stream_get_state(pw_stream_, &error) != PW_STREAM_STATE_STREAMING) { if (error) { @@ -163,13 +164,27 @@ void TestScreenCastStreamProvider::RecordFrame(RgbaColor rgba_color) { spa_data->chunk->size = height_ * stride; spa_data->chunk->stride = stride; - uint32_t color = rgba_color.ToUInt32(); - for (uint32_t i = 0; i < height_; i++) { - uint32_t* column = reinterpret_cast(data); - for (uint32_t j = 0; j < width_; j++) { - column[j] = color; + // Produce a frame with given defect + if (frame_defect == EmptyData) { + spa_data->chunk->size = 0; + } else if (frame_defect == CorruptedData) { + spa_data->chunk->flags = SPA_CHUNK_FLAG_CORRUPTED; + } else if (frame_defect == CorruptedMetadata) { + struct spa_meta_header* spa_header = + static_cast(spa_buffer_find_meta_data( + spa_buffer, SPA_META_Header, sizeof(spa_meta_header))); + if (spa_header) { + spa_header->flags = SPA_META_HEADER_FLAG_CORRUPTED; + } + } else { + uint32_t color = rgba_color.ToUInt32(); + for (uint32_t i = 0; i < height_; i++) { + uint32_t* column = reinterpret_cast(data); + for (uint32_t j = 0; j < width_; j++) { + column[j] = color; + } + data += stride; } - data += stride; } pw_stream_queue_buffer(pw_stream_, buffer); diff --git a/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h b/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h index d893aa63ab..f63a2e647c 100644 --- a/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h +++ b/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h @@ -35,6 +35,8 @@ class TestScreenCastStreamProvider { virtual ~Observer() = default; }; + enum FrameDefect { None, EmptyData, CorruptedData, CorruptedMetadata }; + explicit TestScreenCastStreamProvider(Observer* observer, uint32_t width, uint32_t height); @@ -42,7 +44,7 @@ class TestScreenCastStreamProvider { uint32_t PipeWireNodeId(); - void RecordFrame(RgbaColor rgba_color); + void RecordFrame(RgbaColor rgba_color, FrameDefect frame_defect = None); void StartStreaming(); void StopStreaming();