From 86cc6ffc7ca14c79d760695e89ab9698ed251873 Mon Sep 17 00:00:00 2001 From: mflodman Date: Tue, 26 Jul 2016 04:44:06 -0700 Subject: [PATCH] Variable audio bitrate. This is a first CL wiring up AudioSendStream to BitrateAllocator. This is still experimental and there is a test added for the audio only case, combined audio video variable bitrate test cases will be added as a follow up. BUG=5079 Review-Url: https://codereview.webrtc.org/2165743003 Cr-Commit-Position: refs/heads/master@{#13527} --- webrtc/audio/DEPS | 2 ++ webrtc/audio/audio_send_stream.cc | 32 ++++++++++++++++++++-- webrtc/audio/audio_send_stream.h | 15 ++++++++-- webrtc/audio/audio_send_stream_unittest.cc | 28 +++++++++++++++---- webrtc/audio_send_stream.h | 6 ++++ webrtc/call/call.cc | 3 +- webrtc/call/rampup_tests.cc | 31 +++++++++++++++++---- webrtc/call/rampup_tests.h | 2 +- webrtc/media/engine/webrtcvoiceengine.cc | 8 ++++++ webrtc/test/call_test.cc | 2 +- webrtc/test/mock_voe_channel_proxy.h | 1 + webrtc/voice_engine/channel_proxy.cc | 5 ++++ webrtc/voice_engine/channel_proxy.h | 1 + 13 files changed, 118 insertions(+), 18 deletions(-) diff --git a/webrtc/audio/DEPS b/webrtc/audio/DEPS index 82c3165366..5e43c34821 100644 --- a/webrtc/audio/DEPS +++ b/webrtc/audio/DEPS @@ -2,12 +2,14 @@ include_rules = [ "+webrtc/base", "+webrtc/voice_engine", "+webrtc/modules/audio_coding/codecs/mock", + "+webrtc/call", "+webrtc/modules/bitrate_controller", "+webrtc/modules/congestion_controller", "+webrtc/modules/pacing", "+webrtc/modules/remote_bitrate_estimator", "+webrtc/modules/rtp_rtcp", "+webrtc/system_wrappers", + "+webrtc/voice_engine", ] specific_include_rules = { diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index d08bfeaa1e..17979d5760 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -59,8 +59,11 @@ namespace internal { AudioSendStream::AudioSendStream( const webrtc::AudioSendStream::Config& config, const rtc::scoped_refptr& audio_state, - CongestionController* congestion_controller) - : config_(config), audio_state_(audio_state) { + CongestionController* congestion_controller, + BitrateAllocator* bitrate_allocator) + : config_(config), + audio_state_(audio_state), + bitrate_allocator_(bitrate_allocator) { LOG(LS_INFO) << "AudioSendStream: " << config_.ToString(); RTC_DCHECK_NE(config_.voe_channel_id, -1); RTC_DCHECK(audio_state_.get()); @@ -104,6 +107,12 @@ AudioSendStream::~AudioSendStream() { void AudioSendStream::Start() { RTC_DCHECK(thread_checker_.CalledOnValidThread()); + if (config_.min_bitrate_kbps != -1 && config_.max_bitrate_kbps != -1) { + RTC_DCHECK_GE(config_.max_bitrate_kbps, config_.min_bitrate_kbps); + bitrate_allocator_->AddObserver(this, config_.min_bitrate_kbps * 1000, + config_.max_bitrate_kbps * 1000, 0, true); + } + ScopedVoEInterface base(voice_engine()); int error = base->StartSend(config_.voe_channel_id); if (error != 0) { @@ -113,6 +122,7 @@ void AudioSendStream::Start() { void AudioSendStream::Stop() { RTC_DCHECK(thread_checker_.CalledOnValidThread()); + bitrate_allocator_->RemoveObserver(this); ScopedVoEInterface base(voice_engine()); int error = base->StopSend(config_.voe_channel_id); if (error != 0) { @@ -227,6 +237,24 @@ bool AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { return channel_proxy_->ReceivedRTCPPacket(packet, length); } +uint32_t AudioSendStream::OnBitrateUpdated(uint32_t bitrate_bps, + uint8_t fraction_loss, + int64_t rtt) { + RTC_DCHECK_GE(bitrate_bps, + static_cast(config_.min_bitrate_kbps * 1000)); + // The bitrate allocator might allocate an higher than max configured bitrate + // if there is room, to allow for, as example, extra FEC. Ignore that for now. + const uint32_t max_bitrate_bps = config_.max_bitrate_kbps * 1000; + if (bitrate_bps > max_bitrate_bps) + bitrate_bps = max_bitrate_bps; + + channel_proxy_->SetBitrate(bitrate_bps); + + // The amount of audio protection is not exposed by the encoder, hence + // always returning 0. + return 0; +} + const webrtc::AudioSendStream::Config& AudioSendStream::config() const { RTC_DCHECK(thread_checker_.CalledOnValidThread()); return config_; diff --git a/webrtc/audio/audio_send_stream.h b/webrtc/audio/audio_send_stream.h index 264f9b3a69..a993d5f2f9 100644 --- a/webrtc/audio/audio_send_stream.h +++ b/webrtc/audio/audio_send_stream.h @@ -17,6 +17,7 @@ #include "webrtc/audio_state.h" #include "webrtc/base/constructormagic.h" #include "webrtc/base/thread_checker.h" +#include "webrtc/call/bitrate_allocator.h" namespace webrtc { class CongestionController; @@ -27,11 +28,13 @@ class ChannelProxy; } // namespace voe namespace internal { -class AudioSendStream final : public webrtc::AudioSendStream { +class AudioSendStream final : public webrtc::AudioSendStream, + public webrtc::BitrateAllocatorObserver { public: AudioSendStream(const webrtc::AudioSendStream::Config& config, const rtc::scoped_refptr& audio_state, - CongestionController* congestion_controller); + CongestionController* congestion_controller, + BitrateAllocator* bitrate_allocator); ~AudioSendStream() override; // webrtc::AudioSendStream implementation. @@ -44,6 +47,12 @@ class AudioSendStream final : public webrtc::AudioSendStream { void SignalNetworkState(NetworkState state); bool DeliverRtcp(const uint8_t* packet, size_t length); + + // Implements BitrateAllocatorObserver. + uint32_t OnBitrateUpdated(uint32_t bitrate_bps, + uint8_t fraction_loss, + int64_t rtt) override; + const webrtc::AudioSendStream::Config& config() const; private: @@ -54,6 +63,8 @@ class AudioSendStream final : public webrtc::AudioSendStream { rtc::scoped_refptr audio_state_; std::unique_ptr channel_proxy_; + BitrateAllocator* const bitrate_allocator_; + RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(AudioSendStream); }; } // namespace internal diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index ea70d304d7..7f940fc767 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -50,6 +50,13 @@ const int kTelephoneEventPayloadType = 123; const int kTelephoneEventCode = 45; const int kTelephoneEventDuration = 6789; +class MockLimitObserver : public BitrateAllocator::LimitObserver { + public: + MOCK_METHOD2(OnAllocationLimitsChanged, + void(uint32_t min_send_bitrate_bps, + uint32_t max_padding_bitrate_bps)); +}; + struct ConfigHelper { ConfigHelper() : simulated_clock_(123456), @@ -57,7 +64,8 @@ struct ConfigHelper { congestion_controller_(&simulated_clock_, &bitrate_observer_, &remote_bitrate_observer_, - &event_log_) { + &event_log_), + bitrate_allocator_(&limit_observer_) { using testing::Invoke; using testing::StrEq; @@ -116,6 +124,7 @@ struct ConfigHelper { CongestionController* congestion_controller() { return &congestion_controller_; } + BitrateAllocator* bitrate_allocator() { return &bitrate_allocator_; } void SetupMockForSendTelephoneEvent() { EXPECT_TRUE(channel_proxy_); @@ -170,6 +179,8 @@ struct ConfigHelper { testing::NiceMock remote_bitrate_observer_; CongestionController congestion_controller_; MockRtcEventLog event_log_; + testing::NiceMock limit_observer_; + BitrateAllocator bitrate_allocator_; }; } // namespace @@ -192,13 +203,15 @@ TEST(AudioSendStreamTest, ConfigToString) { TEST(AudioSendStreamTest, ConstructDestruct) { ConfigHelper helper; internal::AudioSendStream send_stream(helper.config(), helper.audio_state(), - helper.congestion_controller()); + helper.congestion_controller(), + helper.bitrate_allocator()); } TEST(AudioSendStreamTest, SendTelephoneEvent) { ConfigHelper helper; internal::AudioSendStream send_stream(helper.config(), helper.audio_state(), - helper.congestion_controller()); + helper.congestion_controller(), + helper.bitrate_allocator()); helper.SetupMockForSendTelephoneEvent(); EXPECT_TRUE(send_stream.SendTelephoneEvent(kTelephoneEventPayloadType, kTelephoneEventCode, kTelephoneEventDuration)); @@ -207,7 +220,8 @@ TEST(AudioSendStreamTest, SendTelephoneEvent) { TEST(AudioSendStreamTest, SetMuted) { ConfigHelper helper; internal::AudioSendStream send_stream(helper.config(), helper.audio_state(), - helper.congestion_controller()); + helper.congestion_controller(), + helper.bitrate_allocator()); EXPECT_CALL(*helper.channel_proxy(), SetInputMute(true)); send_stream.SetMuted(true); } @@ -215,7 +229,8 @@ TEST(AudioSendStreamTest, SetMuted) { TEST(AudioSendStreamTest, GetStats) { ConfigHelper helper; internal::AudioSendStream send_stream(helper.config(), helper.audio_state(), - helper.congestion_controller()); + helper.congestion_controller(), + helper.bitrate_allocator()); helper.SetupMockForGetStats(); AudioSendStream::Stats stats = send_stream.GetStats(); EXPECT_EQ(kSsrc, stats.local_ssrc); @@ -243,7 +258,8 @@ TEST(AudioSendStreamTest, GetStats) { TEST(AudioSendStreamTest, GetStatsTypingNoiseDetected) { ConfigHelper helper; internal::AudioSendStream send_stream(helper.config(), helper.audio_state(), - helper.congestion_controller()); + helper.congestion_controller(), + helper.bitrate_allocator()); helper.SetupMockForGetStats(); EXPECT_FALSE(send_stream.GetStats().typing_noise_detected); diff --git a/webrtc/audio_send_stream.h b/webrtc/audio_send_stream.h index 1c6803ca99..c3d0d339de 100644 --- a/webrtc/audio_send_stream.h +++ b/webrtc/audio_send_stream.h @@ -88,6 +88,12 @@ class AudioSendStream { // TODO(solenberg): Implement, once we configure codecs through the new API. // std::unique_ptr encoder; int cng_payload_type = -1; // pt, or -1 to disable Comfort Noise Generator. + + // Bitrate limits used for variable audio bitrate streams. Set both to -1 to + // disable audio bitrate adaptation. + // Note: This is still an experimental feature and not ready for real usage. + int min_bitrate_kbps = -1; + int max_bitrate_kbps = -1; }; // Starts stream activity. diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index fa2d1e7e78..6df86e3c1a 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -348,7 +348,8 @@ webrtc::AudioSendStream* Call::CreateAudioSendStream( TRACE_EVENT0("webrtc", "Call::CreateAudioSendStream"); RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); AudioSendStream* send_stream = new AudioSendStream( - config, config_.audio_state, congestion_controller_.get()); + config, config_.audio_state, congestion_controller_.get(), + bitrate_allocator_.get()); { WriteLockScoped write_lock(*send_crit_); RTC_DCHECK(audio_send_ssrcs_.find(config.rtp.ssrc) == diff --git a/webrtc/call/rampup_tests.cc b/webrtc/call/rampup_tests.cc index 150fc770ff..320d22de64 100644 --- a/webrtc/call/rampup_tests.cc +++ b/webrtc/call/rampup_tests.cc @@ -41,6 +41,7 @@ RampUpTester::RampUpTester(size_t num_video_streams, num_audio_streams_(num_audio_streams), rtx_(rtx), red_(red), + sender_call_(nullptr), send_stream_(nullptr), start_bitrate_bps_(start_bitrate_bps), start_bitrate_verified_(false), @@ -53,8 +54,7 @@ RampUpTester::RampUpTester(size_t num_video_streams, audio_ssrcs_(GenerateSsrcs(num_audio_streams_, 300)), poller_thread_(&BitrateStatsPollingThread, this, - "BitrateStatsPollingThread"), - sender_call_(nullptr) { + "BitrateStatsPollingThread") { EXPECT_LE(num_audio_streams_, 1u); if (rtx_) { for (size_t i = 0; i < video_ssrcs_.size(); ++i) @@ -188,6 +188,9 @@ void RampUpTester::ModifyAudioConfigs( send_config->rtp.ssrc = audio_ssrcs_[0]; send_config->rtp.extensions.clear(); + send_config->min_bitrate_kbps = 6; + send_config->max_bitrate_kbps = 60; + bool transport_cc = false; if (extension_type_ == RtpExtension::kAbsSendTimeUri) { transport_cc = false; @@ -267,6 +270,9 @@ void RampUpTester::TriggerTestDone() { RTC_DCHECK_GE(test_start_ms_, 0); // TODO(holmer): Add audio send stats here too when those APIs are available. + if (!send_stream_) + return; + VideoSendStream::Stats send_stats = send_stream_->GetStats(); size_t total_packets_sent = 0; @@ -341,6 +347,11 @@ bool RampUpDownUpTester::PollStats() { transmit_bitrate_bps += it.second.total_bitrate_bps; } EvolveTestState(transmit_bitrate_bps, stats.suspended); + } else if (num_audio_streams_ > 0 && sender_call_ != nullptr) { + // An audio send stream doesn't have bitrate stats, so the call send BW is + // currently used instead. + int transmit_bitrate_bps = sender_call_->GetStats().send_bandwidth_bps; + EvolveTestState(transmit_bitrate_bps, false); } return !event_.Wait(kPollIntervalMs); @@ -380,7 +391,7 @@ void RampUpDownUpTester::EvolveTestState(int bitrate_bps, bool suspended) { switch (test_state_) { case kFirstRampup: { EXPECT_FALSE(suspended); - if (bitrate_bps > kExpectedHighBitrateBps) { + if (bitrate_bps >= kExpectedHighBitrateBps) { // The first ramp-up has reached the target bitrate. Change the // channel limit, and move to the next test state. forward_transport_config_.link_capacity_kbps = @@ -397,7 +408,10 @@ void RampUpDownUpTester::EvolveTestState(int bitrate_bps, bool suspended) { break; } case kLowRate: { - if (bitrate_bps < kExpectedLowBitrateBps && suspended) { + // Audio streams are never suspended. + bool check_suspend_state = num_video_streams_ > 0; + if (bitrate_bps < kExpectedLowBitrateBps && + suspended == check_suspend_state) { // The ramp-down was successful. Change the channel limit back to a // high value, and move to the next test state. forward_transport_config_.link_capacity_kbps = @@ -414,7 +428,7 @@ void RampUpDownUpTester::EvolveTestState(int bitrate_bps, bool suspended) { break; } case kSecondRampup: { - if (bitrate_bps > kExpectedHighBitrateBps && !suspended) { + if (bitrate_bps >= kExpectedHighBitrateBps && !suspended) { webrtc::test::PrintResult("ramp_up_down_up", GetModifierString(), "second_rampup", now - state_start_ms_, "ms", false); @@ -520,6 +534,13 @@ TEST_F(RampUpTest, DISABLED_SendSideAudioVideoUpDownUpRtx) { RunBaseTest(&test); } +TEST_F(RampUpTest, SendSideAudioOnlyUpDownUpRtx) { + RampUpDownUpTester test(0, 1, kStartBitrateBps, + RtpExtension::kTransportSequenceNumberUri, true, + false); + RunBaseTest(&test); +} + TEST_F(RampUpTest, AbsSendTimeSingleStream) { RampUpTester test(1, 0, 0, RtpExtension::kAbsSendTimeUri, false, false); RunBaseTest(&test); diff --git a/webrtc/call/rampup_tests.h b/webrtc/call/rampup_tests.h index fa73c63f9a..efc56ad263 100644 --- a/webrtc/call/rampup_tests.h +++ b/webrtc/call/rampup_tests.h @@ -64,6 +64,7 @@ class RampUpTester : public test::EndToEndTest { const size_t num_audio_streams_; const bool rtx_; const bool red_; + Call* sender_call_; VideoSendStream* send_stream_; test::PacketTransport* send_transport_; @@ -99,7 +100,6 @@ class RampUpTester : public test::EndToEndTest { SsrcMap rtx_ssrc_map_; rtc::PlatformThread poller_thread_; - Call* sender_call_; }; class RampUpDownUpTester : public RampUpTester { diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index 469098a322..2aa0552b93 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -1136,6 +1136,14 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream stream_ = nullptr; } config_.rtp.extensions = extensions; + if (webrtc::field_trial::FindFullName("WebRTC-AdaptAudioBitrate") == + "Enabled") { + // TODO(mflodman): Keep testing this and set proper values. + // Note: This is an early experiment currently only supported by Opus. + config_.min_bitrate_kbps = kOpusMinBitrate; + config_.max_bitrate_kbps = kOpusBitrateFb; + } + RTC_DCHECK(!stream_); stream_ = call_->CreateAudioSendStream(config_); RTC_CHECK(stream_); diff --git a/webrtc/test/call_test.cc b/webrtc/test/call_test.cc index 919ebe812b..590479f4d4 100644 --- a/webrtc/test/call_test.cc +++ b/webrtc/test/call_test.cc @@ -62,8 +62,8 @@ void CallTest::RunBaseTest(BaseTest* test) { CreateReceiverCall(recv_config); } test->OnCallsCreated(sender_call_.get(), receiver_call_.get()); - send_transport_.reset(test->CreateSendTransport(sender_call_.get())); receive_transport_.reset(test->CreateReceiveTransport()); + send_transport_.reset(test->CreateSendTransport(sender_call_.get())); if (test->ShouldCreateReceivers()) { send_transport_->SetReceiver(receiver_call_->Receiver()); diff --git a/webrtc/test/mock_voe_channel_proxy.h b/webrtc/test/mock_voe_channel_proxy.h index fef8b7d899..069b4f1c78 100644 --- a/webrtc/test/mock_voe_channel_proxy.h +++ b/webrtc/test/mock_voe_channel_proxy.h @@ -58,6 +58,7 @@ class MockVoEChannelProxy : public voe::ChannelProxy { const rtc::scoped_refptr&()); MOCK_METHOD1(SetChannelOutputVolumeScaling, void(float scaling)); MOCK_METHOD1(SetRtcEventLog, void(RtcEventLog* event_log)); + MOCK_METHOD1(SetBitrate, void(int bitrate_bps)); }; } // namespace test } // namespace webrtc diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc index 215d931f1a..8c942f3701 100644 --- a/webrtc/voice_engine/channel_proxy.cc +++ b/webrtc/voice_engine/channel_proxy.cc @@ -158,6 +158,11 @@ bool ChannelProxy::SendTelephoneEventOutband(int event, int duration_ms) { return channel()->SendTelephoneEventOutband(event, duration_ms) == 0; } +void ChannelProxy::SetBitrate(int bitrate_bps) { + // May be called on different threads and needs to be handled by the channel. + channel()->SetBitRate(bitrate_bps); +} + void ChannelProxy::SetSink(std::unique_ptr sink) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); channel()->SetSink(std::move(sink)); diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h index 0c26b00986..d84e73b60f 100644 --- a/webrtc/voice_engine/channel_proxy.h +++ b/webrtc/voice_engine/channel_proxy.h @@ -73,6 +73,7 @@ class ChannelProxy { virtual bool SetSendTelephoneEventPayloadType(int payload_type); virtual bool SendTelephoneEventOutband(int event, int duration_ms); + virtual void SetBitrate(int bitrate_bps); virtual void SetSink(std::unique_ptr sink); virtual void SetInputMute(bool muted);