diff --git a/webrtc/modules/pacing/include/paced_sender.h b/webrtc/modules/pacing/include/paced_sender.h index 45c89b77e..95f1a86e8 100644 --- a/webrtc/modules/pacing/include/paced_sender.h +++ b/webrtc/modules/pacing/include/paced_sender.h @@ -56,8 +56,7 @@ class PacedSender : public Module { static const int kDefaultMaxQueueLengthMs = 2000; - PacedSender(Callback* callback, int target_bitrate_kbps, - float pace_multiplier); + PacedSender(Callback* callback, int max_bitrate_kbps, int min_bitrate_kbps); virtual ~PacedSender(); @@ -72,13 +71,9 @@ class PacedSender : public Module { // Resume sending packets. void Resume(); - // Set the pacing target bitrate and the bitrate up to which we are allowed to - // pad. We will send padding packets to increase the total bitrate until we - // reach |pad_up_to_bitrate_kbps|. If the media bitrate is above - // |pad_up_to_bitrate_kbps| no padding will be sent. - void UpdateBitrate(int target_bitrate_kbps, - int max_padding_bitrate_kbps, - int pad_up_to_bitrate_kbps); + // Set target bitrates for the pacer. Padding packets will be utilized to + // reach |min_bitrate| unless enough media packets are available. + void UpdateBitrate(int max_bitrate_kbps, int min_bitrate_kbps); // Returns true if we send the packet now, else it will add the packet // information to the queue and call TimeToSendPacket when it's time to send. @@ -120,7 +115,6 @@ class PacedSender : public Module { void UpdateMediaBytesSent(int num_bytes); Callback* callback_; - const float pace_multiplier_; bool enabled_; bool paused_; int max_queue_length_ms_; @@ -129,12 +123,9 @@ class PacedSender : public Module { // we can pace out during the current interval. scoped_ptr media_budget_; // This is the padding budget, keeping track of how many bits of padding we're - // allowed to send out during the current interval. + // allowed to send out during the current interval. This budget will be + // utilized when there's no media to send. scoped_ptr padding_budget_; - // Media and padding share this budget, therefore no padding will be sent if - // media uses all of this budget. This is used to avoid padding above a given - // bitrate. - scoped_ptr pad_up_to_bitrate_budget_; TickTime time_last_update_; TickTime time_last_send_; diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index e47614c89..125d3c0cd 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -120,19 +120,16 @@ class IntervalBudget { }; } // namespace paced_sender -PacedSender::PacedSender(Callback* callback, int target_bitrate_kbps, - float pace_multiplier) +PacedSender::PacedSender(Callback* callback, + int max_bitrate_kbps, + int min_bitrate_kbps) : callback_(callback), - pace_multiplier_(pace_multiplier), enabled_(false), paused_(false), max_queue_length_ms_(kDefaultMaxQueueLengthMs), critsect_(CriticalSectionWrapper::CreateCriticalSection()), - media_budget_(new paced_sender::IntervalBudget( - pace_multiplier_ * target_bitrate_kbps)), - padding_budget_(new paced_sender::IntervalBudget(0)), - // No padding until UpdateBitrate is called. - pad_up_to_bitrate_budget_(new paced_sender::IntervalBudget(0)), + media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)), + padding_budget_(new paced_sender::IntervalBudget(min_bitrate_kbps)), time_last_update_(TickTime::Now()), capture_time_ms_last_queued_(0), capture_time_ms_last_sent_(0), @@ -165,13 +162,11 @@ bool PacedSender::Enabled() const { return enabled_; } -void PacedSender::UpdateBitrate(int target_bitrate_kbps, - int max_padding_bitrate_kbps, - int pad_up_to_bitrate_kbps) { +void PacedSender::UpdateBitrate(int max_bitrate_kbps, + int min_bitrate_kbps) { CriticalSectionScoped cs(critsect_.get()); - media_budget_->set_target_rate_kbps(pace_multiplier_ * target_bitrate_kbps); - padding_budget_->set_target_rate_kbps(max_padding_bitrate_kbps); - pad_up_to_bitrate_budget_->set_target_rate_kbps(pad_up_to_bitrate_kbps); + media_budget_->set_target_rate_kbps(max_bitrate_kbps); + padding_budget_->set_target_rate_kbps(min_bitrate_kbps); } bool PacedSender::SendPacket(Priority priority, uint32_t ssrc, @@ -273,17 +268,13 @@ int32_t PacedSender::Process() { if (high_priority_packets_->empty() && normal_priority_packets_->empty() && low_priority_packets_->empty() && - padding_budget_->bytes_remaining() > 0 && - pad_up_to_bitrate_budget_->bytes_remaining() > 0) { - int padding_needed = std::min( - padding_budget_->bytes_remaining(), - pad_up_to_bitrate_budget_->bytes_remaining()); + padding_budget_->bytes_remaining() > 0) { + int padding_needed = padding_budget_->bytes_remaining(); critsect_->Leave(); int bytes_sent = callback_->TimeToSendPadding(padding_needed); critsect_->Enter(); media_budget_->UseBudget(bytes_sent); padding_budget_->UseBudget(bytes_sent); - pad_up_to_bitrate_budget_->UseBudget(bytes_sent); } } return 0; @@ -324,7 +315,6 @@ bool PacedSender::SendPacketFromList(paced_sender::PacketList* packet_list) void PacedSender::UpdateBytesPerInterval(uint32_t delta_time_ms) { media_budget_->IncreaseBudget(delta_time_ms); padding_budget_->IncreaseBudget(delta_time_ms); - pad_up_to_bitrate_budget_->IncreaseBudget(delta_time_ms); } // MUST have critsect_ when calling. @@ -388,7 +378,7 @@ paced_sender::Packet PacedSender::GetNextPacketFromList( void PacedSender::UpdateMediaBytesSent(int num_bytes) { time_last_send_ = TickTime::Now(); media_budget_->UseBudget(num_bytes); - pad_up_to_bitrate_budget_->UseBudget(num_bytes); + padding_budget_->UseBudget(num_bytes); } } // namespace webrtc diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc index a99101db3..435c93c54 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -59,8 +59,8 @@ class PacedSenderTest : public ::testing::Test { srand(0); TickTime::UseFakeClock(123456); // Need to initialize PacedSender after we initialize clock. - send_bucket_.reset(new PacedSender(&callback_, kTargetBitrate, - kPaceMultiplier)); + send_bucket_.reset( + new PacedSender(&callback_, kPaceMultiplier * kTargetBitrate, 0)); send_bucket_->SetStatus(true); } @@ -209,7 +209,7 @@ TEST_F(PacedSenderTest, Padding) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; - send_bucket_->UpdateBitrate(kTargetBitrate, kTargetBitrate, kTargetBitrate); + send_bucket_->UpdateBitrate(kPaceMultiplier * kTargetBitrate, kTargetBitrate); // Due to the multiplicative factor we can send 3 packets not 2 packets. SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, TickTime::MillisecondTimestamp(), 250, false); @@ -235,7 +235,7 @@ TEST_F(PacedSenderTest, Padding) { TEST_F(PacedSenderTest, NoPaddingWhenDisabled) { send_bucket_->SetStatus(false); - send_bucket_->UpdateBitrate(kTargetBitrate, kTargetBitrate, kTargetBitrate); + send_bucket_->UpdateBitrate(kPaceMultiplier * kTargetBitrate, kTargetBitrate); // No padding is expected since the pacer is disabled. EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0); EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); @@ -255,7 +255,7 @@ TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) { int64_t capture_time_ms = 56789; const int kTimeStep = 5; const int64_t kBitrateWindow = 100; - send_bucket_->UpdateBitrate(kTargetBitrate, kTargetBitrate, kTargetBitrate); + send_bucket_->UpdateBitrate(kPaceMultiplier * kTargetBitrate, kTargetBitrate); int64_t start_time = TickTime::MillisecondTimestamp(); while (TickTime::MillisecondTimestamp() - start_time < kBitrateWindow) { SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, @@ -267,27 +267,6 @@ TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) { } } -TEST_F(PacedSenderTest, VerifyMaxPaddingBitrate) { - uint32_t ssrc = 12345; - uint16_t sequence_number = 1234; - int64_t capture_time_ms = 56789; - const int kTimeStep = 5; - const int64_t kBitrateWindow = 100; - const int kTargetBitrate = 1500; - const int kMaxPaddingBitrate = 800; - send_bucket_->UpdateBitrate(kTargetBitrate, kMaxPaddingBitrate, - kTargetBitrate); - int64_t start_time = TickTime::MillisecondTimestamp(); - while (TickTime::MillisecondTimestamp() - start_time < kBitrateWindow) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - capture_time_ms, 250, false); - TickTime::AdvanceFakeClock(kTimeStep); - EXPECT_CALL(callback_, TimeToSendPadding(500)).Times(1). - WillOnce(Return(250)); - send_bucket_->Process(); - } -} - TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; @@ -295,10 +274,10 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) { const int kTimeStep = 5; const int64_t kBitrateWindow = 10000; PacedSenderPadding callback; - send_bucket_.reset(new PacedSender(&callback, kTargetBitrate, - kPaceMultiplier)); + send_bucket_.reset( + new PacedSender(&callback, kPaceMultiplier * kTargetBitrate, 0)); send_bucket_->SetStatus(true); - send_bucket_->UpdateBitrate(kTargetBitrate, kTargetBitrate, kTargetBitrate); + send_bucket_->UpdateBitrate(kPaceMultiplier * kTargetBitrate, kTargetBitrate); int64_t start_time = TickTime::MillisecondTimestamp(); int media_bytes = 0; while (TickTime::MillisecondTimestamp() - start_time < kBitrateWindow) { @@ -497,7 +476,7 @@ TEST_F(PacedSenderTest, MaxQueueLength) { uint16_t sequence_number = 1234; EXPECT_EQ(0, send_bucket_->QueueInMs()); - send_bucket_->UpdateBitrate(30, 0, 0); + send_bucket_->UpdateBitrate(kPaceMultiplier * 30, 0); for (int i = 0; i < 30; ++i) { SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, @@ -526,7 +505,7 @@ TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) { uint16_t sequence_number = 1234; EXPECT_EQ(0, send_bucket_->QueueInMs()); - send_bucket_->UpdateBitrate(30, 0, 0); + send_bucket_->UpdateBitrate(kPaceMultiplier * 30, 0); SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number, diff --git a/webrtc/video/call_perf_tests.cc b/webrtc/video/call_perf_tests.cc index e2f4775ea..9edf5aed2 100644 --- a/webrtc/video/call_perf_tests.cc +++ b/webrtc/video/call_perf_tests.cc @@ -398,7 +398,7 @@ TEST_F(CallPerfTest, RegisterCpuOveruseObserver) { void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) { static const int kMaxEncodeBitrateKbps = 30; - static const int kMinTransmitBitrateKbps = 150; + static const int kMinTransmitBitrateBps = 150000; static const int kMinAcceptableTransmitBitrate = 130; static const int kMaxAcceptableTransmitBitrate = 170; static const int kNumBitrateObservationsInRange = 100; @@ -475,9 +475,9 @@ void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) { send_config.pacing = true; if (pad_to_min_bitrate) { - send_config.rtp.min_transmit_bitrate_kbps = kMinTransmitBitrateKbps; + send_config.rtp.min_transmit_bitrate_bps = kMinTransmitBitrateBps; } else { - assert(send_config.rtp.min_transmit_bitrate_kbps == 0); + assert(send_config.rtp.min_transmit_bitrate_bps == 0); } VideoReceiveStream::Config receive_config = diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index a480acd21..a7eebef73 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -50,9 +50,9 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport, config_.pacing = true; rtp_rtcp_->SetTransmissionSmoothingStatus(channel_, config_.pacing); - assert(config_.rtp.min_transmit_bitrate_kbps >= 0); + assert(config_.rtp.min_transmit_bitrate_bps >= 0); rtp_rtcp_->SetMinTransmitBitrate(channel_, - config_.rtp.min_transmit_bitrate_kbps); + config_.rtp.min_transmit_bitrate_bps / 1000); for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) { const std::string& extension = config_.rtp.extensions[i].name; diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 5a4969884..971926c60 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -15,6 +15,7 @@ #include "webrtc/common_video/interface/i420_video_frame.h" #include "webrtc/frame_callback.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h" +#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_sender.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" #include "webrtc/system_wrappers/interface/critical_section_wrapper.h" @@ -23,12 +24,13 @@ #include "webrtc/system_wrappers/interface/sleep.h" #include "webrtc/system_wrappers/interface/thread_wrapper.h" #include "webrtc/test/direct_transport.h" +#include "webrtc/test/configurable_frame_size_encoder.h" #include "webrtc/test/encoder_settings.h" #include "webrtc/test/fake_encoder.h" -#include "webrtc/test/configurable_frame_size_encoder.h" #include "webrtc/test/frame_generator_capturer.h" #include "webrtc/test/null_transport.h" #include "webrtc/test/rtp_rtcp_observer.h" +#include "webrtc/test/testsupport/perf_test.h" #include "webrtc/video/transport_adapter.h" #include "webrtc/video_send_stream.h" @@ -1117,4 +1119,94 @@ TEST_F(VideoSendStreamTest, ProducesStats) { call->DestroyVideoSendStream(send_stream_); } +// This test first observes "high" bitrate use at which point it sends a REMB to +// indicate that it should be lowered significantly. The test then observes that +// the bitrate observed is sinking well below the min-transmit-bitrate threshold +// to verify that the min-transmit bitrate respects incoming REMB. +TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { + static const int kMinTransmitBitrateBps = 400000; + static const int kHighBitrateBps = 200000; + static const int kRembBitrateBps = 80000; + static const int kRembRespectedBitrateBps = 100000; + class BitrateObserver: public test::RtpRtcpObserver, public PacketReceiver { + public: + BitrateObserver() + : RtpRtcpObserver(30 * 1000), + feedback_transport_(ReceiveTransport()), + send_stream_(NULL), + bitrate_capped_(false) { + RtpRtcp::Configuration config; + feedback_transport_.Enable(); + config.outgoing_transport = &feedback_transport_; + rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(config)); + rtp_rtcp_->SetREMBStatus(true); + rtp_rtcp_->SetRTCPStatus(kRtcpNonCompound); + } + + void SetSendStream(VideoSendStream* send_stream) { + send_stream_ = send_stream; + } + + private: + virtual bool DeliverPacket(const uint8_t* packet, size_t length) { + if (RtpHeaderParser::IsRtcp(packet, static_cast(length))) + return true; + + RTPHeader header; + if (!parser_->Parse(packet, static_cast(length), &header)) + return true; + assert(send_stream_ != NULL); + VideoSendStream::Stats stats = send_stream_->GetStats(); + if (!stats.substreams.empty()) { + EXPECT_EQ(1u, stats.substreams.size()); + int bitrate_bps = stats.substreams.begin()->second.bitrate_bps; + test::PrintResult( + "bitrate_stats_", + "min_transmit_bitrate_low_remb", + "bitrate_bps", + static_cast(bitrate_bps), + "bps", + false); + if (bitrate_bps > kHighBitrateBps) { + rtp_rtcp_->SetREMBData(kRembBitrateBps, 1, &header.ssrc); + rtp_rtcp_->Process(); + bitrate_capped_ = true; + } else if (bitrate_capped_ && + bitrate_bps < kRembRespectedBitrateBps) { + observation_complete_->Set(); + } + } + return true; + } + + scoped_ptr rtp_rtcp_; + internal::TransportAdapter feedback_transport_; + VideoSendStream* send_stream_; + bool bitrate_capped_; + } observer; + + Call::Config call_config(observer.SendTransport()); + scoped_ptr call(Call::Create(call_config)); + observer.SetReceivers(&observer, call->Receiver()); + + VideoSendStream::Config send_config = GetSendTestConfig(call.get(), 1); + send_config.rtp.min_transmit_bitrate_bps = kMinTransmitBitrateBps; + send_stream_ = call->CreateVideoSendStream(send_config); + observer.SetSendStream(send_stream_); + + scoped_ptr frame_generator_capturer( + test::FrameGeneratorCapturer::Create( + send_stream_->Input(), 320, 240, 30, Clock::GetRealTimeClock())); + send_stream_->StartSending(); + frame_generator_capturer->Start(); + + EXPECT_EQ(kEventSignaled, observer.Wait()) + << "Timeout while waiting for low bitrate stats after REMB."; + + observer.StopSending(); + frame_generator_capturer->Stop(); + send_stream_->StopSending(); + call->DestroyVideoSendStream(send_stream_); +} + } // namespace webrtc diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index 5583c7585..fd619a0ce 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -465,8 +465,8 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) { if (pad_up_to_bitrate_kbps < min_transmit_bitrate_kbps_) pad_up_to_bitrate_kbps = min_transmit_bitrate_kbps_; - paced_sender_->UpdateBitrate( - video_codec.startBitrate, pad_up_to_bitrate_kbps, pad_up_to_bitrate_kbps); + paced_sender_->UpdateBitrate(kPaceMultiplier * video_codec.startBitrate, + pad_up_to_bitrate_kbps); return 0; } @@ -1066,58 +1066,47 @@ void ViEEncoder::OnNetworkChanged(const uint32_t bitrate_bps, // 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 // available bandwidth is. - int max_padding_bitrate_kbps = 0; int pad_up_to_bitrate_kbps = 0; if (send_codec.numberOfSimulcastStreams == 0) { - max_padding_bitrate_kbps = send_codec.minBitrate; pad_up_to_bitrate_kbps = send_codec.minBitrate; } else { - int i = send_codec.numberOfSimulcastStreams - 1; - for (std::vector::reverse_iterator it = stream_bitrates.rbegin(); - it != stream_bitrates.rend(); ++it) { - if (*it > 0) { - max_padding_bitrate_kbps = std::min((*it + 500) / 1000, - stream_configs[i].minBitrate); - break; - } - --i; - } pad_up_to_bitrate_kbps = stream_configs[send_codec.numberOfSimulcastStreams - 1].minBitrate; for (int i = 0; i < send_codec.numberOfSimulcastStreams - 1; ++i) { pad_up_to_bitrate_kbps += stream_configs[i].targetBitrate; } } - if (video_is_suspended || send_codec.numberOfSimulcastStreams > 1) { - pad_up_to_bitrate_kbps = std::min(bitrate_kbps, pad_up_to_bitrate_kbps); - } else { - // Disable padding if only sending one stream and video isn't suspended. + + // Disable padding if only sending one stream and video isn't suspended and + // min-transmit bitrate isn't used (applied later). + if (!video_is_suspended && send_codec.numberOfSimulcastStreams <= 1) pad_up_to_bitrate_kbps = 0; - } { - // The amount of padding should decay to zero if no frames are being - // captured. CriticalSectionScoped cs(data_cs_.get()); + // The amount of padding should decay to zero if no frames are being + // captured unless a min-transmit bitrate is used. int64_t now_ms = TickTime::MillisecondTimestamp(); if (now_ms - time_of_last_incoming_frame_ms_ > kStopPaddingThresholdMs) - max_padding_bitrate_kbps = 0; - } + pad_up_to_bitrate_kbps = 0; - { - CriticalSectionScoped cs(data_cs_.get()); + // Pad up to min bitrate. if (pad_up_to_bitrate_kbps < min_transmit_bitrate_kbps_) pad_up_to_bitrate_kbps = min_transmit_bitrate_kbps_; - if (max_padding_bitrate_kbps < min_transmit_bitrate_kbps_) - max_padding_bitrate_kbps = min_transmit_bitrate_kbps_; - paced_sender_->UpdateBitrate( - bitrate_kbps, max_padding_bitrate_kbps, pad_up_to_bitrate_kbps); + + // Padding may never exceed bitrate estimate. + if (pad_up_to_bitrate_kbps > bitrate_kbps) + pad_up_to_bitrate_kbps = bitrate_kbps; + + paced_sender_->UpdateBitrate(kPaceMultiplier * bitrate_kbps, + pad_up_to_bitrate_kbps); default_rtp_rtcp_->SetTargetSendBitrate(stream_bitrates); if (video_suspended_ == video_is_suspended) return; video_suspended_ = video_is_suspended; } - // State changed, inform codec observer. + + // Video suspend-state changed, inform codec observer. CriticalSectionScoped crit(callback_cs_.get()); if (codec_observer_) { WEBRTC_TRACE(webrtc::kTraceInfo, webrtc::kTraceVideo, diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h index 3fd6ef7d3..4aa04193d 100644 --- a/webrtc/video_send_stream.h +++ b/webrtc/video_send_stream.h @@ -86,7 +86,7 @@ class VideoSendStream { struct Rtp { Rtp() : max_packet_size(kDefaultMaxPacketSize), - min_transmit_bitrate_kbps(0) {} + min_transmit_bitrate_bps(0) {} std::vector ssrcs; @@ -96,7 +96,7 @@ class VideoSendStream { // Padding will be used up to this bitrate regardless of the bitrate // produced by the encoder. Padding above what's actually produced by the // encoder helps maintaining a higher bitrate estimate. - int min_transmit_bitrate_kbps; + int min_transmit_bitrate_bps; // RTP header extensions to use for this send stream. std::vector extensions;