Skip to content

Commit

Permalink
Video capture PipeWire: drop corrupted PipeWire buffers
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit cfbd6b0)

Bug: chromium:341928670
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-Original-Commit-Position: refs/heads/main@{#42292}
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/351563
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/6478@{#1}
Cr-Branched-From: 16fb790-refs/heads/main@{#42290}
  • Loading branch information
grulja authored and WebRTC LUCI CQ committed May 23, 2024
1 parent 16fb790 commit a18e38f
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,19 @@ void SharedScreenCastStreamPrivate::OnStreamProcess(void* data) {
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);
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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<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;
}
} 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;
}
data += stride;
}

pw_stream_queue_buffer(pw_stream_, buffer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@ class TestScreenCastStreamProvider {
virtual ~Observer() = default;
};

enum FrameDefect { None, EmptyData, CorruptedData, CorruptedMetadata };

explicit TestScreenCastStreamProvider(Observer* observer,
uint32_t width,
uint32_t height);
~TestScreenCastStreamProvider();

uint32_t PipeWireNodeId();

void RecordFrame(RgbaColor rgba_color);
void RecordFrame(RgbaColor rgba_color, FrameDefect frame_defect = None);
void StartStreaming();
void StopStreaming();

Expand Down

0 comments on commit a18e38f

Please sign in to comment.