From de98478965979e3de7578caae192c5110bc578ef Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Tue, 4 Jun 2013 12:15:40 +0000 Subject: [PATCH] Update the remote bitrate estimator before passing the packet to the RTP module. This solves the problem of reconstructed packets biasing the bandwidth estimate. TEST=vie_auto_test --automated, trybots R=mflodman@webrtc.org, solenberg@webrtc.org Review URL: https://webrtc-codereview.appspot.com/1594005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4171 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../include/remote_bitrate_estimator.h | 2 +- .../remote_bitrate_estimator_multi_stream.cc | 10 +++++----- .../remote_bitrate_estimator_single_stream.cc | 10 +++++----- .../remote_bitrate_estimator_unittest_helper.cc | 8 ++++---- webrtc/video_engine/vie_channel_group.cc | 2 +- webrtc/video_engine/vie_receiver.cc | 9 +++------ 6 files changed, 19 insertions(+), 22 deletions(-) diff --git a/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h index 2ccb92cc55..3d0167b5fd 100644 --- a/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h +++ b/webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h @@ -55,7 +55,7 @@ class RemoteBitrateEstimator : public CallStatsObserver, public Module { // the WebRtcRTPHeader to be initialized. virtual void IncomingPacket(int64_t arrival_time_ms, int payload_size, - const WebRtcRTPHeader& header) = 0; + const RTPHeader& header) = 0; // Removes all data for |ssrc|. virtual void RemoveStream(unsigned int ssrc) = 0; diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.cc index c92914e7d8..90c6f24a33 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_multi_stream.cc @@ -44,7 +44,7 @@ class RemoteBitrateEstimatorMultiStream : public RemoteBitrateEstimator { // Note that |payload_size| is the packet size excluding headers. virtual void IncomingPacket(int64_t arrival_time_ms, int payload_size, - const WebRtcRTPHeader& header); + const RTPHeader& header); // Triggers a new estimate calculation. // Implements the Module interface. @@ -139,10 +139,10 @@ void RemoteBitrateEstimatorMultiStream::IncomingRtcp(unsigned int ssrc, void RemoteBitrateEstimatorMultiStream::IncomingPacket( int64_t arrival_time_ms, int payload_size, - const WebRtcRTPHeader& header) { - uint32_t ssrc = header.header.ssrc; - uint32_t rtp_timestamp = header.header.timestamp + - header.header.extension.transmissionTimeOffset; + const RTPHeader& header) { + uint32_t ssrc = header.ssrc; + uint32_t rtp_timestamp = header.timestamp + + header.extension.transmissionTimeOffset; CriticalSectionScoped cs(crit_sect_.get()); incoming_bitrate_.Update(payload_size, arrival_time_ms); // Add this stream to the map of streams if it doesn't already exist. diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc index 864c8da343..4cbfbbef9d 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc @@ -37,7 +37,7 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator { // packet size excluding headers. virtual void IncomingPacket(int64_t arrival_time_ms, int payload_size, - const WebRtcRTPHeader& header); + const RTPHeader& header); // Triggers a new estimate calculation. // Implements the Module interface. @@ -86,10 +86,10 @@ RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream( void RemoteBitrateEstimatorSingleStream::IncomingPacket( int64_t arrival_time_ms, int payload_size, - const WebRtcRTPHeader& header) { - uint32_t ssrc = header.header.ssrc; - uint32_t rtp_timestamp = header.header.timestamp + - header.header.extension.transmissionTimeOffset; + const RTPHeader& header) { + uint32_t ssrc = header.ssrc; + uint32_t rtp_timestamp = header.timestamp + + header.extension.transmissionTimeOffset; CriticalSectionScoped cs(crit_sect_.get()); SsrcOveruseDetectorMap::iterator it = overuse_detectors_.find(ssrc); if (it == overuse_detectors_.end()) { diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc index 7ae23687fe..6c02109803 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc @@ -223,11 +223,11 @@ void RemoteBitrateEstimatorTest::IncomingPacket(uint32_t ssrc, int64_t arrival_time, uint32_t rtp_timestamp, uint32_t absolute_send_time) { - WebRtcRTPHeader header; + RTPHeader header; memset(&header, 0, sizeof(header)); - header.header.ssrc = ssrc; - header.header.timestamp = rtp_timestamp; - header.header.extension.absoluteSendTime = absolute_send_time; + header.ssrc = ssrc; + header.timestamp = rtp_timestamp; + header.extension.absoluteSendTime = absolute_send_time; bitrate_estimator_->IncomingPacket(arrival_time, payload_size, header); } diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index 191230df49..596ba33d3f 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -68,7 +68,7 @@ class WrappingBitrateEstimator : public RemoteBitrateEstimator { virtual void IncomingPacket(int64_t arrival_time_ms, int payload_size, - const WebRtcRTPHeader& header) { + const RTPHeader& header) { CriticalSectionScoped cs(crit_sect_.get()); rbe_->IncomingPacket(arrival_time_ms, payload_size, header); } diff --git a/webrtc/video_engine/vie_receiver.cc b/webrtc/video_engine/vie_receiver.cc index 3ec0eed8cd..0284ad53b6 100644 --- a/webrtc/video_engine/vie_receiver.cc +++ b/webrtc/video_engine/vie_receiver.cc @@ -133,12 +133,6 @@ int32_t ViEReceiver::OnReceivedPayloadData( if (rtp_header == NULL) { return 0; } - - // TODO(holmer): Make sure packets reconstructed using FEC are not passed to - // the bandwidth estimator. - const int packet_size = payload_size + rtp_header->header.paddingLength; - remote_bitrate_estimator_->IncomingPacket(TickTime::MillisecondTimestamp(), - packet_size, *rtp_header); if (vcm_->IncomingPacket(payload_data, payload_size, *rtp_header) != 0) { // Check this... return -1; @@ -197,6 +191,9 @@ int ViEReceiver::InsertRTPPacket(const int8_t* rtp_packet, "IncomingPacket invalid RTP header"); return -1; } + const int payload_size = received_packet_length - header.headerLength; + remote_bitrate_estimator_->IncomingPacket(TickTime::MillisecondTimestamp(), + payload_size, header); assert(rtp_rtcp_); // Should be set by owner at construction time. return rtp_rtcp_->IncomingRtpPacket(received_packet, received_packet_length, header);