Enforce that VideoProcessor is only run on a TaskQueue.

Prior to this change, the VideoProcessor was run on the main thread
in the unit tests. Using a TaskQueue there instead, we can be
stricter in the thread checks.

Bug: webrtc:8524
Change-Id: Ice7b68f7344fc52801dff7a98cbc219b7231bfbc
Reviewed-on: https://webrtc-review.googlesource.com/48921
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21942}
This commit is contained in:
Rasmus Brandt
2018-02-07 13:56:16 +01:00
committed by Commit Bot
parent b0acec3679
commit 4b381afd8e
3 changed files with 50 additions and 22 deletions

View File

@ -122,6 +122,8 @@ VideoProcessor::VideoProcessor(webrtc::VideoEncoder* encoder,
num_encoded_frames_(0),
num_decoded_frames_(0),
stats_(stats) {
RTC_CHECK(rtc::TaskQueue::Current())
<< "VideoProcessor must be run on a task queue.";
RTC_CHECK(encoder);
RTC_CHECK(decoders && decoders->size() == num_simulcast_or_spatial_layers_);
RTC_CHECK(input_frame_reader);

View File

@ -77,7 +77,9 @@ class VideoProcessor {
explicit VideoProcessorEncodeCompleteCallback(
VideoProcessor* video_processor)
: video_processor_(video_processor),
task_queue_(rtc::TaskQueue::Current()) {}
task_queue_(rtc::TaskQueue::Current()) {
RTC_DCHECK(task_queue_);
}
Result OnEncodedImage(
const webrtc::EncodedImage& encoded_image,
@ -85,7 +87,8 @@ class VideoProcessor {
const webrtc::RTPFragmentationHeader* fragmentation) override {
RTC_CHECK(codec_specific_info);
if (task_queue_ && !task_queue_->IsCurrent()) {
// Post the callback to the right task queue, if needed.
if (!task_queue_->IsCurrent()) {
task_queue_->PostTask(
std::unique_ptr<rtc::QueuedTask>(new EncodeCallbackTask(
video_processor_, encoded_image, codec_specific_info)));
@ -131,10 +134,13 @@ class VideoProcessor {
explicit VideoProcessorDecodeCompleteCallback(
VideoProcessor* video_processor)
: video_processor_(video_processor),
task_queue_(rtc::TaskQueue::Current()) {}
task_queue_(rtc::TaskQueue::Current()) {
RTC_DCHECK(task_queue_);
}
int32_t Decoded(webrtc::VideoFrame& image) override {
if (task_queue_ && !task_queue_->IsCurrent()) {
// Post the callback to the right task queue, if needed.
if (!task_queue_->IsCurrent()) {
task_queue_->PostTask(
[this, image]() { video_processor_->FrameDecoded(image); });
return 0;

View File

@ -15,11 +15,12 @@
#include "modules/video_coding/codecs/test/videoprocessor.h"
#include "modules/video_coding/include/mock/mock_video_codec_interface.h"
#include "modules/video_coding/include/video_coding.h"
#include "rtc_base/event.h"
#include "rtc_base/ptr_util.h"
#include "rtc_base/task_queue.h"
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/testsupport/mock/mock_frame_reader.h"
#include "test/video_codec_settings.h"
#include "typedefs.h" // NOLINT(build/include)
using ::testing::_;
@ -37,13 +38,21 @@ const int kFrameSize = kWidth * kHeight * 3 / 2; // I420.
} // namespace
#define DO_SYNC(q, block) \
{ \
rtc::Event event(false, false); \
q.PostTask([&, this] { \
block; \
event.Set(); \
}); \
RTC_CHECK(event.Wait(rtc::Event::kForever)); \
}
class VideoProcessorTest : public testing::Test {
protected:
VideoProcessorTest() {
// Get a codec configuration struct and configure it.
webrtc::test::CodecSettings(kVideoCodecVP8, &config_.codec_settings);
config_.codec_settings.width = kWidth;
config_.codec_settings.height = kHeight;
VideoProcessorTest() : q_("VP queue") {
config_.SetCodecSettings(kVideoCodecVP8, 1, 1, 1, false, false, false,
false, false, kWidth, kHeight);
stats_.resize(1);
@ -53,9 +62,16 @@ class VideoProcessorTest : public testing::Test {
ExpectInit();
EXPECT_CALL(frame_reader_mock_, FrameLength())
.WillRepeatedly(Return(kFrameSize));
video_processor_ = rtc::MakeUnique<VideoProcessor>(
&encoder_mock_, &decoders_, &frame_reader_mock_, config_, &stats_,
nullptr /* encoded_frame_writer */, nullptr /* decoded_frame_writer */);
DO_SYNC(q_, {
video_processor_ = rtc::MakeUnique<VideoProcessor>(
&encoder_mock_, &decoders_, &frame_reader_mock_, config_, &stats_,
nullptr /* encoded_frame_writer */,
nullptr /* decoded_frame_writer */);
});
}
~VideoProcessorTest() {
DO_SYNC(q_, { video_processor_.reset(); });
}
void ExpectInit() {
@ -72,6 +88,8 @@ class VideoProcessorTest : public testing::Test {
EXPECT_CALL(*decoder_mock_, RegisterDecodeCompleteCallback(_)).Times(1);
}
rtc::TaskQueue q_;
TestConfig config_;
MockVideoEncoder encoder_mock_;
@ -92,7 +110,7 @@ TEST_F(VideoProcessorTest, ProcessFrames_FixedFramerate) {
EXPECT_CALL(encoder_mock_, SetRateAllocation(_, kFramerateFps))
.Times(1)
.WillOnce(Return(0));
video_processor_->SetRates(kBitrateKbps, kFramerateFps);
DO_SYNC(q_, { video_processor_->SetRates(kBitrateKbps, kFramerateFps); });
EXPECT_CALL(frame_reader_mock_, ReadFrame())
.WillRepeatedly(Return(I420Buffer::Create(kWidth, kHeight)));
@ -100,13 +118,13 @@ TEST_F(VideoProcessorTest, ProcessFrames_FixedFramerate) {
encoder_mock_,
Encode(Property(&VideoFrame::timestamp, 1 * 90000 / kFramerateFps), _, _))
.Times(1);
video_processor_->ProcessFrame();
DO_SYNC(q_, { video_processor_->ProcessFrame(); });
EXPECT_CALL(
encoder_mock_,
Encode(Property(&VideoFrame::timestamp, 2 * 90000 / kFramerateFps), _, _))
.Times(1);
video_processor_->ProcessFrame();
DO_SYNC(q_, { video_processor_->ProcessFrame(); });
ExpectRelease();
}
@ -118,27 +136,28 @@ TEST_F(VideoProcessorTest, ProcessFrames_VariableFramerate) {
EXPECT_CALL(encoder_mock_, SetRateAllocation(_, kStartFramerateFps))
.Times(1)
.WillOnce(Return(0));
video_processor_->SetRates(kBitrateKbps, kStartFramerateFps);
DO_SYNC(q_,
{ video_processor_->SetRates(kBitrateKbps, kStartFramerateFps); });
EXPECT_CALL(frame_reader_mock_, ReadFrame())
.WillRepeatedly(Return(I420Buffer::Create(kWidth, kHeight)));
EXPECT_CALL(encoder_mock_,
Encode(Property(&VideoFrame::timestamp, kStartTimestamp), _, _))
.Times(1);
video_processor_->ProcessFrame();
DO_SYNC(q_, { video_processor_->ProcessFrame(); });
const int kNewFramerateFps = 13;
EXPECT_CALL(encoder_mock_, SetRateAllocation(_, kNewFramerateFps))
.Times(1)
.WillOnce(Return(0));
video_processor_->SetRates(kBitrateKbps, kNewFramerateFps);
DO_SYNC(q_, { video_processor_->SetRates(kBitrateKbps, kNewFramerateFps); });
EXPECT_CALL(encoder_mock_,
Encode(Property(&VideoFrame::timestamp,
kStartTimestamp + 90000 / kNewFramerateFps),
_, _))
.Times(1);
video_processor_->ProcessFrame();
DO_SYNC(q_, { video_processor_->ProcessFrame(); });
ExpectRelease();
}
@ -151,7 +170,7 @@ TEST_F(VideoProcessorTest, SetRates) {
Property(&BitrateAllocation::get_sum_kbps, kBitrateKbps),
kFramerateFps))
.Times(1);
video_processor_->SetRates(kBitrateKbps, kFramerateFps);
DO_SYNC(q_, { video_processor_->SetRates(kBitrateKbps, kFramerateFps); });
const int kNewBitrateKbps = 456;
const int kNewFramerateFps = 34;
@ -160,7 +179,8 @@ TEST_F(VideoProcessorTest, SetRates) {
Property(&BitrateAllocation::get_sum_kbps, kNewBitrateKbps),
kNewFramerateFps))
.Times(1);
video_processor_->SetRates(kNewBitrateKbps, kNewFramerateFps);
DO_SYNC(q_,
{ video_processor_->SetRates(kNewBitrateKbps, kNewFramerateFps); });
ExpectRelease();
}