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 <mfoltz@chromium.org>
Reviewed-by: Mark Foltz <mfoltz@chromium.org>
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/main@{#42292}
diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
index 6998d65..4e042ef 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 @@
return;
}
+ struct spa_meta_header* header =
+ static_cast<spa_meta_header*>(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 @@
}
}
- 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 f57e22c..1b00e27 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 @@
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 6a72edd..72ce01e 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 @@
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 @@
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 @@
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 ee5c17e..1055104 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 @@
}
}
-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 @@
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<uint32_t*>(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_meta_header*>(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;
}
- data += stride;
+ } else {
+ uint32_t color = rgba_color.ToUInt32();
+ for (uint32_t i = 0; i < height_; i++) {
+ uint32_t* column = reinterpret_cast<uint32_t*>(data);
+ for (uint32_t j = 0; j < width_; j++) {
+ column[j] = color;
+ }
+ 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 d893aa6..f63a2e6 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 @@
virtual ~Observer() = default;
};
+ enum FrameDefect { None, EmptyData, CorruptedData, CorruptedMetadata };
+
explicit TestScreenCastStreamProvider(Observer* observer,
uint32_t width,
uint32_t height);
@@ -42,7 +44,7 @@
uint32_t PipeWireNodeId();
- void RecordFrame(RgbaColor rgba_color);
+ void RecordFrame(RgbaColor rgba_color, FrameDefect frame_defect = None);
void StartStreaming();
void StopStreaming();
