From cedd351e738e0fa81a9141c8e094d05ac0645896 Mon Sep 17 00:00:00 2001 From: Alex Narest Date: Thu, 7 Dec 2017 20:54:55 +0100 Subject: [PATCH] Do not add audio bitrate observer if TWCC sending is not supported Bug: webrtc:8243 Change-Id: Ida076dca72a6894053bdd0884f818ab3eaf5128a Reviewed-on: https://webrtc-review.googlesource.com/30840 Commit-Queue: Alex Narest Reviewed-by: Stefan Holmer Cr-Commit-Position: refs/heads/master@{#21149} --- audio/BUILD.gn | 1 + audio/audio_send_stream.cc | 54 ++++++++++++++++++++------------------ audio/audio_send_stream.h | 10 +++++++ 3 files changed, 40 insertions(+), 25 deletions(-) diff --git a/audio/BUILD.gn b/audio/BUILD.gn index 5a63a6f90c..ef132cc55d 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn @@ -58,6 +58,7 @@ rtc_static_library("audio") { "../rtc_base:rtc_base_approved", "../rtc_base:rtc_task_queue", "../system_wrappers", + "../system_wrappers:field_trial_api", "../voice_engine", ] } diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 2a5628cfa9..1596c96085 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -28,6 +28,7 @@ #include "rtc_base/logging.h" #include "rtc_base/task_queue.h" #include "rtc_base/timeutils.h" +#include "system_wrappers/include/field_trial.h" #include "voice_engine/channel_proxy.h" #include "voice_engine/include/voe_base.h" #include "voice_engine/transmit_mixer.h" @@ -137,6 +138,19 @@ void AudioSendStream::Reconfigure( ConfigureStream(this, new_config, false); } +AudioSendStream::ExtensionIds AudioSendStream::FindExtensionIds( + const std::vector& extensions) { + ExtensionIds ids; + for (const auto& extension : extensions) { + if (extension.uri == RtpExtension::kAudioLevelUri) { + ids.audio_level = extension.id; + } else if (extension.uri == RtpExtension::kTransportSequenceNumberUri) { + ids.transport_sequence_number = extension.id; + } + } + return ids; +} + void AudioSendStream::ConfigureStream( webrtc::internal::AudioSendStream* stream, const webrtc::AudioSendStream::Config& new_config, @@ -177,28 +191,8 @@ void AudioSendStream::ConfigureStream( stream->timed_send_transport_adapter_.get()); } - // RFC 5285: Each distinct extension MUST have a unique ID. The value 0 is - // reserved for padding and MUST NOT be used as a local identifier. - // So it should be safe to use 0 here to indicate "not configured". - struct ExtensionIds { - int audio_level = 0; - int transport_sequence_number = 0; - }; - - auto find_extension_ids = [](const std::vector& extensions) { - ExtensionIds ids; - for (const auto& extension : extensions) { - if (extension.uri == RtpExtension::kAudioLevelUri) { - ids.audio_level = extension.id; - } else if (extension.uri == RtpExtension::kTransportSequenceNumberUri) { - ids.transport_sequence_number = extension.id; - } - } - return ids; - }; - - const ExtensionIds old_ids = find_extension_ids(old_config.rtp.extensions); - const ExtensionIds new_ids = find_extension_ids(new_config.rtp.extensions); + const ExtensionIds old_ids = FindExtensionIds(old_config.rtp.extensions); + const ExtensionIds new_ids = FindExtensionIds(new_config.rtp.extensions); // Audio level indication if (first_time || new_ids.audio_level != old_ids.audio_level) { channel_proxy->SetSendAudioLevelIndicationStatus(new_ids.audio_level != 0, @@ -238,7 +232,10 @@ void AudioSendStream::ConfigureStream( void AudioSendStream::Start() { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); - if (config_.min_bitrate_bps != -1 && config_.max_bitrate_bps != -1) { + if (config_.min_bitrate_bps != -1 && config_.max_bitrate_bps != -1 && + (FindExtensionIds(config_.rtp.extensions).transport_sequence_number != + 0 || + !webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe"))) { // Audio BWE is enabled. transport_->packet_sender()->SetAccountForAudioPackets(true); ConfigureBitrateObserver(config_.min_bitrate_bps, config_.max_bitrate_bps); @@ -599,12 +596,19 @@ void AudioSendStream::ReconfigureBitrateObserver( // allow us to configure the bitrate observer if the new config has bitrate // limits set, but would only have us call RemoveBitrateObserver if we were // previously configured with bitrate limits. + int new_transport_seq_num_id = + FindExtensionIds(new_config.rtp.extensions).transport_sequence_number; if (stream->config_.min_bitrate_bps == new_config.min_bitrate_bps && - stream->config_.max_bitrate_bps == new_config.max_bitrate_bps) { + stream->config_.max_bitrate_bps == new_config.max_bitrate_bps && + (FindExtensionIds(stream->config_.rtp.extensions) + .transport_sequence_number == new_transport_seq_num_id || + !webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe"))) { return; } - if (new_config.min_bitrate_bps != -1 && new_config.max_bitrate_bps != -1) { + if (new_config.min_bitrate_bps != -1 && new_config.max_bitrate_bps != -1 && + (new_transport_seq_num_id != 0 || + !webrtc::field_trial::IsEnabled("WebRTC-Audio-SendSideBwe"))) { stream->ConfigureBitrateObserver(new_config.min_bitrate_bps, new_config.max_bitrate_bps); } else { diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index 08bdddb203..1414e39c64 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -124,6 +124,16 @@ class AudioSendStream final : public webrtc::AudioSendStream, std::unique_ptr timed_send_transport_adapter_; TimeInterval active_lifetime_; + // RFC 5285: Each distinct extension MUST have a unique ID. The value 0 is + // reserved for padding and MUST NOT be used as a local identifier. + // So it should be safe to use 0 here to indicate "not configured". + struct ExtensionIds { + int audio_level = 0; + int transport_sequence_number = 0; + }; + static ExtensionIds FindExtensionIds( + const std::vector& extensions); + RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(AudioSendStream); }; } // namespace internal