From 0a7d4eed98ccec0c2b3e7522e7b2dde1919a4ae3 Mon Sep 17 00:00:00 2001 From: "mflodman@webrtc.org" Date: Tue, 17 Feb 2015 12:57:14 +0000 Subject: [PATCH] Remove usage of BitrateController in VoiceEngine. Bitrate controller is used in VoiceEngine to smoothen the fraction loss from RTCP report blocks. This CL removes the usage of the BitrateController and calculates its own fraction loss average insted. This introduces some duplicated code between BitrateController and Channel, but removes processing overhead and the incorrect bandwidth estimation numbers reported by the bitrate controller. BUG=4310 TEST=voe_cmd_test with network simulator R=minyue@webrtc.org Review URL: https://webrtc-codereview.appspot.com/39999004 Cr-Commit-Position: refs/heads/master@{#8386} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8386 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/voice_engine/channel.cc | 88 +++++++++++++++--------- webrtc/voice_engine/channel.h | 15 ++-- webrtc/voice_engine/network_predictor.cc | 11 +-- 3 files changed, 67 insertions(+), 47 deletions(-) diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 745ac9e90e..bb8fcb10ed 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -92,22 +92,59 @@ class StatisticsProxy : public RtcpStatisticsCallback { ChannelStatistics stats_; }; -class VoEBitrateObserver : public BitrateObserver { +class VoERtcpObserver : public RtcpBandwidthObserver { public: - explicit VoEBitrateObserver(Channel* owner) - : owner_(owner) {} - virtual ~VoEBitrateObserver() {} + explicit VoERtcpObserver(Channel* owner) : owner_(owner) {} + virtual ~VoERtcpObserver() {} - // Implements BitrateObserver. - virtual void OnNetworkChanged(const uint32_t bitrate_bps, - const uint8_t fraction_lost, - const int64_t rtt) OVERRIDE { - // |fraction_lost| has a scale of 0 - 255. - owner_->OnNetworkChanged(bitrate_bps, fraction_lost, rtt); + void OnReceivedEstimatedBitrate(uint32_t bitrate) override { + // Not used for Voice Engine. + } + + virtual void OnReceivedRtcpReceiverReport( + const ReportBlockList& report_blocks, + int64_t rtt, + int64_t now_ms) override { + // TODO(mflodman): Do we need to aggregate reports here or can we jut send + // what we get? I.e. do we ever get multiple reports bundled into one RTCP + // report for VoiceEngine? + if (report_blocks.empty()) + return; + + int fraction_lost_aggregate = 0; + int total_number_of_packets = 0; + + // If receiving multiple report blocks, calculate the weighted average based + // on the number of packets a report refers to. + for (ReportBlockList::const_iterator block_it = report_blocks.begin(); + block_it != report_blocks.end(); ++block_it) { + // Find the previous extended high sequence number for this remote SSRC, + // to calculate the number of RTP packets this report refers to. Ignore if + // we haven't seen this SSRC before. + std::map::iterator seq_num_it = + extended_max_sequence_number_.find(block_it->sourceSSRC); + int number_of_packets = 0; + if (seq_num_it != extended_max_sequence_number_.end()) { + number_of_packets = block_it->extendedHighSeqNum - seq_num_it->second; + } + fraction_lost_aggregate += number_of_packets * block_it->fractionLost; + total_number_of_packets += number_of_packets; + + extended_max_sequence_number_[block_it->sourceSSRC] = + block_it->extendedHighSeqNum; + } + int weighted_fraction_lost = 0; + if (total_number_of_packets > 0) { + weighted_fraction_lost = (fraction_lost_aggregate + + total_number_of_packets / 2) / total_number_of_packets; + } + owner_->OnIncomingFractionLoss(weighted_fraction_lost); } private: Channel* owner_; + // Maps remote side ssrc to extended highest sequence number received. + std::map extended_max_sequence_number_; }; int32_t @@ -788,12 +825,7 @@ Channel::Channel(int32_t channelId, _rxAgcIsEnabled(false), _rxNsIsEnabled(false), restored_packet_in_use_(false), - bitrate_controller_( - BitrateController::CreateBitrateController(Clock::GetRealTimeClock(), - true)), - rtcp_bandwidth_observer_( - bitrate_controller_->CreateRtcpBandwidthObserver()), - send_bitrate_observer_(new VoEBitrateObserver(this)), + rtcp_observer_(new VoERtcpObserver(this)), network_predictor_(new NetworkPredictor(Clock::GetRealTimeClock())) { WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(_instanceId,_channelId), @@ -808,7 +840,7 @@ Channel::Channel(int32_t channelId, configuration.outgoing_transport = this; configuration.audio_messages = this; configuration.receive_statistics = rtp_receive_statistics_.get(); - configuration.bandwidth_callback = rtcp_bandwidth_observer_.get(); + configuration.bandwidth_callback = rtcp_observer_.get(); _rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration)); @@ -1321,28 +1353,16 @@ Channel::SetSendCodec(const CodecInst& codec) return -1; } - bitrate_controller_->SetBitrateObserver(send_bitrate_observer_.get(), - codec.rate, 0, 0); - return 0; } -void -Channel::OnNetworkChanged(const uint32_t bitrate_bps, - const uint8_t fraction_lost, // 0 - 255. - const int64_t rtt) { - WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId,_channelId), - "Channel::OnNetworkChanged(bitrate_bps=%d, fration_lost=%d, rtt=%" PRId64 - ")", bitrate_bps, fraction_lost, rtt); - // |fraction_lost| from BitrateObserver is short time observation of packet - // loss rate from past. We use network predictor to make a more reasonable - // loss rate estimation. +void Channel::OnIncomingFractionLoss(int fraction_lost) { network_predictor_->UpdatePacketLossRate(fraction_lost); - uint8_t loss_rate = network_predictor_->GetLossRate(); + uint8_t average_fraction_loss = network_predictor_->GetLossRate(); + // Normalizes rate to 0 - 100. - if (audio_coding_->SetPacketLossRate(100 * loss_rate / 255) != 0) { - _engineStatisticsPtr->SetLastError(VE_AUDIO_CODING_MODULE_ERROR, - kTraceError, "OnNetworkChanged() failed to set packet loss rate"); + if (audio_coding_->SetPacketLossRate( + 100 * average_fraction_loss / 255) != 0) { assert(false); // This should not happen. } } diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index b856af37f8..e7bedc5d11 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -68,10 +68,11 @@ struct SenderInfo; namespace voe { +class OutputMixer; class Statistics; class StatisticsProxy; class TransmitMixer; -class OutputMixer; +class VoERtcpObserver; // Helper class to simplify locking scheme for members that are accessed from // multiple threads. @@ -163,6 +164,8 @@ class Channel: public MixerParticipant // supplies output mixer with audio frames { public: + friend class VoERtcpObserver; + enum {KNumSocketThreads = 1}; enum {KNumberOfSocketBuffers = 8}; virtual ~Channel(); @@ -449,10 +452,8 @@ public: uint32_t PrepareEncodeAndSend(int mixingFrequency); uint32_t EncodeAndSend(); - // From BitrateObserver (called by the RTP/RTCP module). - void OnNetworkChanged(const uint32_t bitrate_bps, - const uint8_t fraction_lost, // 0 - 255. - const int64_t rtt); +protected: + void OnIncomingFractionLoss(int fraction_lost); private: bool ReceivePacket(const uint8_t* packet, size_t packet_length, @@ -581,9 +582,7 @@ private: bool _rxNsIsEnabled; bool restored_packet_in_use_; // RtcpBandwidthObserver - scoped_ptr bitrate_controller_; - scoped_ptr rtcp_bandwidth_observer_; - scoped_ptr send_bitrate_observer_; + scoped_ptr rtcp_observer_; scoped_ptr network_predictor_; }; diff --git a/webrtc/voice_engine/network_predictor.cc b/webrtc/voice_engine/network_predictor.cc index 4093877343..f8fa2e315c 100644 --- a/webrtc/voice_engine/network_predictor.cc +++ b/webrtc/voice_engine/network_predictor.cc @@ -19,6 +19,12 @@ NetworkPredictor::NetworkPredictor(Clock* clock) loss_rate_filter_(new rtc::ExpFilter(0.9999f)) { } +uint8_t NetworkPredictor::GetLossRate() { + float value = loss_rate_filter_->filtered(); + return (value == rtc::ExpFilter::kValueUndefined) ? 0 : + static_cast(value + 0.5); +} + void NetworkPredictor::UpdatePacketLossRate(uint8_t loss_rate) { int64_t now_ms = clock_->TimeInMilliseconds(); // Update the recursive average filter. @@ -28,10 +34,5 @@ void NetworkPredictor::UpdatePacketLossRate(uint8_t loss_rate) { last_loss_rate_update_time_ms_ = now_ms; } -uint8_t NetworkPredictor::GetLossRate() { - float value = loss_rate_filter_->filtered(); - return (value == rtc::ExpFilter::kValueUndefined) ? 0 : - static_cast(value + 0.5); -} } // namespace voe } // namespace webrtc