Refactor TestVideoCapturer to support multiple sinks.
To be able to reuse VideoBroadcaster, that class needs to be
officially threadsafe. It already had the needed locks, but thread
checkers have to be deleted to allow calls to AddOrUpdateSink on
multiple threads (worker thread + encoder thread).
Bug: webrtc:6353, webrtc:10147
Change-Id: I16128ac205c566f09402b6f22587a340d9a983c1
Reviewed-on: https://webrtc-review.googlesource.com/c/115201
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26073}diff --git a/media/base/videobroadcaster.cc b/media/base/videobroadcaster.cc
index f953bab..125cf17 100644
--- a/media/base/videobroadcaster.cc
+++ b/media/base/videobroadcaster.cc
@@ -20,15 +20,12 @@
namespace rtc {
-VideoBroadcaster::VideoBroadcaster() {
- thread_checker_.DetachFromThread();
-}
+VideoBroadcaster::VideoBroadcaster() = default;
VideoBroadcaster::~VideoBroadcaster() = default;
void VideoBroadcaster::AddOrUpdateSink(
VideoSinkInterface<webrtc::VideoFrame>* sink,
const VideoSinkWants& wants) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(sink != nullptr);
rtc::CritScope cs(&sinks_and_wants_lock_);
VideoSourceBase::AddOrUpdateSink(sink, wants);
@@ -37,7 +34,6 @@
void VideoBroadcaster::RemoveSink(
VideoSinkInterface<webrtc::VideoFrame>* sink) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(sink != nullptr);
rtc::CritScope cs(&sinks_and_wants_lock_);
VideoSourceBase::RemoveSink(sink);
@@ -83,8 +79,6 @@
}
void VideoBroadcaster::UpdateWants() {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
-
VideoSinkWants wants;
wants.rotation_applied = false;
for (auto& sink : sink_pairs()) {
diff --git a/media/base/videobroadcaster.h b/media/base/videobroadcaster.h
index 5f02a35..18bfc9d 100644
--- a/media/base/videobroadcaster.h
+++ b/media/base/videobroadcaster.h
@@ -21,12 +21,11 @@
namespace rtc {
-// VideoBroadcaster broadcast video frames to sinks and combines
-// VideoSinkWants from its sinks. It does that by implementing
-// rtc::VideoSourceInterface and rtc::VideoSinkInterface.
-// Sinks must be added and removed on one and only one thread.
-// Video frames can be broadcasted on any thread. I.e VideoBroadcaster::OnFrame
-// can be called on any thread.
+// VideoBroadcaster broadcast video frames to sinks and combines VideoSinkWants
+// from its sinks. It does that by implementing rtc::VideoSourceInterface and
+// rtc::VideoSinkInterface. The class is threadsafe; methods may be called on
+// any thread. This is needed because VideoStreamEncoder calls AddOrUpdateSink
+// both on the worker thread and on the encoder task queue.
class VideoBroadcaster : public VideoSourceBase,
public VideoSinkInterface<webrtc::VideoFrame> {
public:
@@ -57,7 +56,6 @@
int width,
int height) RTC_EXCLUSIVE_LOCKS_REQUIRED(sinks_and_wants_lock_);
- ThreadChecker thread_checker_;
rtc::CriticalSection sinks_and_wants_lock_;
VideoSinkWants current_wants_ RTC_GUARDED_BY(sinks_and_wants_lock_);
diff --git a/media/base/videosourcebase.cc b/media/base/videosourcebase.cc
index 64b49cc..47dfaab 100644
--- a/media/base/videosourcebase.cc
+++ b/media/base/videosourcebase.cc
@@ -16,15 +16,12 @@
namespace rtc {
-VideoSourceBase::VideoSourceBase() {
- thread_checker_.DetachFromThread();
-}
+VideoSourceBase::VideoSourceBase() = default;
VideoSourceBase::~VideoSourceBase() = default;
void VideoSourceBase::AddOrUpdateSink(
VideoSinkInterface<webrtc::VideoFrame>* sink,
const VideoSinkWants& wants) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(sink != nullptr);
SinkPair* sink_pair = FindSinkPair(sink);
@@ -36,7 +33,6 @@
}
void VideoSourceBase::RemoveSink(VideoSinkInterface<webrtc::VideoFrame>* sink) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(sink != nullptr);
RTC_DCHECK(FindSinkPair(sink));
sinks_.erase(std::remove_if(sinks_.begin(), sinks_.end(),
@@ -48,7 +44,6 @@
VideoSourceBase::SinkPair* VideoSourceBase::FindSinkPair(
const VideoSinkInterface<webrtc::VideoFrame>* sink) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
auto sink_pair_it = std::find_if(
sinks_.begin(), sinks_.end(),
[sink](const SinkPair& sink_pair) { return sink_pair.sink == sink; });
diff --git a/media/base/videosourcebase.h b/media/base/videosourcebase.h
index 6c7d5a3..aaae61c 100644
--- a/media/base/videosourcebase.h
+++ b/media/base/videosourcebase.h
@@ -39,7 +39,6 @@
SinkPair* FindSinkPair(const VideoSinkInterface<webrtc::VideoFrame>* sink);
const std::vector<SinkPair>& sink_pairs() const { return sinks_; }
- ThreadChecker thread_checker_;
private:
std::vector<SinkPair> sinks_;
diff --git a/test/frame_generator_capturer.cc b/test/frame_generator_capturer.cc
index 858f895..768bab5 100644
--- a/test/frame_generator_capturer.cc
+++ b/test/frame_generator_capturer.cc
@@ -127,7 +127,6 @@
int target_fps)
: clock_(clock),
sending_(true),
- sink_(nullptr),
sink_wants_observer_(nullptr),
frame_generator_(std::move(frame_generator)),
source_fps_(target_fps),
@@ -186,11 +185,7 @@
first_frame_capture_time_ = frame->ntp_time_ms();
}
- if (sink_) {
- absl::optional<VideoFrame> out_frame = AdaptFrame(*frame);
- if (out_frame)
- sink_->OnFrame(*out_frame);
- }
+ TestVideoCapturer::OnFrame(*frame);
}
}
@@ -236,31 +231,29 @@
void FrameGeneratorCapturer::AddOrUpdateSink(
rtc::VideoSinkInterface<VideoFrame>* sink,
const rtc::VideoSinkWants& wants) {
+ TestVideoCapturer::AddOrUpdateSink(sink, wants);
rtc::CritScope cs(&lock_);
- RTC_CHECK(!sink_ || sink_ == sink);
- sink_ = sink;
- if (sink_wants_observer_)
+ if (sink_wants_observer_) {
+ // Tests need to observe unmodified sink wants.
sink_wants_observer_->OnSinkWantsChanged(sink, wants);
-
- // Handle framerate within this class, just pass on resolution for possible
- // adaptation.
- rtc::VideoSinkWants resolution_wants = wants;
- resolution_wants.max_framerate_fps = std::numeric_limits<int>::max();
- TestVideoCapturer::AddOrUpdateSink(sink, resolution_wants);
-
- // Ignore any requests for framerate higher than initially configured.
- if (wants.max_framerate_fps < target_capture_fps_) {
- wanted_fps_.emplace(wants.max_framerate_fps);
- } else {
- wanted_fps_.reset();
}
+ UpdateFps(GetSinkWants().max_framerate_fps);
}
void FrameGeneratorCapturer::RemoveSink(
rtc::VideoSinkInterface<VideoFrame>* sink) {
+ TestVideoCapturer::RemoveSink(sink);
+
rtc::CritScope cs(&lock_);
- RTC_CHECK(sink_ == sink);
- sink_ = nullptr;
+ UpdateFps(GetSinkWants().max_framerate_fps);
+}
+
+void FrameGeneratorCapturer::UpdateFps(int max_fps) {
+ if (max_fps < target_capture_fps_) {
+ wanted_fps_.emplace(max_fps);
+ } else {
+ wanted_fps_.reset();
+ }
}
void FrameGeneratorCapturer::ForceFrame() {
diff --git a/test/frame_generator_capturer.h b/test/frame_generator_capturer.h
index cc938b0..cb76806 100644
--- a/test/frame_generator_capturer.h
+++ b/test/frame_generator_capturer.h
@@ -89,10 +89,10 @@
void InsertFrame();
static bool Run(void* obj);
int GetCurrentConfiguredFramerate();
+ void UpdateFps(int max_fps) RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_);
Clock* const clock_;
bool sending_;
- rtc::VideoSinkInterface<VideoFrame>* sink_ RTC_GUARDED_BY(&lock_);
SinkWantsObserver* sink_wants_observer_ RTC_GUARDED_BY(&lock_);
rtc::CriticalSection lock_;
diff --git a/test/test_video_capturer.cc b/test/test_video_capturer.cc
index bc0ceda..0d57715 100644
--- a/test/test_video_capturer.cc
+++ b/test/test_video_capturer.cc
@@ -10,6 +10,8 @@
#include "test/test_video_capturer.h"
+#include <algorithm>
+
#include "api/video/i420_buffer.h"
#include "api/video/video_frame_buffer.h"
#include "api/video/video_rotation.h"
@@ -17,45 +19,55 @@
namespace webrtc {
namespace test {
-TestVideoCapturer::TestVideoCapturer()
- : video_adapter_(new cricket::VideoAdapter()) {}
-TestVideoCapturer::~TestVideoCapturer() {}
+TestVideoCapturer::TestVideoCapturer() = default;
+TestVideoCapturer::~TestVideoCapturer() = default;
-absl::optional<VideoFrame> TestVideoCapturer::AdaptFrame(
- const VideoFrame& frame) {
+void TestVideoCapturer::OnFrame(const VideoFrame& frame) {
int cropped_width = 0;
int cropped_height = 0;
int out_width = 0;
int out_height = 0;
- if (!video_adapter_->AdaptFrameResolution(
+ if (!video_adapter_.AdaptFrameResolution(
frame.width(), frame.height(), frame.timestamp_us() * 1000,
&cropped_width, &cropped_height, &out_width, &out_height)) {
// Drop frame in order to respect frame rate constraint.
- return absl::nullopt;
+ return;
}
- absl::optional<VideoFrame> out_frame;
if (out_height != frame.height() || out_width != frame.width()) {
// Video adapter has requested a down-scale. Allocate a new buffer and
// return scaled version.
rtc::scoped_refptr<I420Buffer> scaled_buffer =
I420Buffer::Create(out_width, out_height);
scaled_buffer->ScaleFrom(*frame.video_frame_buffer()->ToI420());
- out_frame.emplace(
+ broadcaster_.OnFrame(
VideoFrame(scaled_buffer, kVideoRotation_0, frame.timestamp_us()));
} else {
// No adaptations needed, just return the frame as is.
- out_frame.emplace(frame);
+ broadcaster_.OnFrame(frame);
}
+}
- return out_frame;
+rtc::VideoSinkWants TestVideoCapturer::GetSinkWants() {
+ return broadcaster_.wants();
}
void TestVideoCapturer::AddOrUpdateSink(
rtc::VideoSinkInterface<VideoFrame>* sink,
const rtc::VideoSinkWants& wants) {
- video_adapter_->OnResolutionFramerateRequest(
+ broadcaster_.AddOrUpdateSink(sink, wants);
+ UpdateVideoAdapter();
+}
+
+void TestVideoCapturer::RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) {
+ broadcaster_.RemoveSink(sink);
+ UpdateVideoAdapter();
+}
+
+void TestVideoCapturer::UpdateVideoAdapter() {
+ rtc::VideoSinkWants wants = broadcaster_.wants();
+ video_adapter_.OnResolutionFramerateRequest(
wants.target_pixel_count, wants.max_pixel_count, wants.max_framerate_fps);
}
diff --git a/test/test_video_capturer.h b/test/test_video_capturer.h
index 93b3e8f..250736e 100644
--- a/test/test_video_capturer.h
+++ b/test/test_video_capturer.h
@@ -14,19 +14,12 @@
#include <memory>
-#include "absl/types/optional.h"
-#include "api/video/i420_buffer.h"
#include "api/video/video_frame.h"
#include "api/video/video_source_interface.h"
#include "media/base/videoadapter.h"
-#include "rtc_base/criticalsection.h"
-
-namespace cricket {
-class VideoAdapter;
-} // namespace cricket
+#include "media/base/videobroadcaster.h"
namespace webrtc {
-class Clock;
namespace test {
class TestVideoCapturer : public rtc::VideoSourceInterface<VideoFrame> {
@@ -36,13 +29,17 @@
void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
const rtc::VideoSinkWants& wants) override;
+ void RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) override;
protected:
- absl::optional<VideoFrame> AdaptFrame(const VideoFrame& frame);
+ void OnFrame(const VideoFrame& frame);
rtc::VideoSinkWants GetSinkWants();
private:
- const std::unique_ptr<cricket::VideoAdapter> video_adapter_;
+ void UpdateVideoAdapter();
+
+ rtc::VideoBroadcaster broadcaster_;
+ cricket::VideoAdapter video_adapter_;
};
} // namespace test
} // namespace webrtc
diff --git a/test/vcm_capturer.cc b/test/vcm_capturer.cc
index 40402f8..353be0a 100644
--- a/test/vcm_capturer.cc
+++ b/test/vcm_capturer.cc
@@ -13,8 +13,6 @@
#include <stdint.h>
#include <memory>
-#include "absl/types/optional.h"
-#include "common_types.h" // NOLINT(build/include)
#include "modules/video_capture/video_capture_factory.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
@@ -22,7 +20,7 @@
namespace webrtc {
namespace test {
-VcmCapturer::VcmCapturer() : sink_(nullptr), vcm_(nullptr) {}
+VcmCapturer::VcmCapturer() : vcm_(nullptr) {}
bool VcmCapturer::Init(size_t width,
size_t height,
@@ -74,20 +72,6 @@
return vcm_capturer.release();
}
-void VcmCapturer::AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
- const rtc::VideoSinkWants& wants) {
- rtc::CritScope lock(&crit_);
- RTC_CHECK(!sink_ || sink_ == sink);
- sink_ = sink;
- TestVideoCapturer::AddOrUpdateSink(sink, wants);
-}
-
-void VcmCapturer::RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) {
- rtc::CritScope lock(&crit_);
- RTC_CHECK(sink_ == sink);
- sink_ = nullptr;
-}
-
void VcmCapturer::Destroy() {
if (!vcm_)
return;
@@ -103,12 +87,7 @@
}
void VcmCapturer::OnFrame(const VideoFrame& frame) {
- rtc::CritScope lock(&crit_);
- if (sink_) {
- absl::optional<VideoFrame> out_frame = AdaptFrame(frame);
- if (out_frame)
- sink_->OnFrame(*out_frame);
- }
+ TestVideoCapturer::OnFrame(frame);
}
} // namespace test
diff --git a/test/vcm_capturer.h b/test/vcm_capturer.h
index 208a771..59000ae 100644
--- a/test/vcm_capturer.h
+++ b/test/vcm_capturer.h
@@ -11,10 +11,9 @@
#define TEST_VCM_CAPTURER_H_
#include <memory>
+#include <vector>
-#include "common_video/libyuv/include/webrtc_libyuv.h"
#include "modules/video_capture/video_capture.h"
-#include "rtc_base/criticalsection.h"
#include "rtc_base/scoped_ref_ptr.h"
#include "test/test_video_capturer.h"
@@ -30,10 +29,6 @@
size_t capture_device_index);
virtual ~VcmCapturer();
- void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
- const rtc::VideoSinkWants& wants) override;
- void RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) override;
-
void OnFrame(const VideoFrame& frame) override;
private:
@@ -44,8 +39,6 @@
size_t capture_device_index);
void Destroy();
- rtc::CriticalSection crit_;
- rtc::VideoSinkInterface<VideoFrame>* sink_ RTC_GUARDED_BY(crit_);
rtc::scoped_refptr<VideoCaptureModule> vcm_;
VideoCaptureCapability capability_;
};
