From fbea4e555d1fbd384832e102d79581cc32821fae Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Thu, 27 Oct 2011 16:08:29 +0000 Subject: [PATCH] Solves two bandwidth estimation issues and measures the sent video bitrate. Issues solved: 1. Possible overflow when reducing the bandwidth estimate at the send-side 2. A burst of loss reports could make us reduce the rate way too far since we reduced the rate relative the current estimate and not the actual rate sent. BUG= TEST= Review URL: http://webrtc-codereview.appspot.com/244011 git-svn-id: http://webrtc.googlecode.com/svn/trunk@822 4adac7df-926f-26a2-2b94-8c16560cd09d --- src/modules/rtp_rtcp/interface/rtp_rtcp.h | 1 + .../rtp_rtcp/source/bandwidth_management.cc | 9 ++-- .../rtp_rtcp/source/bandwidth_management.h | 5 +- src/modules/rtp_rtcp/source/rtcp_sender.cc | 2 + src/modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 25 ++++++++-- src/modules/rtp_rtcp/source/rtp_rtcp_impl.h | 1 + src/modules/rtp_rtcp/source/rtp_sender.cc | 13 ++++- src/modules/rtp_rtcp/source/rtp_sender.h | 1 + .../rtp_rtcp/source/rtp_sender_video.cc | 49 +++++++++++++++---- .../rtp_rtcp/source/rtp_sender_video.h | 5 ++ .../main/interface/vie_rtp_rtcp.h | 1 + src/video_engine/main/source/vie_channel.cc | 5 +- src/video_engine/main/source/vie_channel.h | 1 + .../main/source/vie_rtp_rtcp_impl.cc | 2 + .../main/source/vie_rtp_rtcp_impl.h | 1 + .../AutoTest/source/vie_autotest_rtp_rtcp.cc | 4 ++ 16 files changed, 105 insertions(+), 20 deletions(-) diff --git a/src/modules/rtp_rtcp/interface/rtp_rtcp.h b/src/modules/rtp_rtcp/interface/rtp_rtcp.h index 733943807..26e3ac21e 100644 --- a/src/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/src/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -496,6 +496,7 @@ public: * get sent bitrate in Kbit/s */ virtual void BitrateSent(WebRtc_UWord32* totalRate, + WebRtc_UWord32* videoRate, WebRtc_UWord32* fecRate, WebRtc_UWord32* nackRate) const = 0; diff --git a/src/modules/rtp_rtcp/source/bandwidth_management.cc b/src/modules/rtp_rtcp/source/bandwidth_management.cc index 919730bc6..74038372a 100644 --- a/src/modules/rtp_rtcp/source/bandwidth_management.cc +++ b/src/modules/rtp_rtcp/source/bandwidth_management.cc @@ -104,6 +104,7 @@ BandwidthManagement::UpdateBandwidthEstimate(const WebRtc_UWord16 bandWidthKbit, WebRtc_Word32 BandwidthManagement::UpdatePacketLoss( const WebRtc_UWord32 lastReceivedExtendedHighSeqNum, + WebRtc_UWord32 sentBitrate, const WebRtc_UWord16 rtt, WebRtc_UWord8* loss, WebRtc_UWord32* newBitrate) @@ -168,7 +169,7 @@ WebRtc_Word32 BandwidthManagement::UpdatePacketLoss( // Remember the sequence number until next time _lastPacketLossExtendedHighSeqNum = lastReceivedExtendedHighSeqNum; - WebRtc_UWord32 bitRate = ShapeSimple(*loss, rtt); + WebRtc_UWord32 bitRate = ShapeSimple(*loss, rtt, sentBitrate); if (bitRate == 0) { // no change @@ -211,7 +212,8 @@ WebRtc_Word32 BandwidthManagement::CalcTFRCbps(WebRtc_Word16 avgPackSizeBytes, */ // protected WebRtc_UWord32 BandwidthManagement::ShapeSimple(WebRtc_Word32 packetLoss, - WebRtc_Word32 rtt) + WebRtc_Word32 rtt, + WebRtc_UWord32 sentBitrate) { WebRtc_UWord32 newBitRate = 0; bool reducing = false; @@ -226,7 +228,8 @@ WebRtc_UWord32 BandwidthManagement::ShapeSimple(WebRtc_Word32 packetLoss, // 26/256 ~= 10% // reduce rate: newRate = rate * (1 - 0.5*lossRate) // packetLoss = 256*lossRate - newBitRate = (_bitRate * (512 - packetLoss)) / 512; + newBitRate = static_cast( + (sentBitrate * static_cast(512 - packetLoss)) / 512.0); reducing = true; } else diff --git a/src/modules/rtp_rtcp/source/bandwidth_management.h b/src/modules/rtp_rtcp/source/bandwidth_management.h index c4ab939bd..77e39fad5 100644 --- a/src/modules/rtp_rtcp/source/bandwidth_management.h +++ b/src/modules/rtp_rtcp/source/bandwidth_management.h @@ -35,6 +35,7 @@ public: // Call when we receive a RTCP message with a ReceiveBlock WebRtc_Word32 UpdatePacketLoss( const WebRtc_UWord32 lastReceivedExtendedHighSeqNum, + WebRtc_UWord32 sentBitrate, const WebRtc_UWord16 rtt, WebRtc_UWord8* loss, WebRtc_UWord32* newBitrate); @@ -48,7 +49,9 @@ public: WebRtc_Word32 MaxConfiguredBitrate(WebRtc_UWord16* maxBitrateKbit); protected: - WebRtc_UWord32 ShapeSimple(WebRtc_Word32 packetLoss, WebRtc_Word32 rtt); + WebRtc_UWord32 ShapeSimple(WebRtc_Word32 packetLoss, + WebRtc_Word32 rtt, + WebRtc_UWord32 sentBitrate); WebRtc_Word32 CalcTFRCbps(WebRtc_Word16 avgPackSizeBytes, WebRtc_Word32 rttMs, diff --git a/src/modules/rtp_rtcp/source/rtcp_sender.cc b/src/modules/rtp_rtcp/source/rtcp_sender.cc index cb80b9840..7e6f38536 100644 --- a/src/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/src/modules/rtp_rtcp/source/rtcp_sender.cc @@ -1694,9 +1694,11 @@ RTCPSender::SendRTCP(const WebRtc_UWord32 packetTypeFlags, { // calc bw for video 360/sendBW in kbit/s WebRtc_UWord32 sendBitrateKbit = 0; + WebRtc_UWord32 videoRate = 0; WebRtc_UWord32 fecRate = 0; WebRtc_UWord32 nackRate = 0; _rtpRtcp.BitrateSent(&sendBitrateKbit, + &videoRate, &fecRate, &nackRate); sendBitrateKbit /= 1000; diff --git a/src/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/src/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index b17b3d170..4a1976782 100644 --- a/src/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/src/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -2340,8 +2340,9 @@ WebRtc_UWord32 ModuleRtpRtcpImpl::BitrateReceivedNow() const { } void ModuleRtpRtcpImpl::BitrateSent(WebRtc_UWord32* totalRate, - WebRtc_UWord32* fecRate, - WebRtc_UWord32* nackRate) const { + WebRtc_UWord32* videoRate, + WebRtc_UWord32* fecRate, + WebRtc_UWord32* nackRate) const { const bool defaultInstance(_childModules.empty() ? false : true); if (defaultInstance) { @@ -2354,9 +2355,11 @@ void ModuleRtpRtcpImpl::BitrateSent(WebRtc_UWord32* totalRate, RtpRtcp* module = *it; if (module) { WebRtc_UWord32 childTotalRate = 0; + WebRtc_UWord32 childVideoRate = 0; WebRtc_UWord32 childFecRate = 0; WebRtc_UWord32 childNackRate = 0; module->BitrateSent(&childTotalRate, + &childVideoRate, &childFecRate, &childNackRate); if (totalRate != NULL && childTotalRate > *totalRate) @@ -2372,6 +2375,8 @@ void ModuleRtpRtcpImpl::BitrateSent(WebRtc_UWord32* totalRate, } if (totalRate != NULL) *totalRate = _rtpSender.BitrateLast(); + if (videoRate != NULL) + *videoRate = _rtpSender.VideoBitrateSent(); if (fecRate != NULL) *fecRate = _rtpSender.FecOverheadRate(); if (nackRate != NULL) @@ -2663,8 +2668,13 @@ void ModuleRtpRtcpImpl::OnPacketLossStatisticsUpdate( if (!defaultInstance) { WebRtc_UWord32 newBitrate = 0; WebRtc_UWord8 loss = fractionLost; // local copy since it can change + WebRtc_UWord32 videoRate = 0; + WebRtc_UWord32 fecRate = 0; + WebRtc_UWord32 nackRate = 0; + BitrateSent(NULL, &videoRate, &fecRate, &nackRate); if (_bandwidthManagement.UpdatePacketLoss( lastReceivedExtendedHighSeqNum, + videoRate + fecRate + nackRate, roundTripTime, &loss, &newBitrate) != 0) { @@ -2735,10 +2745,15 @@ void ModuleRtpRtcpImpl::OnPacketLossStatisticsUpdate( // default and simulcast WebRtc_UWord32 newBitrate = 0; WebRtc_UWord8 loss = fractionLost; // local copy + WebRtc_UWord32 videoRate = 0; + WebRtc_UWord32 fecRate = 0; + WebRtc_UWord32 nackRate = 0; + BitrateSent(NULL, &videoRate, &fecRate, &nackRate); if (_bandwidthManagement.UpdatePacketLoss(0, // we can't use this - roundTripTime, - &loss, - &newBitrate) != 0) { + videoRate + fecRate + nackRate, + roundTripTime, + &loss, + &newBitrate) != 0) { // ignore this update return; } diff --git a/src/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/src/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 642f3023c..a11d54c8a 100644 --- a/src/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/src/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -460,6 +460,7 @@ public: TMMBRSet*& boundingSetRec); virtual void BitrateSent(WebRtc_UWord32* totalRate, + WebRtc_UWord32* videoRate, WebRtc_UWord32* fecRate, WebRtc_UWord32* nackRate) const; diff --git a/src/modules/rtp_rtcp/source/rtp_sender.cc b/src/modules/rtp_rtcp/source/rtp_sender.cc index 73687e499..8e8db3866 100644 --- a/src/modules/rtp_rtcp/source/rtp_sender.cc +++ b/src/modules/rtp_rtcp/source/rtp_sender.cc @@ -241,9 +241,20 @@ RTPSender::ActualSendBitrateKbit() const return (WebRtc_UWord16) (Bitrate::BitrateNow()/1000); } +WebRtc_UWord32 +RTPSender::VideoBitrateSent() const { + if (_video) + return _video->VideoBitrateSent(); + else + return 0; +} + WebRtc_UWord32 RTPSender::FecOverheadRate() const { - return _video->FecOverheadRate(); + if (_video) + return _video->FecOverheadRate(); + else + return 0; } WebRtc_UWord32 diff --git a/src/modules/rtp_rtcp/source/rtp_sender.h b/src/modules/rtp_rtcp/source/rtp_sender.h index 3d054272b..03c13cc51 100644 --- a/src/modules/rtp_rtcp/source/rtp_sender.h +++ b/src/modules/rtp_rtcp/source/rtp_sender.h @@ -73,6 +73,7 @@ public: WebRtc_UWord16 TargetSendBitrateKbit() const; WebRtc_UWord16 ActualSendBitrateKbit() const; + WebRtc_UWord32 VideoBitrateSent() const; WebRtc_UWord32 FecOverheadRate() const; WebRtc_UWord32 NackOverheadRate() const; diff --git a/src/modules/rtp_rtcp/source/rtp_sender_video.cc b/src/modules/rtp_rtcp/source/rtp_sender_video.cc index 5bb295128..3b466b27f 100644 --- a/src/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/src/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -157,7 +157,6 @@ RTPSenderVideo::SendVideoPacket(const FrameType frameType, const WebRtc_UWord16 payloadLength, const WebRtc_UWord16 rtpHeaderLength) { - int fecOverheadSent = 0; if(_fecEnabled) { WebRtc_Word32 retVal = 0; @@ -209,6 +208,10 @@ RTPSenderVideo::SendVideoPacket(const FrameType frameType, _numberFirstPartition, _fecUseUepProtection, fecPacketList); + + int fecOverheadSent = 0; + int videoSent = 0; + while(!_rtpPacketListFec.Empty()) { WebRtc_UWord8 newDataBuffer[IP_PACKET_SIZE]; @@ -218,6 +221,10 @@ RTPSenderVideo::SendVideoPacket(const FrameType frameType, RtpPacket* packetToSend = static_cast(item->GetItem()); + item = _mediaPacketListFec.First(); + ForwardErrorCorrection::Packet* mediaPacket = + static_cast(item->GetItem()); + // Copy RTP header memcpy(newDataBuffer, packetToSend->pkt->data, packetToSend->rtpHeaderLength); @@ -244,13 +251,24 @@ RTPSenderVideo::SendVideoPacket(const FrameType frameType, _mediaPacketListFec.PopFront(); // Send normal packet with RED header - retVal |= _rtpSender.SendToNetwork( + int packetSuccess = _rtpSender.SendToNetwork( newDataBuffer, packetToSend->pkt->length - packetToSend->rtpHeaderLength + REDForFECHeaderLength, packetToSend->rtpHeaderLength); + retVal |= packetSuccess; + + if (packetSuccess == 0) + { + videoSent += mediaPacket->length; + fecOverheadSent += (packetToSend->pkt->length - + mediaPacket->length + + packetToSend->rtpHeaderLength + + REDForFECHeaderLength); + } + delete packetToSend->pkt; delete packetToSend; packetToSend = NULL; @@ -295,24 +313,32 @@ RTPSenderVideo::SendVideoPacket(const FrameType frameType, // No marker bit on FEC packets, last media packet have the // marker send FEC packet with RED header - retVal |= _rtpSender.SendToNetwork( + int packetSuccess = _rtpSender.SendToNetwork( newDataBuffer, packetToSend->length + REDForFECHeaderLength, lastMediaRtpHeader.length); - if (retVal == 0) + retVal |= packetSuccess; + + if (packetSuccess == 0) { fecOverheadSent += packetToSend->length + - REDForFECHeaderLength; + REDForFECHeaderLength + lastMediaRtpHeader.length; } } + _videoBitrate.Update(videoSent); + _fecOverheadRate.Update(fecOverheadSent); } - _fecOverheadRate.Update(fecOverheadSent); return retVal; } - return _rtpSender.SendToNetwork(dataBuffer, - payloadLength, - rtpHeaderLength); + int retVal = _rtpSender.SendToNetwork(dataBuffer, + payloadLength, + rtpHeaderLength); + if (retVal == 0) + { + _videoBitrate.Update(payloadLength + rtpHeaderLength); + } + return retVal; } WebRtc_Word32 @@ -1283,9 +1309,14 @@ RTPSenderVideo::SendVP8(const FrameType frameType, } void RTPSenderVideo::ProcessBitrate() { + _videoBitrate.Process(); _fecOverheadRate.Process(); } +WebRtc_UWord32 RTPSenderVideo::VideoBitrateSent() const { + return _videoBitrate.BitrateLast(); +} + WebRtc_UWord32 RTPSenderVideo::FecOverheadRate() const { return _fecOverheadRate.BitrateLast(); } diff --git a/src/modules/rtp_rtcp/source/rtp_sender_video.h b/src/modules/rtp_rtcp/source/rtp_sender_video.h index 5a3781dcf..5a3dccfb1 100644 --- a/src/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/src/modules/rtp_rtcp/source/rtp_sender_video.h @@ -87,6 +87,7 @@ public: void ProcessBitrate(); + WebRtc_UWord32 VideoBitrateSent() const; WebRtc_UWord32 FecOverheadRate() const; protected: @@ -167,7 +168,11 @@ private: int _numberFirstPartition; ListWrapper _mediaPacketListFec; ListWrapper _rtpPacketListFec; + // Bitrate used for FEC payload, RED headers, RTP headers for FEC packets + // and any padding overhead. Bitrate _fecOverheadRate; + // Bitrate used for video payload and RTP headers + Bitrate _videoBitrate; // H263 WebRtc_UWord8 _savedByte; diff --git a/src/video_engine/main/interface/vie_rtp_rtcp.h b/src/video_engine/main/interface/vie_rtp_rtcp.h index 9a37147ac..69118d615 100644 --- a/src/video_engine/main/interface/vie_rtp_rtcp.h +++ b/src/video_engine/main/interface/vie_rtp_rtcp.h @@ -245,6 +245,7 @@ public: // bits/s. virtual int GetBandwidthUsage(const int videoChannel, unsigned int& totalBitrateSent, + unsigned int& videoBitrateSent, unsigned int& fecBitrateSent, unsigned int& nackBitrateSent) const = 0; diff --git a/src/video_engine/main/source/vie_channel.cc b/src/video_engine/main/source/vie_channel.cc index 75755eaf4..b7208df15 100644 --- a/src/video_engine/main/source/vie_channel.cc +++ b/src/video_engine/main/source/vie_channel.cc @@ -1452,6 +1452,7 @@ WebRtc_Word32 ViEChannel::GetRtpStatistics( } void ViEChannel::GetBandwidthUsage(WebRtc_UWord32& totalBitrateSent, + WebRtc_UWord32& videoBitrateSent, WebRtc_UWord32& fecBitrateSent, WebRtc_UWord32& nackBitrateSent) const { WEBRTC_TRACE(webrtc::kTraceInfo, webrtc::kTraceVideo, @@ -1459,15 +1460,17 @@ void ViEChannel::GetBandwidthUsage(WebRtc_UWord32& totalBitrateSent, "%s", __FUNCTION__); _rtpRtcp.BitrateSent(&totalBitrateSent, + &videoBitrateSent, &fecBitrateSent, &nackBitrateSent); for (std::list::const_iterator it = _simulcastRtpRtcp.begin(); it != _simulcastRtpRtcp.end(); it++) { WebRtc_UWord32 streamRate = 0; + WebRtc_UWord32 videoRate = 0; WebRtc_UWord32 fecRate = 0; WebRtc_UWord32 nackRate = 0; RtpRtcp* rtpRtcp = *it; - rtpRtcp->BitrateSent(&streamRate, &fecRate, &nackRate); + rtpRtcp->BitrateSent(&streamRate, &videoRate, &fecRate, &nackRate); totalBitrateSent += streamRate; fecBitrateSent += fecRate; nackBitrateSent += nackRate; diff --git a/src/video_engine/main/source/vie_channel.h b/src/video_engine/main/source/vie_channel.h index a68e85ed3..ead3f19f9 100644 --- a/src/video_engine/main/source/vie_channel.h +++ b/src/video_engine/main/source/vie_channel.h @@ -172,6 +172,7 @@ public: WebRtc_UWord32& packetsReceived) const; void GetBandwidthUsage(WebRtc_UWord32& totalBitrateSent, + WebRtc_UWord32& videoBitrateSent, WebRtc_UWord32& fecBitrateSent, WebRtc_UWord32& nackBitrateSent) const; diff --git a/src/video_engine/main/source/vie_rtp_rtcp_impl.cc b/src/video_engine/main/source/vie_rtp_rtcp_impl.cc index 157a5a0d5..43dfa584d 100644 --- a/src/video_engine/main/source/vie_rtp_rtcp_impl.cc +++ b/src/video_engine/main/source/vie_rtp_rtcp_impl.cc @@ -972,6 +972,7 @@ int ViERTP_RTCPImpl::GetRTPStatistics(const int videoChannel, // The function gets bandwidth usage statistics from the sent RTP streams. int ViERTP_RTCPImpl::GetBandwidthUsage(const int videoChannel, unsigned int& totalBitrateSent, + unsigned int& videoBitrateSent, unsigned int& fecBitrateSent, unsigned int& nackBitrateSent) const { WEBRTC_TRACE(webrtc::kTraceApiCall, webrtc::kTraceVideo, @@ -992,6 +993,7 @@ int ViERTP_RTCPImpl::GetBandwidthUsage(const int videoChannel, ptrViEChannel->GetBandwidthUsage( static_cast(totalBitrateSent), + static_cast(videoBitrateSent), static_cast(fecBitrateSent), static_cast(nackBitrateSent)); return 0; diff --git a/src/video_engine/main/source/vie_rtp_rtcp_impl.h b/src/video_engine/main/source/vie_rtp_rtcp_impl.h index 62f959b9f..5c3a479cd 100644 --- a/src/video_engine/main/source/vie_rtp_rtcp_impl.h +++ b/src/video_engine/main/source/vie_rtp_rtcp_impl.h @@ -112,6 +112,7 @@ public: virtual int GetBandwidthUsage(const int videoChannel, unsigned int& totalBitrateSent, + unsigned int& videoBitrateSent, unsigned int& fecBitrateSent, unsigned int& nackBitrateSent) const; diff --git a/src/video_engine/main/test/AutoTest/source/vie_autotest_rtp_rtcp.cc b/src/video_engine/main/test/AutoTest/source/vie_autotest_rtp_rtcp.cc index d6ab12fc0..b82a069e5 100644 --- a/src/video_engine/main/test/AutoTest/source/vie_autotest_rtp_rtcp.cc +++ b/src/video_engine/main/test/AutoTest/source/vie_autotest_rtp_rtcp.cc @@ -233,11 +233,13 @@ int ViEAutoTest::ViERtpRtcpStandardTest() int recRttMs = 0; unsigned int sentTotalBitrate = 0; + unsigned int sentVideoBitrate = 0; unsigned int sentFecBitrate = 0; unsigned int sentNackBitrate = 0; error = ViE.ptrViERtpRtcp->GetBandwidthUsage(tbChannel.videoChannel, sentTotalBitrate, + sentVideoBitrate, sentFecBitrate, sentNackBitrate); numberOfErrors += ViETest::TestError(error == 0, "ERROR: %s at line %d", @@ -311,6 +313,7 @@ int ViEAutoTest::ViERtpRtcpStandardTest() error = ViE.ptrViERtpRtcp->GetBandwidthUsage(tbChannel.videoChannel, sentTotalBitrate, + sentVideoBitrate, sentFecBitrate, sentNackBitrate); numberOfErrors += ViETest::TestError(error == 0, "ERROR: %s at line %d", @@ -340,6 +343,7 @@ int ViEAutoTest::ViERtpRtcpStandardTest() error = ViE.ptrViERtpRtcp->GetBandwidthUsage(tbChannel.videoChannel, sentTotalBitrate, + sentVideoBitrate, sentFecBitrate, sentNackBitrate); numberOfErrors += ViETest::TestError(error == 0, "ERROR: %s at line %d",