Change FakePeriodicVideoCapturer to use a TaskQueue instead of Thread.
This changes callbacks to OnFrame methods to occur on a task queue which
is in line with how it's called in production.
The change is essentially around inheriting from FakeVideoCapturerWithTaskQueue
instead of FakeVideoCapturer, but also removes the dependency on rtc::MessageHandler.
Along the way I'm also updating an ortc test that uses FakePeriodicVideoCapturer
and had a bug that was masked by the fact that FakePeriodicVideoCapturer
previously used rtc::Thread::Current internally, but was being called
by the wrong thread (and there were no checks for it).
As a result, I'm also adding a bunch of checks to help with correct usage.
Bug: webrtc:8841, webrtc:8848
Change-Id: I21b710873b508ebc55f8d2e4545d862766656871
Reviewed-on: https://webrtc-review.googlesource.com/49400
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21951}diff --git a/ortc/ortcfactory_integrationtest.cc b/ortc/ortcfactory_integrationtest.cc
index 6385dee..def0e08 100644
--- a/ortc/ortcfactory_integrationtest.cc
+++ b/ortc/ortcfactory_integrationtest.cc
@@ -77,6 +77,7 @@
// Sockets are bound to the ANY address, so this is needed to tell the
// virtual network which address to use in this case.
virtual_socket_server_.SetDefaultRoute(kIPv4LocalHostAddress);
+ network_thread_.SetName("TestNetworkThread", this);
network_thread_.Start();
// Need to create after network thread is started.
ortc_factory1_ =
@@ -220,7 +221,7 @@
rtc::scoped_refptr<webrtc::VideoTrackInterface>
CreateLocalVideoTrackAndFakeCapturer(const std::string& id,
OrtcFactoryInterface* ortc_factory) {
- cricket::FakeVideoCapturer* fake_capturer =
+ webrtc::FakePeriodicVideoCapturer* fake_capturer =
new webrtc::FakePeriodicVideoCapturer();
fake_video_capturers_.push_back(fake_capturer);
rtc::scoped_refptr<webrtc::VideoTrackSourceInterface> source =
@@ -350,7 +351,7 @@
std::unique_ptr<OrtcFactoryInterface> ortc_factory1_;
std::unique_ptr<OrtcFactoryInterface> ortc_factory2_;
// Actually owned by video tracks.
- std::vector<cricket::FakeVideoCapturer*> fake_video_capturers_;
+ std::vector<webrtc::FakePeriodicVideoCapturer*> fake_video_capturers_;
int received_audio_frames1_ = 0;
int received_audio_frames2_ = 0;
int rendered_video_frames1_ = 0;
@@ -462,7 +463,7 @@
// Stop the old capturer, set a new track, and verify new frames are received
// from the new track. Stopping the old capturer ensures that we aren't
// actually still getting frames from it.
- fake_video_capturers_[0]->Stop();
+ fake_video_capturers_[0]->StopFrameDelivery();
int prev_num_frames = fake_renderer.num_rendered_frames();
error = sender->SetTrack(
CreateLocalVideoTrackAndFakeCapturer("video_2", ortc_factory1_.get()));
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 52e2daf..06166e2 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -368,6 +368,7 @@
"../rtc_base:rtc_base",
"../rtc_base:rtc_base_approved",
"../rtc_base:rtc_base_tests_utils",
+ "../rtc_base:rtc_task_queue_api",
"../test:test_support",
]
diff --git a/pc/test/fakeperiodicvideocapturer.h b/pc/test/fakeperiodicvideocapturer.h
index 38f2c75..37eef2d 100644
--- a/pc/test/fakeperiodicvideocapturer.h
+++ b/pc/test/fakeperiodicvideocapturer.h
@@ -14,59 +14,83 @@
#ifndef PC_TEST_FAKEPERIODICVIDEOCAPTURER_H_
#define PC_TEST_FAKEPERIODICVIDEOCAPTURER_H_
+#include <memory>
#include <vector>
#include "media/base/fakevideocapturer.h"
-#include "rtc_base/thread.h"
+#include "rtc_base/arraysize.h"
+#include "rtc_base/thread_checker.h"
namespace webrtc {
-class FakePeriodicVideoCapturer : public cricket::FakeVideoCapturer,
- public rtc::MessageHandler {
+class FakePeriodicVideoCapturer
+ : public cricket::FakeVideoCapturerWithTaskQueue {
public:
FakePeriodicVideoCapturer() {
- std::vector<cricket::VideoFormat> formats;
- formats.push_back(cricket::VideoFormat(1280, 720,
- cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420));
- formats.push_back(cricket::VideoFormat(640, 480,
- cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420));
- formats.push_back(cricket::VideoFormat(640, 360,
- cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420));
- formats.push_back(cricket::VideoFormat(320, 240,
- cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420));
- formats.push_back(cricket::VideoFormat(160, 120,
- cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420));
- ResetSupportedFormats(formats);
+ worker_thread_checker_.DetachFromThread();
+ using cricket::VideoFormat;
+ static const VideoFormat formats[] = {
+ {1280, 720, VideoFormat::FpsToInterval(30), cricket::FOURCC_I420},
+ {640, 480, VideoFormat::FpsToInterval(30), cricket::FOURCC_I420},
+ {640, 360, VideoFormat::FpsToInterval(30), cricket::FOURCC_I420},
+ {320, 240, VideoFormat::FpsToInterval(30), cricket::FOURCC_I420},
+ {160, 120, VideoFormat::FpsToInterval(30), cricket::FOURCC_I420},
+ };
+
+ ResetSupportedFormats({&formats[0], &formats[arraysize(formats)]});
}
- virtual cricket::CaptureState Start(const cricket::VideoFormat& format) {
- cricket::CaptureState state = FakeVideoCapturer::Start(format);
- if (state != cricket::CS_FAILED) {
- rtc::Thread::Current()->Post(RTC_FROM_HERE, this, MSG_CREATEFRAME);
- }
- return state;
+ ~FakePeriodicVideoCapturer() override {
+ RTC_DCHECK(main_thread_checker_.CalledOnValidThread());
}
- virtual void Stop() {
- rtc::Thread::Current()->Clear(this);
- }
- // Inherited from MesageHandler.
- virtual void OnMessage(rtc::Message* msg) {
- if (msg->message_id == MSG_CREATEFRAME) {
- if (IsRunning()) {
- CaptureFrame();
- rtc::Thread::Current()->PostDelayed(
- RTC_FROM_HERE, static_cast<int>(GetCaptureFormat()->interval /
- rtc::kNumNanosecsPerMillisec),
- this, MSG_CREATEFRAME);
- }
- }
+
+ // Workaround method for tests to allow stopping frame delivery directly.
+ // The |Start()| method expects to be called from the "worker thread" that
+ // is owned by the factory (think OrtcFactoryInterface or
+ // PeerConnectionFactoryInterface). That thread is not directly exposed,
+ // which can make it tricky for tests to inject calls on.
+ // So, in order to allow tests to stop frame delivery directly from the
+ // test thread, we expose this method publicly.
+ void StopFrameDelivery() {
+ RunSynchronouslyOnTaskQueue([this]() {
+ RTC_DCHECK_RUN_ON(&task_queue_);
+ deliver_frames_ = false;
+ });
}
private:
- enum {
- // Offset 0xFF to make sure this don't collide with base class messages.
- MSG_CREATEFRAME = 0xFF
- };
+ cricket::CaptureState Start(const cricket::VideoFormat& format) override {
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+ cricket::CaptureState state = FakeVideoCapturer::Start(format);
+ if (state != cricket::CS_FAILED) {
+ task_queue_.PostTask([this]() {
+ RTC_DCHECK_RUN_ON(&task_queue_);
+ deliver_frames_ = true;
+ DeliverFrame();
+ });
+ }
+ return state;
+ }
+
+ void Stop() override {
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+ StopFrameDelivery();
+ }
+
+ void DeliverFrame() {
+ RTC_DCHECK_RUN_ON(&task_queue_);
+ if (IsRunning() && deliver_frames_) {
+ CaptureFrame();
+ task_queue_.PostDelayedTask(
+ [this]() { DeliverFrame(); },
+ static_cast<int>(GetCaptureFormat()->interval /
+ rtc::kNumNanosecsPerMillisec));
+ }
+ }
+
+ rtc::ThreadChecker main_thread_checker_;
+ rtc::ThreadChecker worker_thread_checker_;
+ bool deliver_frames_ RTC_GUARDED_BY(task_queue_) = false;
};
} // namespace webrtc
