From 4346d92578e5acbf3c40c89967c548e8f72e7543 Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Wed, 18 Mar 2015 13:40:54 +0000 Subject: [PATCH] Use SendTimeHistory to keep track of send times in simulations. Use SendTimeHistory to keep track of send times in simulations. Keep piggybacking send time in PacketInfo for now but use history in order to be more in line with what we expect to do. Landing this for sprang@. Original CL: https://review.webrtc.org/43559004/ TBR=sprang@webrtc.org BUG=4308 Review URL: https://webrtc-codereview.appspot.com/48569004 Cr-Commit-Position: refs/heads/master@{#8778} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8778 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../modules/interface/module_common_types.h | 1 + .../bwe_simulations.cc | 1 + .../remote_bitrate_estimator/test/bwe.cc | 1 + .../remote_bitrate_estimator/test/bwe.h | 1 + .../test/estimators/nada.h | 1 + .../test/estimators/remb.h | 1 + .../test/estimators/send_side.cc | 26 +++++++++++++++++-- .../test/estimators/send_side.h | 3 +++ .../remote_bitrate_estimator/test/packet.h | 11 +++++++- .../test/packet_sender.cc | 12 ++++++--- 10 files changed, 51 insertions(+), 7 deletions(-) diff --git a/webrtc/modules/interface/module_common_types.h b/webrtc/modules/interface/module_common_types.h index 8322ba2c4d..66bbf1994f 100644 --- a/webrtc/modules/interface/module_common_types.h +++ b/webrtc/modules/interface/module_common_types.h @@ -321,6 +321,7 @@ class EncodedVideoData { if (data.payloadSize > 0) { payloadData = new uint8_t[data.payloadSize]; memcpy(payloadData, data.payloadData, data.payloadSize); + bufferSize = data.payloadSize; } else { payloadData = NULL; } diff --git a/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc b/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc index c0d5f22b0d..df82764a6c 100644 --- a/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc +++ b/webrtc/modules/remote_bitrate_estimator/bwe_simulations.cc @@ -9,6 +9,7 @@ */ #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/base/scoped_ptr.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe_test.h" #include "webrtc/modules/remote_bitrate_estimator/test/packet_receiver.h" diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe.cc b/webrtc/modules/remote_bitrate_estimator/test/bwe.cc index bd22e3eb44..3b233709c8 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe.cc @@ -28,6 +28,7 @@ class NullBweSender : public BweSender { int GetFeedbackIntervalMs() const override { return 1000; } void GiveFeedback(const FeedbackPacket& feedback) override {} + void OnPacketsSent(const Packets& packets) override {} int64_t TimeUntilNextProcess() override { return std::numeric_limits::max(); } diff --git a/webrtc/modules/remote_bitrate_estimator/test/bwe.h b/webrtc/modules/remote_bitrate_estimator/test/bwe.h index a0515db8cd..0bab5a98a1 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/bwe.h +++ b/webrtc/modules/remote_bitrate_estimator/test/bwe.h @@ -30,6 +30,7 @@ class BweSender : public Module { virtual int GetFeedbackIntervalMs() const = 0; virtual void GiveFeedback(const FeedbackPacket& feedback) = 0; + virtual void OnPacketsSent(const Packets& packets) = 0; private: DISALLOW_COPY_AND_ASSIGN(BweSender); diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/nada.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/nada.h index 353fac4d9f..14cc8a2733 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/nada.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/nada.h @@ -45,6 +45,7 @@ class NadaBweSender : public BweSender { int GetFeedbackIntervalMs() const override; void GiveFeedback(const FeedbackPacket& feedback) override; + void OnPacketsSent(const Packets& packets) override {} int64_t TimeUntilNextProcess() override; int Process() override; diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h index a88d8a75cb..753f152598 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/remb.h @@ -33,6 +33,7 @@ class RembBweSender : public BweSender { int GetFeedbackIntervalMs() const override; void GiveFeedback(const FeedbackPacket& feedback) override; + void OnPacketsSent(const Packets& packets) override {} int64_t TimeUntilNextProcess() override; int Process() override; diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc index 88acdae8fe..c890b6aa0f 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc @@ -10,6 +10,8 @@ #include "webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h" +#include "webrtc/base/logging.h" + namespace webrtc { namespace testing { namespace bwe { @@ -20,7 +22,8 @@ FullBweSender::FullBweSender(int kbps, BitrateObserver* observer, Clock* clock) rbe_(AbsoluteSendTimeRemoteBitrateEstimatorFactory() .Create(this, clock, kAimdControl, 1000 * kMinBitrateKbps)), feedback_observer_(bitrate_controller_->CreateRtcpBandwidthObserver()), - clock_(clock) { + clock_(clock), + send_time_history_(10000) { assert(kbps >= kMinBitrateKbps); assert(kbps <= kMaxBitrateKbps); bitrate_controller_->SetStartBitrate(1000 * kbps); @@ -40,7 +43,15 @@ void FullBweSender::GiveFeedback(const FeedbackPacket& feedback) { static_cast(feedback); if (fb.packet_feedback_vector().empty()) return; - rbe_->IncomingPacketFeedbackVector(fb.packet_feedback_vector()); + // TODO(sprang): Unconstify PacketInfo so we don't need temp copy? + std::vector packet_feedback_vector(fb.packet_feedback_vector()); + for (PacketInfo& packet : packet_feedback_vector) { + if (!send_time_history_.GetSendTime(packet.sequence_number, + &packet.send_time_ms, true)) { + LOG(LS_WARNING) << "Ack arrived too late."; + } + } + rbe_->IncomingPacketFeedbackVector(packet_feedback_vector); // TODO(holmer): Handle losses in between feedback packets. int expected_packets = fb.packet_feedback_vector().back().sequence_number - fb.packet_feedback_vector().front().sequence_number + @@ -58,6 +69,17 @@ void FullBweSender::GiveFeedback(const FeedbackPacket& feedback) { bitrate_controller_->Process(); } +void FullBweSender::OnPacketsSent(const Packets& packets) { + for (Packet* packet : packets) { + if (packet->GetPacketType() == Packet::kMedia) { + MediaPacket* media_packet = static_cast(packet); + send_time_history_.AddAndRemoveOldSendTimes( + media_packet->header().sequenceNumber, + media_packet->GetAbsSendTimeInMs()); + } + } +} + void FullBweSender::OnReceiveBitrateChanged( const std::vector& ssrcs, unsigned int bitrate) { diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h index 68d4805286..007ea4e0cc 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h @@ -13,6 +13,7 @@ #include +#include "webrtc/modules/bitrate_controller/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe.h" namespace webrtc { @@ -26,6 +27,7 @@ class FullBweSender : public BweSender, public RemoteBitrateObserver { int GetFeedbackIntervalMs() const override; void GiveFeedback(const FeedbackPacket& feedback) override; + void OnPacketsSent(const Packets& packets) override; void OnReceiveBitrateChanged(const std::vector& ssrcs, unsigned int bitrate) override; int64_t TimeUntilNextProcess() override; @@ -39,6 +41,7 @@ class FullBweSender : public BweSender, public RemoteBitrateObserver { private: Clock* const clock_; RTCPReportBlock report_block_; + SendTimeHistory send_time_history_; DISALLOW_IMPLICIT_CONSTRUCTORS(FullBweSender); }; diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet.h b/webrtc/modules/remote_bitrate_estimator/test/packet.h index cf21615adf..12d4a3ee93 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet.h +++ b/webrtc/modules/remote_bitrate_estimator/test/packet.h @@ -57,12 +57,21 @@ class MediaPacket : public Packet { MediaPacket(int64_t send_time_us, uint32_t sequence_number); virtual ~MediaPacket() {} - int64_t GetAbsSendTimeInMs() const; + int64_t GetAbsSendTimeInMs() const { + int64_t timestamp = header_.extension.absoluteSendTime + << kAbsSendTimeInterArrivalUpshift; + return 1000.0 * timestamp / static_cast(1 << kInterArrivalShift); + } void SetAbsSendTimeMs(int64_t abs_send_time_ms); const RTPHeader& header() const { return header_; } virtual Packet::Type GetPacketType() const { return kMedia; } private: + static const int kAbsSendTimeFraction = 18; + static const int kAbsSendTimeInterArrivalUpshift = 8; + static const int kInterArrivalShift = + kAbsSendTimeFraction + kAbsSendTimeInterArrivalUpshift; + RTPHeader header_; }; diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc index 36690b608b..2a34d22c63 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc @@ -22,14 +22,14 @@ namespace bwe { PacketSender::PacketSender(PacketProcessorListener* listener, VideoSource* source, - BandwidthEstimatorType estimator) + BandwidthEstimatorType estimator_type) : PacketProcessor(listener, source->flow_id(), kSender), // For Packet::send_time_us() to be comparable with timestamps from // clock_, the clock of the PacketSender and the Source must be aligned. // We assume that both start at time 0. clock_(0), source_(source), - bwe_(CreateBweSender(estimator, + bwe_(CreateBweSender(estimator_type, source_->bits_per_second() / 1000, this, &clock_)) { @@ -49,7 +49,7 @@ void PacketSender::RunFor(int64_t time_ms, Packets* in_out) { void PacketSender::ProcessFeedbackAndGeneratePackets( int64_t time_ms, std::list* feedbacks, - Packets* generated) { + Packets* packets) { do { // Make sure to at least run Process() below every 100 ms. int64_t time_to_run_ms = std::min(time_ms, 100); @@ -60,7 +60,10 @@ void PacketSender::ProcessFeedbackAndGeneratePackets( time_to_run_ms = std::max(std::min(time_ms, time_until_feedback_ms), 0); } - source_->RunFor(time_to_run_ms, generated); + Packets generated; + source_->RunFor(time_to_run_ms, &generated); + bwe_->OnPacketsSent(generated); + packets->merge(generated, DereferencingComparator); clock_.AdvanceTimeMilliseconds(time_to_run_ms); if (!feedbacks->empty()) { bwe_->GiveFeedback(*feedbacks->front()); @@ -222,6 +225,7 @@ void PacedVideoSender::QueuePackets(Packets* batch, } Packets to_transfer; to_transfer.splice(to_transfer.begin(), queue_, queue_.begin(), it); + bwe_->OnPacketsSent(to_transfer); batch->merge(to_transfer, DereferencingComparator); }