From 0b8bfb9d98b7a2011d80bcdf3f430bc040370dad Mon Sep 17 00:00:00 2001 From: brandtr Date: Fri, 19 May 2017 06:51:42 -0700 Subject: [PATCH] Reuse allocated encoders in SimulcastEncoderAdapter. Prior to this change, the SimulcastEncoderAdapter would destroy and create encoders whenever it is being reinitialized. After this change, the SimulcastEncoderAdapter will cache the already allocated encoders, and reuse them after reinitialization. This change will help in reducing the number of PictureID "jumps" that have been seen around encoder reinitialization. TESTED=AppRTCMobile, Chrome desktop, and internal app, with forced encoder reinits every 30 frames and https://codereview.webrtc.org/2833493003/ applied. BUG=webrtc:7475 Review-Url: https://codereview.webrtc.org/2830793005 Cr-Commit-Position: refs/heads/master@{#18215} --- webrtc/modules/video_coding/BUILD.gn | 1 + .../codecs/vp8/simulcast_encoder_adapter.cc | 140 +++++++--- .../codecs/vp8/simulcast_encoder_adapter.h | 53 ++-- .../vp8/simulcast_encoder_adapter_unittest.cc | 240 +++++++++++++++- webrtc/video/BUILD.gn | 1 + webrtc/video/DEPS | 1 + webrtc/video/end_to_end_tests.cc | 256 +++++++++++++++++- 7 files changed, 620 insertions(+), 72 deletions(-) diff --git a/webrtc/modules/video_coding/BUILD.gn b/webrtc/modules/video_coding/BUILD.gn index 477e06444e..19942a12fc 100644 --- a/webrtc/modules/video_coding/BUILD.gn +++ b/webrtc/modules/video_coding/BUILD.gn @@ -231,6 +231,7 @@ rtc_static_library("webrtc_vp8") { "../..:webrtc_common", "../../api/video_codecs:video_codecs_api", "../../base:rtc_base_approved", + "../../base:rtc_task_queue", "../../common_video", "../../system_wrappers", ] diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc index 8cef40d42e..89aae1b66d 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc @@ -62,7 +62,7 @@ bool ValidSimulcastResolutions(const webrtc::VideoCodec& codec, } int VerifyCodec(const webrtc::VideoCodec* inst) { - if (inst == NULL) { + if (inst == nullptr) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } if (inst->maxFramerate < 1) { @@ -127,36 +127,49 @@ class TemporalLayersFactoryAdapter : public webrtc::TemporalLayersFactory { namespace webrtc { SimulcastEncoderAdapter::SimulcastEncoderAdapter(VideoEncoderFactory* factory) - : factory_(factory), + : inited_(0), + factory_(factory), encoded_complete_callback_(nullptr), implementation_name_("SimulcastEncoderAdapter") { + // The adapter is typically created on the worker thread, but operated on + // the encoder task queue. + encoder_queue_.Detach(); + memset(&codec_, 0, sizeof(webrtc::VideoCodec)); } SimulcastEncoderAdapter::~SimulcastEncoderAdapter() { - Release(); + RTC_DCHECK(!Initialized()); + DestroyStoredEncoders(); } int SimulcastEncoderAdapter::Release() { - // TODO(pbos): Keep the last encoder instance but call ::Release() on it, then - // re-use this instance in ::InitEncode(). This means that changing - // resolutions doesn't require reallocation of the first encoder, but only - // reinitialization, which makes sense. Then Destroy this instance instead in - // ~SimulcastEncoderAdapter(). + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); + while (!streaminfos_.empty()) { VideoEncoder* encoder = streaminfos_.back().encoder; - EncodedImageCallback* callback = streaminfos_.back().callback; encoder->Release(); - factory_->Destroy(encoder); - delete callback; - streaminfos_.pop_back(); + // Even though it seems very unlikely, there are no guarantees that the + // encoder will not call back after being Release()'d. Therefore, we disable + // the callbacks here. + encoder->RegisterEncodeCompleteCallback(nullptr); + streaminfos_.pop_back(); // Deletes callback adapter. + stored_encoders_.push(encoder); } + + // It's legal to move the encoder to another queue now. + encoder_queue_.Detach(); + + rtc::AtomicOps::ReleaseStore(&inited_, 0); + return WEBRTC_VIDEO_CODEC_OK; } int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, int number_of_cores, size_t max_payload_size) { + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); + if (number_of_cores < 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; } @@ -172,6 +185,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, } int number_of_streams = NumberOfStreams(*inst); + RTC_DCHECK_LE(number_of_streams, kMaxSimulcastStreams); const bool doing_simulcast = (number_of_streams > 1); if (doing_simulcast && !ValidSimulcastResolutions(*inst, number_of_streams)) { @@ -202,7 +216,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, start_bitrate_kbps = std::max(codec_.simulcastStream[i].minBitrate, start_bitrate_kbps); bool highest_resolution_stream = (i == (number_of_streams - 1)); - PopulateStreamCodec(&codec_, i, start_bitrate_kbps, + PopulateStreamCodec(codec_, i, start_bitrate_kbps, highest_resolution_stream, &stream_codec); } TemporalLayersFactoryAdapter tl_factory_adapter(i, @@ -214,7 +228,17 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, stream_codec.qpMax = kDefaultMaxQp; } - VideoEncoder* encoder = factory_->Create(); + // If an existing encoder instance exists, reuse it. + // TODO(brandtr): Set initial RTP state (e.g., picture_id/tl0_pic_idx) here, + // when we start storing that state outside the encoder wrappers. + VideoEncoder* encoder; + if (!stored_encoders_.empty()) { + encoder = stored_encoders_.top(); + stored_encoders_.pop(); + } else { + encoder = factory_->Create(); + } + ret = encoder->InitEncode(&stream_codec, number_of_cores, max_payload_size); if (ret < 0) { // Explicitly destroy the current encoder; because we haven't registered a @@ -223,21 +247,30 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, Release(); return ret; } - EncodedImageCallback* callback = new AdapterEncodedImageCallback(this, i); - encoder->RegisterEncodeCompleteCallback(callback); - streaminfos_.push_back(StreamInfo(encoder, callback, stream_codec.width, - stream_codec.height, - start_bitrate_kbps > 0)); - if (i != 0) + std::unique_ptr callback( + new AdapterEncodedImageCallback(this, i)); + encoder->RegisterEncodeCompleteCallback(callback.get()); + streaminfos_.emplace_back(encoder, std::move(callback), stream_codec.width, + stream_codec.height, start_bitrate_kbps > 0); + + if (i != 0) { implementation_name += ", "; + } implementation_name += streaminfos_[i].encoder->ImplementationName(); } + if (doing_simulcast) { implementation_name_ = "SimulcastEncoderAdapter (" + implementation_name + ")"; } else { implementation_name_ = implementation_name; } + + // To save memory, don't store encoders that we don't use. + DestroyStoredEncoders(); + + rtc::AtomicOps::ReleaseStore(&inited_, 1); + return WEBRTC_VIDEO_CODEC_OK; } @@ -245,10 +278,12 @@ int SimulcastEncoderAdapter::Encode( const VideoFrame& input_image, const CodecSpecificInfo* codec_specific_info, const std::vector* frame_types) { + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); + if (!Initialized()) { return WEBRTC_VIDEO_CODEC_UNINITIALIZED; } - if (encoded_complete_callback_ == NULL) { + if (encoded_complete_callback_ == nullptr) { return WEBRTC_VIDEO_CODEC_UNINITIALIZED; } @@ -275,8 +310,9 @@ int SimulcastEncoderAdapter::Encode( int src_height = input_image.height(); for (size_t stream_idx = 0; stream_idx < streaminfos_.size(); ++stream_idx) { // Don't encode frames in resolutions that we don't intend to send. - if (!streaminfos_[stream_idx].send_stream) + if (!streaminfos_[stream_idx].send_stream) { continue; + } std::vector stream_frame_types; if (send_key_frame) { @@ -338,12 +374,14 @@ int SimulcastEncoderAdapter::Encode( int SimulcastEncoderAdapter::RegisterEncodeCompleteCallback( EncodedImageCallback* callback) { + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); encoded_complete_callback_ = callback; return WEBRTC_VIDEO_CODEC_OK; } int SimulcastEncoderAdapter::SetChannelParameters(uint32_t packet_loss, int64_t rtt) { + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); for (size_t stream_idx = 0; stream_idx < streaminfos_.size(); ++stream_idx) { streaminfos_[stream_idx].encoder->SetChannelParameters(packet_loss, rtt); } @@ -352,20 +390,26 @@ int SimulcastEncoderAdapter::SetChannelParameters(uint32_t packet_loss, int SimulcastEncoderAdapter::SetRateAllocation(const BitrateAllocation& bitrate, uint32_t new_framerate) { - if (!Initialized()) + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); + + if (!Initialized()) { return WEBRTC_VIDEO_CODEC_UNINITIALIZED; + } - if (new_framerate < 1) + if (new_framerate < 1) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; + } - if (codec_.maxBitrate > 0 && bitrate.get_sum_kbps() > codec_.maxBitrate) + if (codec_.maxBitrate > 0 && bitrate.get_sum_kbps() > codec_.maxBitrate) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; + } if (bitrate.get_sum_bps() > 0) { // Make sure the bitrate fits the configured min bitrates. 0 is a special // value that means paused, though, so leave it alone. - if (bitrate.get_sum_kbps() < codec_.minBitrate) + if (bitrate.get_sum_kbps() < codec_.minBitrate) { return WEBRTC_VIDEO_CODEC_ERR_PARAMETER; + } if (codec_.numberOfSimulcastStreams > 0 && bitrate.get_sum_kbps() < codec_.simulcastStream[0].minBitrate) { @@ -388,8 +432,9 @@ int SimulcastEncoderAdapter::SetRateAllocation(const BitrateAllocation& bitrate, // Slice the temporal layers out of the full allocation and pass it on to // the encoder handling the current simulcast stream. BitrateAllocation stream_allocation; - for (int i = 0; i < kMaxTemporalStreams; ++i) + for (int i = 0; i < kMaxTemporalStreams; ++i) { stream_allocation.SetBitrate(0, i, bitrate.GetBitrate(stream_idx, i)); + } streaminfos_[stream_idx].encoder->SetRateAllocation(stream_allocation, new_framerate); } @@ -397,6 +442,8 @@ int SimulcastEncoderAdapter::SetRateAllocation(const BitrateAllocation& bitrate, return WEBRTC_VIDEO_CODEC_OK; } +// TODO(brandtr): Add task checker to this member function, when all encoder +// callbacks are coming in on the encoder queue. EncodedImageCallback::Result SimulcastEncoderAdapter::OnEncodedImage( size_t stream_idx, const EncodedImage& encodedImage, @@ -412,24 +459,25 @@ EncodedImageCallback::Result SimulcastEncoderAdapter::OnEncodedImage( } void SimulcastEncoderAdapter::PopulateStreamCodec( - const webrtc::VideoCodec* inst, + const webrtc::VideoCodec& inst, int stream_index, uint32_t start_bitrate_kbps, bool highest_resolution_stream, webrtc::VideoCodec* stream_codec) { - *stream_codec = *inst; + *stream_codec = inst; // Stream specific settings. stream_codec->VP8()->numberOfTemporalLayers = - inst->simulcastStream[stream_index].numberOfTemporalLayers; + inst.simulcastStream[stream_index].numberOfTemporalLayers; stream_codec->numberOfSimulcastStreams = 0; - stream_codec->width = inst->simulcastStream[stream_index].width; - stream_codec->height = inst->simulcastStream[stream_index].height; - stream_codec->maxBitrate = inst->simulcastStream[stream_index].maxBitrate; - stream_codec->minBitrate = inst->simulcastStream[stream_index].minBitrate; - stream_codec->qpMax = inst->simulcastStream[stream_index].qpMax; + stream_codec->width = inst.simulcastStream[stream_index].width; + stream_codec->height = inst.simulcastStream[stream_index].height; + stream_codec->maxBitrate = inst.simulcastStream[stream_index].maxBitrate; + stream_codec->minBitrate = inst.simulcastStream[stream_index].minBitrate; + stream_codec->qpMax = inst.simulcastStream[stream_index].qpMax; // Settings that are based on stream/resolution. - if (stream_index == 0) { + const bool lowest_resolution_stream = (stream_index == 0); + if (lowest_resolution_stream) { // Settings for lowest spatial resolutions. stream_codec->qpMax = kLowestResMaxQp; } @@ -449,28 +497,42 @@ void SimulcastEncoderAdapter::PopulateStreamCodec( } bool SimulcastEncoderAdapter::Initialized() const { - return !streaminfos_.empty(); + return rtc::AtomicOps::AcquireLoad(&inited_) == 1; +} + +void SimulcastEncoderAdapter::DestroyStoredEncoders() { + while (!stored_encoders_.empty()) { + VideoEncoder* encoder = stored_encoders_.top(); + factory_->Destroy(encoder); + stored_encoders_.pop(); + } } bool SimulcastEncoderAdapter::SupportsNativeHandle() const { + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); // We should not be calling this method before streaminfos_ are configured. RTC_DCHECK(!streaminfos_.empty()); for (const auto& streaminfo : streaminfos_) { - if (!streaminfo.encoder->SupportsNativeHandle()) + if (!streaminfo.encoder->SupportsNativeHandle()) { return false; + } } return true; } VideoEncoder::ScalingSettings SimulcastEncoderAdapter::GetScalingSettings() const { + // TODO(brandtr): Investigate why the sequence checker below fails on mac. + // RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); // Turn off quality scaling for simulcast. - if (!Initialized() || NumberOfStreams(codec_) != 1) + if (!Initialized() || NumberOfStreams(codec_) != 1) { return VideoEncoder::ScalingSettings(false); + } return streaminfos_[0].encoder->GetScalingSettings(); } const char* SimulcastEncoderAdapter::ImplementationName() const { + RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); return implementation_name_.c_str(); } diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h index 2be8779d62..744c66a69e 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h @@ -13,15 +13,21 @@ #define WEBRTC_MODULES_VIDEO_CODING_CODECS_VP8_SIMULCAST_ENCODER_ADAPTER_H_ #include +#include #include +#include #include +#include "webrtc/base/atomicops.h" +#include "webrtc/base/sequenced_task_checker.h" #include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h" namespace webrtc { class SimulcastRateAllocator; +// TODO(brandtr): Remove this class and replace its use with a +// WebRtcVideoEncoderFactory. class VideoEncoderFactory { public: virtual VideoEncoder* Create() = 0; @@ -31,14 +37,16 @@ class VideoEncoderFactory { // SimulcastEncoderAdapter implements simulcast support by creating multiple // webrtc::VideoEncoder instances with the given VideoEncoderFactory. -// All the public interfaces are expected to be called from the same thread, -// e.g the encoder thread. +// The object is created and destroyed on the worker thread, but all public +// interfaces should be called from the encoder task queue. class SimulcastEncoderAdapter : public VP8Encoder { public: + // TODO(brandtr): Make it clear that the ownership of |factory| is transferred + // by only accepting a std::unique_ptr here. explicit SimulcastEncoderAdapter(VideoEncoderFactory* factory); virtual ~SimulcastEncoderAdapter(); - // Implements VideoEncoder + // Implements VideoEncoder. int Release() override; int InitEncode(const VideoCodec* inst, int number_of_cores, @@ -67,47 +75,50 @@ class SimulcastEncoderAdapter : public VP8Encoder { private: struct StreamInfo { - StreamInfo() - : encoder(NULL), - callback(NULL), - width(0), - height(0), - key_frame_request(false), - send_stream(true) {} StreamInfo(VideoEncoder* encoder, - EncodedImageCallback* callback, + std::unique_ptr callback, uint16_t width, uint16_t height, bool send_stream) : encoder(encoder), - callback(callback), + callback(std::move(callback)), width(width), height(height), key_frame_request(false), send_stream(send_stream) {} - // Deleted by SimulcastEncoderAdapter::Release(). + // Deleted by SimulcastEncoderAdapter::DestroyStoredEncoders(). VideoEncoder* encoder; - EncodedImageCallback* callback; + std::unique_ptr callback; uint16_t width; uint16_t height; bool key_frame_request; bool send_stream; }; - // Populate the codec settings for each stream. - void PopulateStreamCodec(const webrtc::VideoCodec* inst, - int stream_index, - uint32_t start_bitrate_kbps, - bool highest_resolution_stream, - webrtc::VideoCodec* stream_codec); + // Populate the codec settings for each simulcast stream. + static void PopulateStreamCodec(const webrtc::VideoCodec& inst, + int stream_index, + uint32_t start_bitrate_kbps, + bool highest_resolution_stream, + webrtc::VideoCodec* stream_codec); bool Initialized() const; - std::unique_ptr factory_; + void DestroyStoredEncoders(); + + volatile int inited_; // Accessed atomically. + const std::unique_ptr factory_; VideoCodec codec_; std::vector streaminfos_; EncodedImageCallback* encoded_complete_callback_; std::string implementation_name_; + + // Used for checking the single-threaded access of the encoder interface. + rtc::SequencedTaskChecker encoder_queue_; + + // Store encoders in between calls to Release and InitEncode, so they don't + // have to be recreated. Remaining encoders are destroyed by the destructor. + std::stack stored_encoders_; }; } // namespace webrtc diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc index 83e633eb29..99387c9c74 100644 --- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc +++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc @@ -8,6 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include #include #include @@ -117,7 +118,7 @@ class MockVideoEncoder : public VideoEncoder { return 0; } - int32_t Release() /* override */ { return 0; } + MOCK_METHOD0(Release, int32_t()); int32_t SetRateAllocation(const BitrateAllocation& bitrate_allocation, uint32_t framerate) { @@ -142,7 +143,7 @@ class MockVideoEncoder : public VideoEncoder { image._encodedHeight = height; CodecSpecificInfo codec_specific_info; memset(&codec_specific_info, 0, sizeof(codec_specific_info)); - callback_->OnEncodedImage(image, &codec_specific_info, NULL); + callback_->OnEncodedImage(image, &codec_specific_info, nullptr); } void set_supports_native_handle(bool enabled) { @@ -243,7 +244,11 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, last_encoded_image_width_(-1), last_encoded_image_height_(-1), last_encoded_image_simulcast_index_(-1) {} - virtual ~TestSimulcastEncoderAdapterFake() {} + virtual ~TestSimulcastEncoderAdapterFake() { + if (adapter_) { + adapter_->Release(); + } + } Result OnEncodedImage(const EncodedImage& encoded_image, const CodecSpecificInfo* codec_specific_info, @@ -367,6 +372,17 @@ TEST_F(TestSimulcastEncoderAdapterFake, InitEncode) { VerifyCodecSettings(); } +TEST_F(TestSimulcastEncoderAdapterFake, ReleaseWithoutInitEncode) { + EXPECT_EQ(0, adapter_->Release()); +} + +TEST_F(TestSimulcastEncoderAdapterFake, Reinit) { + SetupCodec(); + EXPECT_EQ(0, adapter_->Release()); + + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); +} + TEST_F(TestSimulcastEncoderAdapterFake, SetChannelParameters) { SetupCodec(); const uint32_t packetLoss = 5; @@ -386,8 +402,9 @@ TEST_F(TestSimulcastEncoderAdapterFake, EncodedCallbackForDifferentEncoders) { // resolutions, to test that the adapter forwards on the correct resolution // and simulcast index values, going only off the encoder that generates the // image. - EXPECT_EQ(3u, helper_->factory()->encoders().size()); - helper_->factory()->encoders()[0]->SendEncodedImage(1152, 704); + std::vector encoders = helper_->factory()->encoders(); + ASSERT_EQ(3u, encoders.size()); + encoders[0]->SendEncodedImage(1152, 704); int width; int height; int simulcast_index; @@ -396,19 +413,224 @@ TEST_F(TestSimulcastEncoderAdapterFake, EncodedCallbackForDifferentEncoders) { EXPECT_EQ(704, height); EXPECT_EQ(0, simulcast_index); - helper_->factory()->encoders()[1]->SendEncodedImage(300, 620); + encoders[1]->SendEncodedImage(300, 620); EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index)); EXPECT_EQ(300, width); EXPECT_EQ(620, height); EXPECT_EQ(1, simulcast_index); - helper_->factory()->encoders()[2]->SendEncodedImage(120, 240); + encoders[2]->SendEncodedImage(120, 240); EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index)); EXPECT_EQ(120, width); EXPECT_EQ(240, height); EXPECT_EQ(2, simulcast_index); } +// This test verifies that the underlying encoders are reused, when the adapter +// is reinited with different number of simulcast streams. It further checks +// that the allocated encoders are reused in the same order as before, starting +// with the lowest stream. +TEST_F(TestSimulcastEncoderAdapterFake, ReusesEncodersInOrder) { + // Set up common settings for three streams. + TestVp8Simulcast::DefaultSettings( + &codec_, static_cast(kTestTemporalLayerProfile)); + rate_allocator_.reset(new SimulcastRateAllocator(codec_, nullptr)); + tl_factory_.SetListener(rate_allocator_.get()); + codec_.VP8()->tl_factory = &tl_factory_; + adapter_->RegisterEncodeCompleteCallback(this); + + // Input data. + rtc::scoped_refptr buffer(I420Buffer::Create(1280, 720)); + VideoFrame input_frame(buffer, 100, 1000, kVideoRotation_180); + std::vector frame_types; + + // Encode with three streams. + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + VerifyCodecSettings(); + std::vector original_encoders = + helper_->factory()->encoders(); + ASSERT_EQ(3u, original_encoders.size()); + EXPECT_CALL(*original_encoders[0], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_CALL(*original_encoders[1], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_CALL(*original_encoders[2], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + frame_types.resize(3, kVideoFrameKey); + EXPECT_EQ(0, adapter_->Encode(input_frame, nullptr, &frame_types)); + EXPECT_CALL(*original_encoders[0], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_CALL(*original_encoders[1], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_CALL(*original_encoders[2], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_EQ(0, adapter_->Release()); + + // Encode with two streams. + codec_.width /= 2; + codec_.height /= 2; + codec_.numberOfSimulcastStreams = 2; + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + std::vector new_encoders = helper_->factory()->encoders(); + ASSERT_EQ(2u, new_encoders.size()); + ASSERT_EQ(original_encoders[0], new_encoders[0]); + EXPECT_CALL(*original_encoders[0], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + ASSERT_EQ(original_encoders[1], new_encoders[1]); + EXPECT_CALL(*original_encoders[1], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + frame_types.resize(2, kVideoFrameKey); + EXPECT_EQ(0, adapter_->Encode(input_frame, nullptr, &frame_types)); + EXPECT_CALL(*original_encoders[0], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_CALL(*original_encoders[1], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_EQ(0, adapter_->Release()); + + // Encode with single stream. + codec_.width /= 2; + codec_.height /= 2; + codec_.numberOfSimulcastStreams = 1; + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + new_encoders = helper_->factory()->encoders(); + ASSERT_EQ(1u, new_encoders.size()); + ASSERT_EQ(original_encoders[0], new_encoders[0]); + EXPECT_CALL(*original_encoders[0], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + frame_types.resize(1, kVideoFrameKey); + EXPECT_EQ(0, adapter_->Encode(input_frame, nullptr, &frame_types)); + EXPECT_CALL(*original_encoders[0], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_EQ(0, adapter_->Release()); + + // Encode with three streams, again. + codec_.width *= 4; + codec_.height *= 4; + codec_.numberOfSimulcastStreams = 3; + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + new_encoders = helper_->factory()->encoders(); + ASSERT_EQ(3u, new_encoders.size()); + // The first encoder is reused. + ASSERT_EQ(original_encoders[0], new_encoders[0]); + EXPECT_CALL(*original_encoders[0], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + // The second and third encoders are new. + EXPECT_CALL(*new_encoders[1], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_CALL(*new_encoders[2], Encode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + frame_types.resize(3, kVideoFrameKey); + EXPECT_EQ(0, adapter_->Encode(input_frame, nullptr, &frame_types)); + EXPECT_CALL(*original_encoders[0], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_CALL(*new_encoders[1], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_CALL(*new_encoders[2], Release()) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + EXPECT_EQ(0, adapter_->Release()); +} + +TEST_F(TestSimulcastEncoderAdapterFake, DoesNotLeakEncoders) { + SetupCodec(); + VerifyCodecSettings(); + + EXPECT_EQ(3u, helper_->factory()->encoders().size()); + + // The adapter should destroy all encoders it has allocated. Since + // |helper_->factory()| is owned by |adapter_|, however, we need to rely on + // lsan to find leaks here. + EXPECT_EQ(0, adapter_->Release()); + adapter_.reset(); +} + +// This test verifies that an adapter reinit with the same codec settings as +// before does not change the underlying encoder codec settings. +TEST_F(TestSimulcastEncoderAdapterFake, ReinitDoesNotReorderEncoderSettings) { + SetupCodec(); + VerifyCodecSettings(); + + // Capture current codec settings. + std::vector encoders = helper_->factory()->encoders(); + ASSERT_EQ(3u, encoders.size()); + std::array codecs_before; + for (int i = 0; i < 3; ++i) { + codecs_before[i] = encoders[i]->codec(); + } + + // Reinitialize and verify that the new codec settings are the same. + EXPECT_EQ(0, adapter_->Release()); + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + for (int i = 0; i < 3; ++i) { + const VideoCodec& codec_before = codecs_before[i]; + const VideoCodec& codec_after = encoders[i]->codec(); + + // webrtc::VideoCodec does not implement operator==. + EXPECT_EQ(codec_before.codecType, codec_after.codecType); + EXPECT_EQ(codec_before.plType, codec_after.plType); + EXPECT_EQ(codec_before.width, codec_after.width); + EXPECT_EQ(codec_before.height, codec_after.height); + EXPECT_EQ(codec_before.startBitrate, codec_after.startBitrate); + EXPECT_EQ(codec_before.maxBitrate, codec_after.maxBitrate); + EXPECT_EQ(codec_before.minBitrate, codec_after.minBitrate); + EXPECT_EQ(codec_before.targetBitrate, codec_after.targetBitrate); + EXPECT_EQ(codec_before.maxFramerate, codec_after.maxFramerate); + EXPECT_EQ(codec_before.qpMax, codec_after.qpMax); + EXPECT_EQ(codec_before.numberOfSimulcastStreams, + codec_after.numberOfSimulcastStreams); + EXPECT_EQ(codec_before.mode, codec_after.mode); + EXPECT_EQ(codec_before.expect_encode_from_texture, + codec_after.expect_encode_from_texture); + } +} + +// This test is similar to the one above, except that it tests the simulcastIdx +// from the CodecSpecificInfo that is connected to an encoded frame. The +// PayloadRouter demuxes the incoming encoded frames on different RTP modules +// using the simulcastIdx, so it's important that there is no corresponding +// encoder reordering in between adapter reinits as this would lead to PictureID +// discontinuities. +TEST_F(TestSimulcastEncoderAdapterFake, ReinitDoesNotReorderFrameSimulcastIdx) { + SetupCodec(); + adapter_->SetRateAllocation(rate_allocator_->GetAllocation(1200, 30), 30); + VerifyCodecSettings(); + + // Send frames on all streams. + std::vector encoders = helper_->factory()->encoders(); + ASSERT_EQ(3u, encoders.size()); + encoders[0]->SendEncodedImage(1152, 704); + int width; + int height; + int simulcast_index; + EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index)); + EXPECT_EQ(0, simulcast_index); + + encoders[1]->SendEncodedImage(300, 620); + EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index)); + EXPECT_EQ(1, simulcast_index); + + encoders[2]->SendEncodedImage(120, 240); + EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index)); + EXPECT_EQ(2, simulcast_index); + + // Reinitialize. + EXPECT_EQ(0, adapter_->Release()); + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + adapter_->SetRateAllocation(rate_allocator_->GetAllocation(1200, 30), 30); + + // Verify that the same encoder sends out frames on the same simulcast index. + encoders[0]->SendEncodedImage(1152, 704); + EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index)); + EXPECT_EQ(0, simulcast_index); + + encoders[1]->SendEncodedImage(300, 620); + EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index)); + EXPECT_EQ(1, simulcast_index); + + encoders[2]->SendEncodedImage(120, 240); + EXPECT_TRUE(GetLastEncodedImageInfo(&width, &height, &simulcast_index)); + EXPECT_EQ(2, simulcast_index); +} + TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleForSingleStreams) { TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); @@ -467,7 +689,7 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsImplementationName) { adapter_->ImplementationName()); // Single streams should not expose "SimulcastEncoderAdapter" in name. - adapter_->Release(); + EXPECT_EQ(0, adapter_->Release()); codec_.numberOfSimulcastStreams = 1; EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); adapter_->RegisterEncodeCompleteCallback(this); @@ -528,7 +750,7 @@ TEST_F(TestSimulcastEncoderAdapterFake, for (MockVideoEncoder* encoder : helper_->factory()->encoders()) EXPECT_CALL(*encoder, Encode(::testing::Ref(input_frame), _, _)).Times(1); std::vector frame_types(3, kVideoFrameKey); - EXPECT_EQ(0, adapter_->Encode(input_frame, NULL, &frame_types)); + EXPECT_EQ(0, adapter_->Encode(input_frame, nullptr, &frame_types)); } TEST_F(TestSimulcastEncoderAdapterFake, TestFailureReturnCodesFromEncodeCalls) { diff --git a/webrtc/video/BUILD.gn b/webrtc/video/BUILD.gn index 3c3b17e2d2..19d0e93b66 100644 --- a/webrtc/video/BUILD.gn +++ b/webrtc/video/BUILD.gn @@ -258,6 +258,7 @@ if (rtc_include_tests) { "../call:call_interfaces", "../common_video", "../logging:rtc_event_log_api", + "../media:rtc_media", "../media:rtc_media_base", "../media:rtc_media_tests_utils", "../modules:module_api", diff --git a/webrtc/video/DEPS b/webrtc/video/DEPS index 1cdad193e3..258bb92954 100644 --- a/webrtc/video/DEPS +++ b/webrtc/video/DEPS @@ -4,6 +4,7 @@ include_rules = [ "+webrtc/common_video", "+webrtc/logging/rtc_event_log", "+webrtc/media/base", + "+webrtc/media/engine", "+webrtc/modules/audio_mixer", "+webrtc/modules/bitrate_controller", "+webrtc/modules/congestion_controller", diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 6e721b60ee..3324bbf5f6 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -26,14 +26,19 @@ #include "webrtc/common_video/include/frame_callback.h" #include "webrtc/logging/rtc_event_log/rtc_event_log.h" #include "webrtc/media/base/fakevideorenderer.h" +#include "webrtc/media/base/mediaconstants.h" +#include "webrtc/media/engine/internalencoderfactory.h" +#include "webrtc/media/engine/webrtcvideoencoderfactory.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/nack.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_format.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" #include "webrtc/modules/video_coding/codecs/h264/include/h264.h" #include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h" +#include "webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h" #include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h" #include "webrtc/modules/video_coding/include/video_coding_defines.h" #include "webrtc/system_wrappers/include/metrics.h" @@ -47,8 +52,8 @@ #include "webrtc/test/field_trial.h" #include "webrtc/test/frame_generator.h" #include "webrtc/test/frame_generator_capturer.h" -#include "webrtc/test/gtest.h" #include "webrtc/test/gmock.h" +#include "webrtc/test/gtest.h" #include "webrtc/test/null_transport.h" #include "webrtc/test/rtcp_packet_parser.h" #include "webrtc/test/rtp_rtcp_observer.h" @@ -132,6 +137,7 @@ class EndToEndTest : public test::CallTest { void RespectsRtcpMode(RtcpMode rtcp_mode); void TestSendsSetSsrcs(size_t num_ssrcs, bool send_single_ssrc_first); void TestRtpStatePreservation(bool use_rtx, bool provoke_rtcpsr_before_rtp); + void TestPictureIdStatePreservation(VideoEncoder* encoder); void VerifyHistogramStats(bool use_rtx, bool use_red, bool screenshare); void VerifyNewVideoSendStreamsRespectNetworkState( MediaType network_to_bring_up, @@ -3674,8 +3680,8 @@ TEST_F(EndToEndTest, DISABLED_RedundantPayloadsTransmittedOnAllSsrcs) { void EndToEndTest::TestRtpStatePreservation(bool use_rtx, bool provoke_rtcpsr_before_rtp) { - // This test use other VideoStream settings than the the default settings - // implemented in DefaultVideoStreamFactory. Therefore this test implement + // This test uses other VideoStream settings than the the default settings + // implemented in DefaultVideoStreamFactory. Therefore this test implements // its own VideoEncoderConfig::VideoStreamFactoryInterface which is created // in ModifyVideoConfigs. class VideoStreamFactory @@ -3925,6 +3931,250 @@ TEST_F(EndToEndTest, RestartingSendStreamKeepsRtpAndRtcpTimestampsSynced) { TestRtpStatePreservation(true, true); } +void EndToEndTest::TestPictureIdStatePreservation(VideoEncoder* encoder) { + const size_t kFrameMaxWidth = 1280; + const size_t kFrameMaxHeight = 720; + const size_t kFrameRate = 30; + + // Use a special stream factory in this test to ensure that all simulcast + // streams are being sent. + class VideoStreamFactory + : public VideoEncoderConfig::VideoStreamFactoryInterface { + public: + VideoStreamFactory() = default; + + private: + std::vector CreateEncoderStreams( + int width, + int height, + const VideoEncoderConfig& encoder_config) override { + std::vector streams = + test::CreateVideoStreams(width, height, encoder_config); + + const size_t kBitrate = 100000; + + if (encoder_config.number_of_streams > 1) { + RTC_DCHECK_EQ(3, encoder_config.number_of_streams); + + for (size_t i = 0; i < encoder_config.number_of_streams; ++i) { + streams[i].min_bitrate_bps = kBitrate; + streams[i].target_bitrate_bps = kBitrate; + streams[i].max_bitrate_bps = kBitrate; + } + + // test::CreateVideoStreams does not return frame sizes for the lower + // streams that are accepted by VP8Impl::InitEncode. + // TODO(brandtr): Fix the problem in test::CreateVideoStreams, rather + // than overriding the values here. + streams[1].width = streams[2].width / 2; + streams[1].height = streams[2].height / 2; + streams[0].width = streams[1].width / 2; + streams[0].height = streams[1].height / 2; + } else { + // Use the same total bitrates when sending a single stream to avoid + // lowering the bitrate estimate and requiring a subsequent rampup. + streams[0].min_bitrate_bps = 3 * kBitrate; + streams[0].target_bitrate_bps = 3 * kBitrate; + streams[0].max_bitrate_bps = 3 * kBitrate; + } + + return streams; + } + }; + + class PictureIdObserver : public test::RtpRtcpObserver { + public: + PictureIdObserver() + : test::RtpRtcpObserver(kDefaultTimeoutMs), num_ssrcs_to_observe_(1) {} + + void ResetExpectations(size_t num_expected_ssrcs) { + rtc::CritScope lock(&crit_); + // Do not clear the timestamp and picture_id, to ensure that we check + // consistency between reinits and recreations. + num_packets_sent_.clear(); + num_ssrcs_to_observe_ = num_expected_ssrcs; + ssrc_observed_.clear(); + } + + private: + Action OnSendRtp(const uint8_t* packet, size_t length) override { + rtc::CritScope lock(&crit_); + + // RTP header. + RTPHeader header; + EXPECT_TRUE(parser_->Parse(packet, length, &header)); + const uint32_t timestamp = header.timestamp; + const uint32_t ssrc = header.ssrc; + + const bool known_ssrc = + (ssrc == kVideoSendSsrcs[0] || ssrc == kVideoSendSsrcs[1] || + ssrc == kVideoSendSsrcs[2]); + EXPECT_TRUE(known_ssrc) << "Unknown SSRC sent."; + + const bool is_padding = + (length == header.headerLength + header.paddingLength); + if (is_padding) { + return SEND_PACKET; + } + + // VP8 header. + std::unique_ptr depacketizer( + RtpDepacketizer::Create(kRtpVideoVp8)); + RtpDepacketizer::ParsedPayload parsed_payload; + EXPECT_TRUE(depacketizer->Parse( + &parsed_payload, &packet[header.headerLength], + length - header.headerLength - header.paddingLength)); + const uint16_t picture_id = + parsed_payload.type.Video.codecHeader.VP8.pictureId; + + // If this is the first packet, we have nothing to compare to. + if (last_observed_timestamp_.find(ssrc) == + last_observed_timestamp_.end()) { + last_observed_timestamp_[ssrc] = timestamp; + last_observed_picture_id_[ssrc] = picture_id; + ++num_packets_sent_[ssrc]; + + return SEND_PACKET; + } + + // Verify continuity and monotonicity of picture_id sequence. + if (last_observed_timestamp_[ssrc] == timestamp) { + // Packet belongs to same frame as before. + EXPECT_EQ(last_observed_picture_id_[ssrc], picture_id); + } else { + // Packet is a new frame. + EXPECT_EQ((last_observed_picture_id_[ssrc] + 1) % (1 << 15), + picture_id); + } + last_observed_timestamp_[ssrc] = timestamp; + last_observed_picture_id_[ssrc] = picture_id; + + // Pass the test when enough media packets have been received + // on all streams. + if (++num_packets_sent_[ssrc] >= 10 && !ssrc_observed_[ssrc]) { + ssrc_observed_[ssrc] = true; + if (--num_ssrcs_to_observe_ == 0) { + observation_complete_.Set(); + } + } + + return SEND_PACKET; + } + + rtc::CriticalSection crit_; + std::map last_observed_timestamp_ GUARDED_BY(crit_); + std::map last_observed_picture_id_ GUARDED_BY(crit_); + std::map num_packets_sent_ GUARDED_BY(crit_); + size_t num_ssrcs_to_observe_ GUARDED_BY(crit_); + std::map ssrc_observed_ GUARDED_BY(crit_); + } observer; + + Call::Config config(event_log_.get()); + CreateCalls(config, config); + + test::PacketTransport send_transport( + sender_call_.get(), &observer, test::PacketTransport::kSender, + payload_type_map_, FakeNetworkPipe::Config()); + test::PacketTransport receive_transport( + nullptr, &observer, test::PacketTransport::kReceiver, payload_type_map_, + FakeNetworkPipe::Config()); + send_transport.SetReceiver(receiver_call_->Receiver()); + receive_transport.SetReceiver(sender_call_->Receiver()); + + CreateSendConfig(kNumSsrcs, 0, 0, &send_transport); + video_send_config_.encoder_settings.encoder = encoder; + video_send_config_.encoder_settings.payload_name = "VP8"; + video_encoder_config_.video_stream_factory = + new rtc::RefCountedObject(); + video_encoder_config_.number_of_streams = 1; + CreateMatchingReceiveConfigs(&receive_transport); + + CreateVideoStreams(); + CreateFrameGeneratorCapturer(kFrameRate, kFrameMaxWidth, kFrameMaxHeight); + + auto reinit_encoder_and_test = [this, &observer](int num_expected_ssrcs) { + video_send_stream_->ReconfigureVideoEncoder(video_encoder_config_.Copy()); + observer.ResetExpectations(num_expected_ssrcs); + EXPECT_TRUE(observer.Wait()) << "Timed out waiting for packets."; + }; + + // TODO(brandtr): Add tests where we recreate the whole VideoSendStream. This + // requires synchronizing the frame generator output with the packetization + // output, to not have any timing-dependent gaps in the picture_id sequence. + + // Initial test with a single stream. + Start(); + EXPECT_TRUE(observer.Wait()) << "Timed out waiting for packets."; + + // Reinit the encoder and make sure the picture_id sequence is continuous. + reinit_encoder_and_test(1); + + // Go up to three streams. + video_encoder_config_.number_of_streams = 3; + reinit_encoder_and_test(3); + reinit_encoder_and_test(3); + + // Go back to one stream. + video_encoder_config_.number_of_streams = 1; + reinit_encoder_and_test(1); + reinit_encoder_and_test(1); + + send_transport.StopSending(); + receive_transport.StopSending(); + + Stop(); + DestroyStreams(); +} + +// These tests exposed a race in libvpx, see +// https://bugs.chromium.org/p/webrtc/issues/detail?id=7663. Disabling the tests +// on tsan until the race has been fixed. +#if defined(THREAD_SANITIZER) +#define MAYBE_PictureIdStateRetainedAfterReinitingVp8 \ + DISABLED_PictureIdStateRetainedAfterReinitingVp8 +#define MAYBE_PictureIdStateRetainedAfterReinitingSimulcastEncoderAdapter \ + DISABLED_PictureIdStateRetainedAfterReinitingSimulcastEncoderAdapter +#else +#define MAYBE_PictureIdStateRetainedAfterReinitingVp8 \ + PictureIdStateRetainedAfterReinitingVp8 +#define MAYBE_PictureIdStateRetainedAfterReinitingSimulcastEncoderAdapter \ + PictureIdStateRetainedAfterReinitingSimulcastEncoderAdapter +#endif +TEST_F(EndToEndTest, MAYBE_PictureIdStateRetainedAfterReinitingVp8) { + std::unique_ptr encoder(VP8Encoder::Create()); + TestPictureIdStatePreservation(encoder.get()); +} + +TEST_F(EndToEndTest, + MAYBE_PictureIdStateRetainedAfterReinitingSimulcastEncoderAdapter) { + class VideoEncoderFactoryAdapter : public webrtc::VideoEncoderFactory { + public: + explicit VideoEncoderFactoryAdapter( + cricket::WebRtcVideoEncoderFactory* factory) + : factory_(factory) {} + virtual ~VideoEncoderFactoryAdapter() {} + + // Implements webrtc::VideoEncoderFactory. + webrtc::VideoEncoder* Create() override { + return factory_->CreateVideoEncoder( + cricket::VideoCodec(cricket::kVp8CodecName)); + } + + void Destroy(webrtc::VideoEncoder* encoder) override { + return factory_->DestroyVideoEncoder(encoder); + } + + private: + cricket::WebRtcVideoEncoderFactory* const factory_; + }; + + cricket::InternalEncoderFactory internal_encoder_factory; + SimulcastEncoderAdapter simulcast_encoder_adapter( + new VideoEncoderFactoryAdapter(&internal_encoder_factory)); + + TestPictureIdStatePreservation(&simulcast_encoder_adapter); +} + TEST_F(EndToEndTest, RespectsNetworkState) { // TODO(pbos): Remove accepted downtime packets etc. when signaling network // down blocks until no more packets will be sent.