PipeWire capturer: improvements to SharedScreenCastStream test
Remove useless comments and properly test frame values. Also rename the
FakeScreenCastStream to TestScreenCastStreamProvider.
Bug: webrtc:13429
Change-Id: I9b1943f0903101a1d9228cded541d3766879d84f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279740
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Commit-Queue: Jan Grulich <grulja@gmail.com>
Cr-Commit-Position: refs/heads/main@{#38450}
diff --git a/modules/desktop_capture/BUILD.gn b/modules/desktop_capture/BUILD.gn
index db70df2..482b03c 100644
--- a/modules/desktop_capture/BUILD.gn
+++ b/modules/desktop_capture/BUILD.gn
@@ -101,8 +101,8 @@
sources = [
"linux/wayland/shared_screencast_stream_unittest.cc",
- "linux/wayland/test/fake_screencast_stream.cc",
- "linux/wayland/test/fake_screencast_stream.h",
+ "linux/wayland/test/test_screencast_stream_provider.cc",
+ "linux/wayland/test/test_screencast_stream_provider.h",
]
configs += [
diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
index a870480..615d42e 100644
--- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
+++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
@@ -649,14 +649,12 @@
mouse_cursor_ = std::make_unique<MouseCursor>(
mouse_frame, DesktopVector(cursor->hotspot.x, cursor->hotspot.y));
- // For testing purpose
if (observer_) {
observer_->OnCursorShapeChanged();
}
}
mouse_cursor_position_.set(cursor->position.x, cursor->position.y);
- // For testing purpose
if (observer_) {
observer_->OnCursorPositionChanged();
}
@@ -734,7 +732,6 @@
}
if (!src) {
- // For testing purpose
if (observer_) {
observer_->OnFailedToProcessBuffer();
}
@@ -862,7 +859,6 @@
}
}
- // For testing purpose
if (observer_) {
observer_->OnDesktopFrameChanged();
}
diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.h b/modules/desktop_capture/linux/wayland/shared_screencast_stream.h
index f18c889..8ab951c 100644
--- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.h
+++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.h
@@ -81,11 +81,6 @@
private:
friend class SharedScreenCastStreamPrivate;
- // Allows test cases to use private functionality
- friend class PipeWireStreamTest;
-
- // FIXME: is this a useful thing to be public?
- explicit SharedScreenCastStream(Observer* notifier);
SharedScreenCastStream(const SharedScreenCastStream&) = delete;
SharedScreenCastStream& operator=(const SharedScreenCastStream&) = delete;
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 23296f5..196e1f7 100644
--- a/modules/desktop_capture/linux/wayland/shared_screencast_stream_unittest.cc
+++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream_unittest.cc
@@ -16,7 +16,7 @@
#include "api/units/time_delta.h"
#include "modules/desktop_capture/desktop_capturer.h"
#include "modules/desktop_capture/desktop_frame.h"
-#include "modules/desktop_capture/linux/wayland/test/fake_screencast_stream.h"
+#include "modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h"
#include "modules/desktop_capture/rgba_color.h"
#include "rtc_base/event.h"
#include "test/gmock.h"
@@ -29,26 +29,29 @@
namespace webrtc {
constexpr TimeDelta kShortWait = TimeDelta::Seconds(2);
-constexpr TimeDelta kLongWait = TimeDelta::Seconds(10);
+constexpr TimeDelta kLongWait = TimeDelta::Seconds(15);
constexpr int kBytesPerPixel = 4;
constexpr int32_t kWidth = 800;
constexpr int32_t kHeight = 640;
class PipeWireStreamTest : public ::testing::Test,
- public FakeScreenCastStream::Observer,
+ public TestScreenCastStreamProvider::Observer,
public SharedScreenCastStream::Observer {
public:
PipeWireStreamTest()
- : fake_screencast_stream_(
- std::make_unique<FakeScreenCastStream>(this, kWidth, kHeight)),
- shared_screencast_stream_(new SharedScreenCastStream()) {
+ : test_screencast_stream_provider_(
+ std::make_unique<TestScreenCastStreamProvider>(this,
+ kWidth,
+ kHeight)) {
+ shared_screencast_stream_ = SharedScreenCastStream::CreateDefault();
shared_screencast_stream_->SetObserver(this);
}
~PipeWireStreamTest() override {}
// FakeScreenCastPortal::Observer
+ MOCK_METHOD(void, OnBufferAdded, (), (override));
MOCK_METHOD(void, OnFrameRecorded, (), (override));
MOCK_METHOD(void, OnStreamReady, (uint32_t stream_node_id), (override));
MOCK_METHOD(void, OnStartStreaming, (), (override));
@@ -67,32 +70,40 @@
protected:
uint recorded_frames_ = 0;
bool streaming_ = false;
- std::unique_ptr<FakeScreenCastStream> fake_screencast_stream_;
+ std::unique_ptr<TestScreenCastStreamProvider>
+ test_screencast_stream_provider_;
rtc::scoped_refptr<SharedScreenCastStream> shared_screencast_stream_;
};
TEST_F(PipeWireStreamTest, TestPipeWire) {
// Set expectations for PipeWire to successfully connect both streams
rtc::Event waitConnectEvent;
+ rtc::Event waitAddBufferEvent;
+
EXPECT_CALL(*this, OnStreamReady(_))
.WillOnce(Invoke(this, &PipeWireStreamTest::StartScreenCastStream));
EXPECT_CALL(*this, OnStartStreaming).WillOnce([&waitConnectEvent] {
waitConnectEvent.Set();
});
+ EXPECT_CALL(*this, OnBufferAdded).WillRepeatedly([&waitAddBufferEvent] {
+ waitAddBufferEvent.Set();
+ });
// Give it some time to connect, the order between these shouldn't matter, but
// we need to be sure we are connected before we proceed to work with frames.
waitConnectEvent.Wait(kLongWait);
+ // Wait for an empty buffer to be added
+ waitAddBufferEvent.Wait(kShortWait);
+
rtc::Event frameRetrievedEvent;
EXPECT_CALL(*this, OnFrameRecorded).Times(3);
EXPECT_CALL(*this, OnDesktopFrameChanged)
.WillRepeatedly([&frameRetrievedEvent] { frameRetrievedEvent.Set(); });
// Record a frame in FakePipeWireStream
- RgbaColor red_color(255, 0, 0);
- fake_screencast_stream_->RecordFrame(red_color);
- frameRetrievedEvent.Wait(kShortWait);
+ RgbaColor red_color(0, 0, 255);
+ test_screencast_stream_provider_->RecordFrame(red_color);
// Retrieve a frame from SharedScreenCastStream
frameRetrievedEvent.Wait(kShortWait);
@@ -105,11 +116,11 @@
EXPECT_EQ(frame->rect().width(), kWidth);
EXPECT_EQ(frame->rect().height(), kHeight);
EXPECT_EQ(frame->stride(), frame->rect().width() * kBytesPerPixel);
- EXPECT_EQ(frame->data()[0], static_cast<uint8_t>(red_color.ToUInt32()));
+ EXPECT_EQ(RgbaColor(frame->data()), red_color);
// Test DesktopFrameQueue
RgbaColor green_color(0, 255, 0);
- fake_screencast_stream_->RecordFrame(green_color);
+ test_screencast_stream_provider_->RecordFrame(green_color);
frameRetrievedEvent.Wait(kShortWait);
std::unique_ptr<SharedDesktopFrame> frame2 =
shared_screencast_stream_->CaptureFrame();
@@ -118,7 +129,7 @@
EXPECT_EQ(frame2->rect().width(), kWidth);
EXPECT_EQ(frame2->rect().height(), kHeight);
EXPECT_EQ(frame2->stride(), frame->rect().width() * kBytesPerPixel);
- EXPECT_EQ(frame2->data()[0], static_cast<uint8_t>(green_color.ToUInt32()));
+ EXPECT_EQ(RgbaColor(frame2->data()), green_color);
// Thanks to DesktopFrameQueue we should be able to have two frames shared
EXPECT_EQ(frame->IsShared(), true);
@@ -127,14 +138,18 @@
// This should result into overwriting a frame in use
rtc::Event frameRecordedEvent;
- RgbaColor blue_color(0, 0, 255);
+ RgbaColor blue_color(255, 0, 0);
EXPECT_CALL(*this, OnFailedToProcessBuffer).WillOnce([&frameRecordedEvent] {
frameRecordedEvent.Set();
});
- fake_screencast_stream_->RecordFrame(blue_color);
+ test_screencast_stream_provider_->RecordFrame(blue_color);
frameRecordedEvent.Wait(kShortWait);
+ // First frame should be now overwritten with blue color
+ frameRetrievedEvent.Wait(kShortWait);
+ EXPECT_EQ(RgbaColor(frame->data()), blue_color);
+
// Test disconnection from stream
EXPECT_CALL(*this, OnStopStreaming);
shared_screencast_stream_->StopScreenCastStream();
diff --git a/modules/desktop_capture/linux/wayland/test/shared_screencast_stream_test.py b/modules/desktop_capture/linux/wayland/test/shared_screencast_stream_test.py
index 39292c8..bf97d0c 100644
--- a/modules/desktop_capture/linux/wayland/test/shared_screencast_stream_test.py
+++ b/modules/desktop_capture/linux/wayland/test/shared_screencast_stream_test.py
@@ -28,16 +28,16 @@
description='Run shared_screencast_screen test.')
parser.add_argument('build_dir',
help='Path to the build directory (e.g. out/Release).')
+ parser.add_argument(
+ '--isolated-script-test-output',
+ default=None,
+ help='Path to output JSON file which Chromium infra requires.')
# Unused args
# We just need to avoid passing these to the test
parser.add_argument(
'--isolated-script-test-perf-output',
default=None,
help='Path to store perf results in histogram proto format.')
- parser.add_argument(
- '--isolated-script-test-output',
- default=None,
- help='Path to output JSON file which Chromium infra requires.')
return parser.parse_known_args()
diff --git a/modules/desktop_capture/linux/wayland/test/fake_screencast_stream.cc b/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.cc
similarity index 83%
rename from modules/desktop_capture/linux/wayland/test/fake_screencast_stream.cc
rename to modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.cc
index 8f973da..c49823d 100644
--- a/modules/desktop_capture/linux/wayland/test/fake_screencast_stream.cc
+++ b/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.cc
@@ -9,7 +9,7 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include "modules/desktop_capture/linux/wayland/test/fake_screencast_stream.h"
+#include "modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h"
#include <fcntl.h>
#include <sys/mman.h>
@@ -37,9 +37,9 @@
constexpr int kBytesPerPixel = 4;
-FakeScreenCastStream::FakeScreenCastStream(Observer* observer,
- uint32_t width,
- uint32_t height)
+TestScreenCastStreamProvider::TestScreenCastStreamProvider(Observer* observer,
+ uint32_t width,
+ uint32_t height)
: observer_(observer), width_(width), height_(height) {
#if defined(WEBRTC_DLOPEN_PIPEWIRE)
StubPathMap paths;
@@ -126,7 +126,7 @@
return;
}
-FakeScreenCastStream::~FakeScreenCastStream() {
+TestScreenCastStreamProvider::~TestScreenCastStreamProvider() {
if (pw_main_loop_) {
pw_thread_loop_stop(pw_main_loop_);
}
@@ -148,7 +148,7 @@
}
}
-void FakeScreenCastStream::RecordFrame(RgbaColor rgba_color) {
+void TestScreenCastStreamProvider::RecordFrame(RgbaColor rgba_color) {
const char* error;
if (pw_stream_get_state(pw_stream_, &error) != PW_STREAM_STATE_STREAMING) {
if (error) {
@@ -195,36 +195,39 @@
}
}
-void FakeScreenCastStream::StartStreaming() {
+void TestScreenCastStreamProvider::StartStreaming() {
if (pw_stream_ && pw_node_id_ != 0) {
pw_stream_set_active(pw_stream_, true);
}
}
-void FakeScreenCastStream::StopStreaming() {
+void TestScreenCastStreamProvider::StopStreaming() {
if (pw_stream_ && pw_node_id_ != 0) {
pw_stream_set_active(pw_stream_, false);
}
}
// static
-void FakeScreenCastStream::OnCoreError(void* data,
- uint32_t id,
- int seq,
- int res,
- const char* message) {
- FakeScreenCastStream* that = static_cast<FakeScreenCastStream*>(data);
+void TestScreenCastStreamProvider::OnCoreError(void* data,
+ uint32_t id,
+ int seq,
+ int res,
+ const char* message) {
+ TestScreenCastStreamProvider* that =
+ static_cast<TestScreenCastStreamProvider*>(data);
RTC_DCHECK(that);
RTC_LOG(LS_ERROR) << "PipeWire test: PipeWire remote error: " << message;
}
// static
-void FakeScreenCastStream::OnStreamStateChanged(void* data,
- pw_stream_state old_state,
- pw_stream_state state,
- const char* error_message) {
- FakeScreenCastStream* that = static_cast<FakeScreenCastStream*>(data);
+void TestScreenCastStreamProvider::OnStreamStateChanged(
+ void* data,
+ pw_stream_state old_state,
+ pw_stream_state state,
+ const char* error_message) {
+ TestScreenCastStreamProvider* that =
+ static_cast<TestScreenCastStreamProvider*>(data);
RTC_DCHECK(that);
switch (state) {
@@ -260,10 +263,12 @@
}
// static
-void FakeScreenCastStream::OnStreamParamChanged(void* data,
- uint32_t id,
- const struct spa_pod* format) {
- FakeScreenCastStream* that = static_cast<FakeScreenCastStream*>(data);
+void TestScreenCastStreamProvider::OnStreamParamChanged(
+ void* data,
+ uint32_t id,
+ const struct spa_pod* format) {
+ TestScreenCastStreamProvider* that =
+ static_cast<TestScreenCastStreamProvider*>(data);
RTC_DCHECK(that);
RTC_LOG(LS_INFO) << "PipeWire test: PipeWire stream format changed.";
@@ -302,8 +307,10 @@
}
// static
-void FakeScreenCastStream::OnStreamAddBuffer(void* data, pw_buffer* buffer) {
- FakeScreenCastStream* that = static_cast<FakeScreenCastStream*>(data);
+void TestScreenCastStreamProvider::OnStreamAddBuffer(void* data,
+ pw_buffer* buffer) {
+ TestScreenCastStreamProvider* that =
+ static_cast<TestScreenCastStreamProvider*>(data);
RTC_DCHECK(that);
struct spa_data* spa_data = buffer->buffer->datas;
@@ -345,14 +352,17 @@
if (spa_data->data == MAP_FAILED) {
RTC_LOG(LS_ERROR) << "PipeWire test: Failed to mmap memory";
} else {
+ that->observer_->OnBufferAdded();
RTC_LOG(LS_INFO) << "PipeWire test: Memfd created successfully: "
<< spa_data->data << spa_data->maxsize;
}
}
// static
-void FakeScreenCastStream::OnStreamRemoveBuffer(void* data, pw_buffer* buffer) {
- FakeScreenCastStream* that = static_cast<FakeScreenCastStream*>(data);
+void TestScreenCastStreamProvider::OnStreamRemoveBuffer(void* data,
+ pw_buffer* buffer) {
+ TestScreenCastStreamProvider* that =
+ static_cast<TestScreenCastStreamProvider*>(data);
RTC_DCHECK(that);
struct spa_buffer* spa_buffer = buffer->buffer;
@@ -363,7 +373,7 @@
}
}
-uint32_t FakeScreenCastStream::PipeWireNodeId() {
+uint32_t TestScreenCastStreamProvider::PipeWireNodeId() {
return pw_node_id_;
}
diff --git a/modules/desktop_capture/linux/wayland/test/fake_screencast_stream.h b/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h
similarity index 81%
rename from modules/desktop_capture/linux/wayland/test/fake_screencast_stream.h
rename to modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h
index 1b3bb06..d893aa6 100644
--- a/modules/desktop_capture/linux/wayland/test/fake_screencast_stream.h
+++ b/modules/desktop_capture/linux/wayland/test/test_screencast_stream_provider.h
@@ -8,8 +8,8 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#ifndef MODULES_DESKTOP_CAPTURE_LINUX_WAYLAND_TEST_FAKE_SCREENCAST_STREAM_H_
-#define MODULES_DESKTOP_CAPTURE_LINUX_WAYLAND_TEST_FAKE_SCREENCAST_STREAM_H_
+#ifndef MODULES_DESKTOP_CAPTURE_LINUX_WAYLAND_TEST_TEST_SCREENCAST_STREAM_PROVIDER_H_
+#define MODULES_DESKTOP_CAPTURE_LINUX_WAYLAND_TEST_TEST_SCREENCAST_STREAM_PROVIDER_H_
#include <pipewire/pipewire.h>
#include <spa/param/video/format-utils.h>
@@ -20,10 +20,11 @@
namespace webrtc {
-class FakeScreenCastStream {
+class TestScreenCastStreamProvider {
public:
class Observer {
public:
+ virtual void OnBufferAdded() = 0;
virtual void OnFrameRecorded() = 0;
virtual void OnStreamReady(uint32_t stream_node_id) = 0;
virtual void OnStartStreaming() = 0;
@@ -34,10 +35,10 @@
virtual ~Observer() = default;
};
- explicit FakeScreenCastStream(Observer* observer,
- uint32_t width,
- uint32_t height);
- ~FakeScreenCastStream();
+ explicit TestScreenCastStreamProvider(Observer* observer,
+ uint32_t width,
+ uint32_t height);
+ ~TestScreenCastStreamProvider();
uint32_t PipeWireNodeId();
@@ -89,4 +90,4 @@
} // namespace webrtc
-#endif // MODULES_DESKTOP_CAPTURE_LINUX_WAYLAND_TEST_FAKE_SCREENCAST_STREAM_H_
+#endif // MODULES_DESKTOP_CAPTURE_LINUX_WAYLAND_TEST_TEST_SCREENCAST_STREAM_PROVIDER_H_