From cd5c25cb801fa0a1a2f1c7e453354ccb0e65beff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Thu, 21 Apr 2016 16:48:08 +0200 Subject: [PATCH] Use vcm::VideoSender in ViEEncoder. ViEEncoder doesn't need a full VideoCodingModule since it only uses the sender side either way. BUG=webrtc:3608,webrtc:5687 R=perkj@webrtc.org Review URL: https://codereview.webrtc.org/1904983002 . Cr-Commit-Position: refs/heads/master@{#12456} --- .../valgrind-webrtc/drmemory/suppressions.txt | 41 ------------- .../valgrind-webrtc/memcheck/suppressions.txt | 17 ------ .../modules/video_coding/video_coding_impl.h | 6 +- webrtc/video/video_send_stream.cc | 6 +- webrtc/video/video_send_stream.h | 6 +- webrtc/video/vie_encoder.cc | 59 +++++++------------ webrtc/video/vie_encoder.h | 9 ++- 7 files changed, 35 insertions(+), 109 deletions(-) diff --git a/tools/valgrind-webrtc/drmemory/suppressions.txt b/tools/valgrind-webrtc/drmemory/suppressions.txt index 0658d34367..bd5d467345 100644 --- a/tools/valgrind-webrtc/drmemory/suppressions.txt +++ b/tools/valgrind-webrtc/drmemory/suppressions.txt @@ -84,47 +84,6 @@ system call NtUserGetThreadDesktop parameter value #1 *!webrtc::ScreenCapturerTest::SetUp *!testing::internal::HandleSehExceptionsInMethodIfSupported -UNINITIALIZED READ -name=https://code.google.com/p/webrtc/issues/detail?id=3158 (1) -*!_output_l -*!_vsnprintf_l -*!_vsnprintf -*!webrtc::Trace::Add -*!webrtc::ViECodecImpl::GetCodecTargetBitrate -*!cricket::WebRtcVideoMediaChannel::MaybeChangeStartBitrate -*!cricket::WebRtcVideoMediaChannel::SetSendCodec -*!cricket::WebRtcVideoMediaChannel::SetSendCodec -*!cricket::WebRtcVideoMediaChannel::SetSendCodecs -*!VideoMediaChannelTest<>::SetOneCodec -*!VideoMediaChannelTest<>::Send -*!VideoMediaChannelTest<>::TwoStreamsSendAndFailUnsignalledRecv -*!WebRtcVideoMediaChannelTest_TwoStreamsSendAndFailUnsignalledRecv_Test::TestBody -*!testing::internal::HandleSehExceptionsInMethodIfSupported<> - -UNINITIALIZED READ -name=https://code.google.com/p/webrtc/issues/detail?id=3158 (2) -*!_output_l -*!_vsnprintf_l -*!_vsnprintf -*!webrtc::Trace::Add -*!webrtc::ViECodecImpl::GetCodecTargetBitrate -*!cricket::WebRtcVideoMediaChannel::MaybeChangeStartBitrate -*!cricket::WebRtcVideoMediaChannel::MaybeResetVieSendCodec -*!cricket::WebRtcVideoMediaChannel::SendFrame -*!cricket::WebRtcVideoMediaChannel::SendFrame -*!sigslot::_connection2<>::emit -*!sigslot::signal2<>::operator() -*!cricket::VideoCapturer::OnFrameCaptured -*!sigslot::_connection2<>::emit -*!sigslot::signal2<>::operator() -*!cricket::FakeVideoCapturer::CaptureCustomFrame -*!cricket::FakeVideoCapturer::CaptureFrame -*!VideoMediaChannelTest<>::SendFrame -*!VideoMediaChannelTest<>::Send -*!VideoMediaChannelTest<>::TwoStreamsSendAndFailUnsignalledRecv -*!WebRtcVideoMediaChannelTest_TwoStreamsSendAndFailUnsignalledRecv_Test::TestBody -*!testing::internal::HandleSehExceptionsInMethodIfSupported<> - # rtc_unittest, fails on Win DrMemory Full UNINITIALIZED READ name=https://code.google.com/p/webrtc/issues/detail?id=3158 (4) diff --git a/tools/valgrind-webrtc/memcheck/suppressions.txt b/tools/valgrind-webrtc/memcheck/suppressions.txt index fce9259b92..4ab84b92b0 100644 --- a/tools/valgrind-webrtc/memcheck/suppressions.txt +++ b/tools/valgrind-webrtc/memcheck/suppressions.txt @@ -385,23 +385,6 @@ fun:_ZN14DtlsTestClient12VerifyPacketEPKcmPj ... } -{ - bug_3063 - Memcheck:Uninitialized - ... - fun:vfprintf - fun:__vsnprintf_chk - fun:_ZN6webrtc5Trace3AddENS_10TraceLevelENS_11TraceModuleEiPKcz - fun:_ZNK6webrtc12ViECodecImpl21GetCodecTargetBitrateEiPj - fun:_ZN7cricket23WebRtcVideoMediaChannel23MaybeChangeStartBitrateEiPN6webrtc10VideoCodecE - fun:_ZN7cricket23WebRtcVideoMediaChannel12SetSendCodecEPNS_26WebRtcVideoChannelSendInfoERKN6webrtc10VideoCodecEiii - fun:_ZN7cricket23WebRtcVideoMediaChannel12SetSendCodecERKN6webrtc10VideoCodecEiii - fun:_ZN7cricket23WebRtcVideoMediaChannel13SetSendCodecsERKSt6vectorINS_10VideoCodecESaIS2_EE - fun:_ZN21VideoMediaChannelTestIN7cricket17WebRtcVideoEngineENS0_23WebRtcVideoMediaChannelEE11SetOneCodecERKNS0_10VideoCodecE - fun:_ZN21VideoMediaChannelTestIN7cricket17WebRtcVideoEngineENS0_23WebRtcVideoMediaChannelEE4SendERKNS0_10VideoCodecE - fun:_ZN21VideoMediaChannelTestIN7cricket17WebRtcVideoEngineENS0_23WebRtcVideoMediaChannelEE36TwoStreamsSendAndFailUnsignalledRecvERKNS0_10VideoCodecE - fun:_ZN69WebRtcVideoMediaChannelTest_TwoStreamsSendAndFailUnsignalledRecv_Test8TestBodyEv -} { bug_3478 Memcheck:Leak diff --git a/webrtc/modules/video_coding/video_coding_impl.h b/webrtc/modules/video_coding/video_coding_impl.h index e9d7abc969..e3184a83ec 100644 --- a/webrtc/modules/video_coding/video_coding_impl.h +++ b/webrtc/modules/video_coding/video_coding_impl.h @@ -51,7 +51,7 @@ class VCMProcessTimer { int64_t _latestMs; }; -class VideoSender { +class VideoSender : public Module { public: typedef VideoCodingModule::SenderNackMode SenderNackMode; @@ -96,8 +96,8 @@ class VideoSender { void SuspendBelowMinBitrate(); bool VideoSuspended() const; - int64_t TimeUntilNextProcess(); - void Process(); + int64_t TimeUntilNextProcess() override; + void Process() override; private: void SetEncoderParameters(EncoderParameters params) diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index b398c7cd0b..40871af121 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -382,7 +382,7 @@ VideoSendStream::VideoSendStream( congestion_controller_->pacer(), &payload_router_, config.post_encode_callback ? &encoded_frame_proxy_ : nullptr), - vcm_(vie_encoder_.vcm()), + video_sender_(vie_encoder_.video_sender()), bandwidth_observer_(congestion_controller_->GetBitrateController() ->CreateRtcpBandwidthObserver()), rtp_rtcp_modules_(CreateRtpRtcpModules( @@ -417,7 +417,7 @@ VideoSendStream::VideoSendStream( congestion_controller_->packet_router()->AddRtpModule(rtp_rtcp); } - vcm_->RegisterProtectionCallback(this); + video_sender_->RegisterProtectionCallback(this); for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) { const std::string& extension = config_.rtp.extensions[i].name; @@ -547,7 +547,7 @@ void VideoSendStream::EncoderProcess() { vie_encoder_.SetEncoder(encoder_settings->video_codec, encoder_settings->min_transmit_bitrate_bps); if (config_.suspend_below_min_bitrate) { - vcm_->SuspendBelowMinBitrate(); + video_sender_->SuspendBelowMinBitrate(); bitrate_allocator_->EnforceMinBitrate(false); } // We might've gotten new settings while configuring the encoder settings, diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h index e52106cefd..ad438ae0cb 100644 --- a/webrtc/video/video_send_stream.h +++ b/webrtc/video/video_send_stream.h @@ -39,6 +39,10 @@ class ViEChannel; class ViEEncoder; class VieRemb; +namespace vcm { +class VideoSender; +} // namespace vcm + namespace internal { class VideoSendStream : public webrtc::VideoSendStream, @@ -122,7 +126,7 @@ class VideoSendStream : public webrtc::VideoSendStream, OveruseFrameDetector overuse_detector_; EncoderStateFeedback encoder_feedback_; ViEEncoder vie_encoder_; - VideoCodingModule* const vcm_; + vcm::VideoSender* const video_sender_; const std::unique_ptr bandwidth_observer_; // RtpRtcp modules, declared here as they use other members on construction. diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 0c781b8663..8240f9bfa6 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -91,9 +91,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores, ssrcs_(ssrcs), vp_(VideoProcessing::Create()), qm_callback_(new QMVideoSettingsCallback(vp_.get())), - vcm_(VideoCodingModule::Create(Clock::GetRealTimeClock(), - this, - qm_callback_.get())), + video_sender_(Clock::GetRealTimeClock(), this, this, qm_callback_.get()), stats_proxy_(stats_proxy), pre_encode_callback_(pre_encode_callback), overuse_detector_(overuse_detector), @@ -114,7 +112,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores, has_received_rpsi_(false), picture_id_rpsi_(0), video_suspended_(false) { - module_process_thread_->RegisterModule(vcm_.get()); + module_process_thread_->RegisterModule(&video_sender_); } bool ViEEncoder::Init() { @@ -123,25 +121,23 @@ bool ViEEncoder::Init() { // Enable/disable content analysis: off by default for now. vp_->EnableContentAnalysis(false); - vcm_->RegisterPostEncodeImageCallback(this); - // TODO(perkj): Remove |RegisterTransportCallback| as soon as we don't use // VCMPacketizationCallback::OnEncoderImplementationName. - if (vcm_->RegisterTransportCallback(this) != 0) { + if (video_sender_.RegisterTransportCallback(this) != 0) { return false; } - if (vcm_->RegisterSendStatisticsCallback(this) != 0) { + if (video_sender_.RegisterSendStatisticsCallback(this) != 0) { return false; } return true; } -VideoCodingModule* ViEEncoder::vcm() const { - return vcm_.get(); +vcm::VideoSender* ViEEncoder::video_sender() { + return &video_sender_; } ViEEncoder::~ViEEncoder() { - module_process_thread_->DeRegisterModule(vcm_.get()); + module_process_thread_->DeRegisterModule(&video_sender_); } void ViEEncoder::SetNetworkTransmissionState(bool is_transmitting) { @@ -164,17 +160,12 @@ void ViEEncoder::Restart() { int32_t ViEEncoder::RegisterExternalEncoder(webrtc::VideoEncoder* encoder, uint8_t pl_type, bool internal_source) { - if (vcm_->RegisterExternalEncoder(encoder, pl_type, internal_source) != - VCM_OK) { - return -1; - } + video_sender_.RegisterExternalEncoder(encoder, pl_type, internal_source); return 0; } int32_t ViEEncoder::DeRegisterExternalEncoder(uint8_t pl_type) { - if (vcm_->RegisterExternalEncoder(nullptr, pl_type) != VCM_OK) { - return -1; - } + video_sender_.RegisterExternalEncoder(nullptr, pl_type, false); return 0; } void ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec, @@ -195,7 +186,7 @@ void ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec, } size_t max_data_payload_length = send_payload_router_->MaxPayloadLength(); - bool success = vcm_->RegisterSendCodec( + bool success = video_sender_.RegisterSendCodec( &video_codec, number_of_cores_, static_cast(max_data_payload_length)) == VCM_OK; if (!success) { @@ -248,7 +239,7 @@ int ViEEncoder::GetPaddingNeededBps() const { send_codec = encoder_config_; } - bool video_is_suspended = vcm_->VideoSuspended(); + bool video_is_suspended = video_sender_.VideoSuspended(); // Find the max amount of padding we can allow ourselves to send at this // point, based on which streams are currently active and what our current @@ -370,26 +361,15 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame) { has_received_rpsi_ = false; } - vcm_->AddVideoFrame(*frame_to_send, vp_->GetContentMetrics(), - &codec_specific_info); + video_sender_.AddVideoFrame(*frame_to_send, vp_->GetContentMetrics(), + &codec_specific_info); return; } - vcm_->AddVideoFrame(*frame_to_send); + video_sender_.AddVideoFrame(*frame_to_send, nullptr, nullptr); } void ViEEncoder::SendKeyFrame() { - vcm_->IntraFrameRequest(0); -} - -uint32_t ViEEncoder::LastObservedBitrateBps() const { - rtc::CritScope lock(&data_cs_); - return last_observed_bitrate_bps_; -} - -int ViEEncoder::CodecTargetBitrate(uint32_t* bitrate) const { - if (vcm_->Bitrate(bitrate) != 0) - return -1; - return 0; + video_sender_.IntraFrameRequest(0); } void ViEEncoder::SetProtectionMethod(bool nack, bool fec) { @@ -401,7 +381,7 @@ void ViEEncoder::SetProtectionMethod(bool nack, bool fec) { } else { protection_mode = nack ? kProtectionNack : kProtectionNone; } - vcm_->SetVideoProtection(protection_mode, true); + video_sender_.SetVideoProtection(protection_mode); } void ViEEncoder::OnSetRates(uint32_t bitrate_bps, int framerate) { @@ -500,7 +480,7 @@ void ViEEncoder::OnReceivedIntraFrameRequest(uint32_t ssrc) { } time_last_intra_request_ms_[i] = now_ms; } - vcm_->IntraFrameRequest(static_cast(i)); + video_sender_.IntraFrameRequest(static_cast(i)); return; } RTC_NOTREACHED() << "Should not receive keyframe requests on unknown SSRCs."; @@ -513,8 +493,9 @@ void ViEEncoder::OnBitrateUpdated(uint32_t bitrate_bps, << " packet loss " << static_cast(fraction_lost) << " rtt " << round_trip_time_ms; RTC_DCHECK(send_payload_router_); - vcm_->SetChannelParameters(bitrate_bps, fraction_lost, round_trip_time_ms); - bool video_is_suspended = vcm_->VideoSuspended(); + video_sender_.SetChannelParameters(bitrate_bps, fraction_lost, + round_trip_time_ms); + bool video_is_suspended = video_sender_.VideoSuspended(); bool video_suspension_changed; VideoCodec send_codec; { diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index 27f2d11ca3..02fec6292a 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -23,6 +23,7 @@ #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/video_coding/include/video_coding_defines.h" #include "webrtc/modules/video_coding/utility/ivf_file_writer.h" +#include "webrtc/modules/video_coding/video_coding_impl.h" #include "webrtc/modules/video_processing/include/video_processing.h" #include "webrtc/typedefs.h" @@ -62,7 +63,7 @@ class ViEEncoder : public VideoEncoderRateObserver, bool Init(); - VideoCodingModule* vcm() const; + vcm::VideoSender* video_sender(); void SetNetworkTransmissionState(bool is_transmitting); @@ -84,11 +85,9 @@ class ViEEncoder : public VideoEncoderRateObserver, void SendKeyFrame(); uint32_t LastObservedBitrateBps() const; - int CodecTargetBitrate(uint32_t* bitrate) const; // Loss protection. Must be called before SetEncoder() to have max packet size // updated according to protection. - // TODO(pbos): Set protection method on construction or extract vcm_ outside - // this class and set it on construction there. + // TODO(pbos): Set protection method on construction. void SetProtectionMethod(bool nack, bool fec); // Implements VideoEncoderRateObserver. @@ -130,7 +129,7 @@ class ViEEncoder : public VideoEncoderRateObserver, const std::unique_ptr vp_; const std::unique_ptr qm_callback_; - const std::unique_ptr vcm_; + vcm::VideoSender video_sender_; rtc::CriticalSection data_cs_;