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.