From bf3dbb4a692f4be31d4dd025fcf57b8c0a88c547 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 16 Mar 2018 13:38:46 +0100 Subject: [PATCH] Delete payload_type from VCMEncoderDatabase and vcm::VideoSender. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:8830 Change-Id: Ie6a874023618a5540e138b34edfcad1ce6e8d391 Reviewed-on: https://webrtc-review.googlesource.com/62102 Commit-Queue: Niels Moller Reviewed-by: Erik SprÃ¥ng Cr-Commit-Position: refs/heads/master@{#22474} --- modules/video_coding/encoder_database.cc | 28 ++++--------------- modules/video_coding/encoder_database.h | 8 ++---- modules/video_coding/video_coding_impl.cc | 4 +-- modules/video_coding/video_coding_impl.h | 1 - modules/video_coding/video_sender.cc | 9 ++---- modules/video_coding/video_sender_unittest.cc | 12 ++++---- video/video_stream_encoder.cc | 5 ++-- 7 files changed, 20 insertions(+), 47 deletions(-) diff --git a/modules/video_coding/encoder_database.cc b/modules/video_coding/encoder_database.cc index c37d47b685..80716df5c1 100644 --- a/modules/video_coding/encoder_database.cc +++ b/modules/video_coding/encoder_database.cc @@ -25,7 +25,6 @@ VCMEncoderDataBase::VCMEncoderDataBase( max_payload_size_(kDefaultPayloadSize), pending_encoder_reset_(true), send_codec_(), - encoder_payload_type_(0), external_encoder_(nullptr), internal_source_(false), encoded_frame_callback_(encoded_frame_callback) {} @@ -43,7 +42,6 @@ bool VCMEncoderDataBase::SetSendCodec(const VideoCodec* send_codec, max_payload_size = kDefaultPayloadSize; } RTC_DCHECK_GE(number_of_cores, 1); - RTC_DCHECK_GE(send_codec->plType, 1); // Make sure the start bit rate is sane... RTC_DCHECK_LE(send_codec->startBitrate, 1000000); RTC_DCHECK(send_codec->codecType != kVideoCodecUnknown); @@ -88,8 +86,6 @@ bool VCMEncoderDataBase::SetSendCodec(const VideoCodec* send_codec, // If encoder exists, will destroy it and create new one. DeleteEncoder(); - RTC_DCHECK_EQ(encoder_payload_type_, send_codec_.plType) - << "Encoder not registered for payload type " << send_codec_.plType; ptr_encoder_.reset(new VCMGenericEncoder( external_encoder_, encoded_frame_callback_, internal_source_)); encoded_frame_callback_->SetInternalSource(internal_source_); @@ -105,32 +101,19 @@ bool VCMEncoderDataBase::SetSendCodec(const VideoCodec* send_codec, return true; } -bool VCMEncoderDataBase::DeregisterExternalEncoder(uint8_t payload_type, - bool* was_send_codec) { - RTC_DCHECK(was_send_codec); - *was_send_codec = false; - if (encoder_payload_type_ != payload_type) { - return false; - } - if (send_codec_.plType == payload_type) { - // De-register as send codec if needed. - DeleteEncoder(); - memset(&send_codec_, 0, sizeof(VideoCodec)); - *was_send_codec = true; - } - encoder_payload_type_ = 0; +void VCMEncoderDataBase::DeregisterExternalEncoder() { + DeleteEncoder(); + memset(&send_codec_, 0, sizeof(VideoCodec)); external_encoder_ = nullptr; internal_source_ = false; - return true; } void VCMEncoderDataBase::RegisterExternalEncoder(VideoEncoder* external_encoder, - uint8_t payload_type, bool internal_source) { // Since only one encoder can be used at a given time, only one external // encoder can be registered/used. + RTC_CHECK(external_encoder_ == nullptr); external_encoder_ = external_encoder; - encoder_payload_type_ = payload_type; internal_source_ = internal_source; pending_encoder_reset_ = true; } @@ -140,9 +123,8 @@ bool VCMEncoderDataBase::RequiresEncoderReset( if (!ptr_encoder_) return true; - // Does not check startBitrate or maxFramerate + // Does not check startBitrate, maxFramerate or plType if (new_send_codec.codecType != send_codec_.codecType || - new_send_codec.plType != send_codec_.plType || new_send_codec.width != send_codec_.width || new_send_codec.height != send_codec_.height || new_send_codec.maxBitrate != send_codec_.maxBitrate || diff --git a/modules/video_coding/encoder_database.h b/modules/video_coding/encoder_database.h index 61e0720351..a9db4e15c8 100644 --- a/modules/video_coding/encoder_database.h +++ b/modules/video_coding/encoder_database.h @@ -34,13 +34,10 @@ class VCMEncoderDataBase { // video source and doesn't need the user to provide it with frames via // the Encode() method. void RegisterExternalEncoder(VideoEncoder* external_encoder, - uint8_t payload_type, bool internal_source); - // Deregisters an external encoder. Returns true if the encoder was - // found and deregistered, false otherwise. |was_send_codec| is set to true - // if the external encoder was the send codec before being deregistered. - bool DeregisterExternalEncoder(uint8_t payload_type, bool* was_send_codec); + // Deregisters any external encoder. + void DeregisterExternalEncoder(); VCMGenericEncoder* GetEncoder(); @@ -57,7 +54,6 @@ class VCMEncoderDataBase { size_t max_payload_size_; bool pending_encoder_reset_; VideoCodec send_codec_; - uint8_t encoder_payload_type_; VideoEncoder* external_encoder_; bool internal_source_; VCMEncodedFrameCallback* const encoded_frame_callback_; diff --git a/modules/video_coding/video_coding_impl.cc b/modules/video_coding/video_coding_impl.cc index b13519c14b..c45f41e3ff 100644 --- a/modules/video_coding/video_coding_impl.cc +++ b/modules/video_coding/video_coding_impl.cc @@ -126,9 +126,9 @@ class VideoCodingModuleImpl : public VideoCodingModule { } int32_t RegisterExternalEncoder(VideoEncoder* externalEncoder, - uint8_t payloadType, + uint8_t /* payloadType */, bool internalSource) override { - sender_.RegisterExternalEncoder(externalEncoder, payloadType, + sender_.RegisterExternalEncoder(externalEncoder, internalSource); return 0; } diff --git a/modules/video_coding/video_coding_impl.h b/modules/video_coding/video_coding_impl.h index 2a6bf44b63..4f96ad92f0 100644 --- a/modules/video_coding/video_coding_impl.h +++ b/modules/video_coding/video_coding_impl.h @@ -75,7 +75,6 @@ class VideoSender { uint32_t maxPayloadSize); void RegisterExternalEncoder(VideoEncoder* externalEncoder, - uint8_t payloadType, bool internalSource); // Update the channel parameters based on new rates and rtt. This will also diff --git a/modules/video_coding/video_sender.cc b/modules/video_coding/video_sender.cc index 381f62867d..680b76f8c0 100644 --- a/modules/video_coding/video_sender.cc +++ b/modules/video_coding/video_sender.cc @@ -122,17 +122,14 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec, // Register an external decoder object. // This can not be used together with external decoder callbacks. void VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder, - uint8_t payloadType, bool internalSource /*= false*/) { RTC_DCHECK(sequenced_checker_.CalledSequentially()); rtc::CritScope lock(&encoder_crit_); if (externalEncoder == nullptr) { - bool wasSendCodec = false; - RTC_CHECK( - _codecDataBase.DeregisterExternalEncoder(payloadType, &wasSendCodec)); - if (wasSendCodec) { + _codecDataBase.DeregisterExternalEncoder(); + { // Make sure the VCM doesn't use the de-registered codec rtc::CritScope params_lock(¶ms_crit_); _encoder = nullptr; @@ -140,7 +137,7 @@ void VideoSender::RegisterExternalEncoder(VideoEncoder* externalEncoder, } return; } - _codecDataBase.RegisterExternalEncoder(externalEncoder, payloadType, + _codecDataBase.RegisterExternalEncoder(externalEncoder, internalSource); } diff --git a/modules/video_coding/video_sender_unittest.cc b/modules/video_coding/video_sender_unittest.cc index 0719b1677a..bce3f626cf 100644 --- a/modules/video_coding/video_sender_unittest.cc +++ b/modules/video_coding/video_sender_unittest.cc @@ -206,7 +206,7 @@ class TestVideoSenderWithMockEncoder : public TestVideoSender { protected: void SetUp() override { TestVideoSender::SetUp(); - sender_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, false); + sender_->RegisterExternalEncoder(&encoder_, false); webrtc::test::CodecSettings(kVideoCodecVP8, &settings_); settings_.numberOfSimulcastStreams = kNumberOfStreams; ConfigureStream(kDefaultWidth / 4, kDefaultHeight / 4, 100, @@ -325,9 +325,9 @@ TEST_F(TestVideoSenderWithMockEncoder, TestSetRate) { TEST_F(TestVideoSenderWithMockEncoder, TestIntraRequestsInternalCapture) { // De-register current external encoder. - sender_->RegisterExternalEncoder(nullptr, kUnusedPayloadType, false); + sender_->RegisterExternalEncoder(nullptr, false); // Register encoder with internal capture. - sender_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, true); + sender_->RegisterExternalEncoder(&encoder_, true); EXPECT_EQ(0, sender_->RegisterSendCodec(&settings_, 1, 1200)); // Initial request should be all keyframes. ExpectInitialKeyFrames(); @@ -344,9 +344,9 @@ TEST_F(TestVideoSenderWithMockEncoder, TestIntraRequestsInternalCapture) { TEST_F(TestVideoSenderWithMockEncoder, TestEncoderParametersForInternalSource) { // De-register current external encoder. - sender_->RegisterExternalEncoder(nullptr, kUnusedPayloadType, false); + sender_->RegisterExternalEncoder(nullptr, false); // Register encoder with internal capture. - sender_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, true); + sender_->RegisterExternalEncoder(&encoder_, true); EXPECT_EQ(0, sender_->RegisterSendCodec(&settings_, 1, 1200)); // Update encoder bitrate parameters. We expect that to immediately call // SetRates on the encoder without waiting for AddFrame processing. @@ -419,7 +419,7 @@ class TestVideoSenderWithVp8 : public TestVideoSender { codec_.VP8()->tl_factory = tl_factory; encoder_ = VP8Encoder::Create(); - sender_->RegisterExternalEncoder(encoder_.get(), codec_.plType, false); + sender_->RegisterExternalEncoder(encoder_.get(), false); EXPECT_EQ(0, sender_->RegisterSendCodec(&codec_, 1, 1200)); } diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index dd7738f90e..8095222088 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -422,7 +422,7 @@ VideoStreamEncoder::VideoStreamEncoder( RTC_DCHECK_RUN_ON(&encoder_queue_); overuse_detector_->StartCheckForOveruse(this); video_sender_.RegisterExternalEncoder( - settings_.encoder, settings_.payload_type, settings_.internal_source); + settings_.encoder, settings_.internal_source); }); } @@ -440,8 +440,7 @@ void VideoStreamEncoder::Stop() { overuse_detector_->StopCheckForOveruse(); rate_allocator_.reset(); bitrate_observer_ = nullptr; - video_sender_.RegisterExternalEncoder(nullptr, settings_.payload_type, - false); + video_sender_.RegisterExternalEncoder(nullptr, false); quality_scaler_ = nullptr; shutdown_event_.Set(); });