Reconfigure, not reconstruct, AudioReceiveStreams.
In preparation of moving ownership of voe::Channel to the audio stream
classes, semantics for changing configuration properties on the receive
streams need to change, otherwise RTP, audio decoding and NetEq state
will be discarded when streams are recreated. The same pattern as for
AudioSendStream is applied, and the reconfigurable information is kept
to a minimum.
AudioReceiveStream:s may still be recreated when an unsignaled stream
is 'promoted' to signaled state, and the sync label changes at the
same time.
Bug: webrtc:4690
Change-Id: Ibad282965310c3c8174a91e05a659fa3e1827607
Reviewed-on: https://webrtc-review.googlesource.com/38300
Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
Commit-Queue: Fredrik Solenberg <solenberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21560}diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc
index 74f1773..3c53818 100644
--- a/audio/audio_receive_stream.cc
+++ b/audio/audio_receive_stream.cc
@@ -68,22 +68,16 @@
const webrtc::AudioReceiveStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
webrtc::RtcEventLog* event_log)
- : config_(config), audio_state_(audio_state) {
- RTC_LOG(LS_INFO) << "AudioReceiveStream: " << config_.ToString();
- RTC_DCHECK_NE(config_.voe_channel_id, -1);
+ : audio_state_(audio_state) {
+ RTC_LOG(LS_INFO) << "AudioReceiveStream: " << config.ToString();
+ RTC_DCHECK_NE(config.voe_channel_id, -1);
RTC_DCHECK(audio_state_.get());
RTC_DCHECK(packet_router);
module_process_thread_checker_.DetachFromThread();
VoiceEngineImpl* voe_impl = static_cast<VoiceEngineImpl*>(voice_engine());
- channel_proxy_ = voe_impl->GetChannelProxy(config_.voe_channel_id);
- channel_proxy_->SetRtcEventLog(event_log);
- channel_proxy_->SetLocalSSRC(config.rtp.local_ssrc);
- // TODO(solenberg): Config NACK history window (which is a packet count),
- // using the actual packet size for the configured codec.
- channel_proxy_->SetNACKStatus(config_.rtp.nack.rtp_history_ms != 0,
- config_.rtp.nack.rtp_history_ms / 20);
+ channel_proxy_ = voe_impl->GetChannelProxy(config.voe_channel_id);
// TODO(ossu): This is where we'd like to set the decoder factory to
// use. However, since it needs to be included when constructing Channel, we
@@ -94,25 +88,18 @@
RTC_CHECK_EQ(config.decoder_factory,
channel_proxy_->GetAudioDecoderFactory());
+ channel_proxy_->SetRtcEventLog(event_log);
channel_proxy_->RegisterTransport(config.rtcp_send_transport);
- channel_proxy_->SetReceiveCodecs(config.decoder_map);
- for (const auto& extension : config.rtp.extensions) {
- if (extension.uri == RtpExtension::kAudioLevelUri) {
- channel_proxy_->SetReceiveAudioLevelIndicationStatus(true, extension.id);
- } else if (extension.uri == RtpExtension::kTransportSequenceNumberUri) {
- channel_proxy_->EnableReceiveTransportSequenceNumber(extension.id);
- } else {
- RTC_NOTREACHED() << "Unsupported RTP extension.";
- }
- }
// Configure bandwidth estimation.
channel_proxy_->RegisterReceiverCongestionControlObjects(packet_router);
// Register with transport.
rtp_stream_receiver_ =
- receiver_controller->CreateReceiver(config_.rtp.remote_ssrc,
+ receiver_controller->CreateReceiver(config.rtp.remote_ssrc,
channel_proxy_.get());
+
+ ConfigureStream(this, config, true);
}
AudioReceiveStream::~AudioReceiveStream() {
@@ -125,6 +112,13 @@
channel_proxy_->SetRtcEventLog(nullptr);
}
+void AudioReceiveStream::Reconfigure(
+ const webrtc::AudioReceiveStream::Config& config) {
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+ RTC_LOG(LS_INFO) << "AudioReceiveStream::Reconfigure: " << config_.ToString();
+ ConfigureStream(this, config, false);
+}
+
void AudioReceiveStream::Start() {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
if (playing_) {
@@ -329,5 +323,67 @@
RTC_DCHECK(audio_state);
return audio_state;
}
+
+AudioReceiveStream::ExtensionIds AudioReceiveStream::FindExtensionIds(
+ const std::vector<RtpExtension>& 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 AudioReceiveStream::ConfigureStream(AudioReceiveStream* stream,
+ const Config& new_config,
+ bool first_time) {
+ RTC_DCHECK(stream);
+ const auto& channel_proxy = stream->channel_proxy_;
+ const auto& old_config = stream->config_;
+
+ // Configuration parameters which cannot be changed.
+ RTC_DCHECK(first_time ||
+ old_config.voe_channel_id == new_config.voe_channel_id);
+ RTC_DCHECK(first_time ||
+ old_config.rtp.remote_ssrc == new_config.rtp.remote_ssrc);
+ RTC_DCHECK(first_time ||
+ old_config.rtcp_send_transport == new_config.rtcp_send_transport);
+ // Decoder factory cannot be changed because it is configured at
+ // voe::Channel construction time.
+ RTC_DCHECK(first_time ||
+ old_config.decoder_factory == new_config.decoder_factory);
+
+ if (first_time || old_config.rtp.local_ssrc != new_config.rtp.local_ssrc) {
+ channel_proxy->SetLocalSSRC(new_config.rtp.local_ssrc);
+ }
+ // TODO(solenberg): Config NACK history window (which is a packet count),
+ // using the actual packet size for the configured codec.
+ if (first_time || old_config.rtp.nack.rtp_history_ms !=
+ new_config.rtp.nack.rtp_history_ms) {
+ channel_proxy->SetNACKStatus(new_config.rtp.nack.rtp_history_ms != 0,
+ new_config.rtp.nack.rtp_history_ms / 20);
+ }
+ if (first_time || old_config.decoder_map != new_config.decoder_map) {
+ channel_proxy->SetReceiveCodecs(new_config.decoder_map);
+ }
+
+ // RTP Header Extensions.
+ const ExtensionIds old_ids = FindExtensionIds(old_config.rtp.extensions);
+ const ExtensionIds new_ids = FindExtensionIds(new_config.rtp.extensions);
+ if (first_time || new_ids.audio_level != old_ids.audio_level) {
+ channel_proxy->SetReceiveAudioLevelIndicationStatus(
+ new_ids.audio_level != 0, new_ids.audio_level);
+ }
+ if (first_time || new_ids.transport_sequence_number !=
+ old_ids.transport_sequence_number) {
+ channel_proxy->EnableReceiveTransportSequenceNumber(
+ new_ids.transport_sequence_number);
+ }
+
+ stream->config_ = new_config;
+}
} // namespace internal
} // namespace webrtc
diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h
index 55e58d7..0c843be 100644
--- a/audio/audio_receive_stream.h
+++ b/audio/audio_receive_stream.h
@@ -48,6 +48,7 @@
~AudioReceiveStream() override;
// webrtc::AudioReceiveStream implementation.
+ void Reconfigure(const webrtc::AudioReceiveStream::Config& config) override;
void Start() override;
void Stop() override;
webrtc::AudioReceiveStream::Stats GetStats() const override;
@@ -80,12 +81,25 @@
const webrtc::AudioReceiveStream::Config& config() const;
private:
+ // 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<RtpExtension>& extensions);
+ static void ConfigureStream(AudioReceiveStream* stream,
+ const Config& new_config,
+ bool first_time);
+
VoiceEngine* voice_engine() const;
AudioState* audio_state() const;
rtc::ThreadChecker worker_thread_checker_;
rtc::ThreadChecker module_process_thread_checker_;
- const webrtc::AudioReceiveStream::Config config_;
+ webrtc::AudioReceiveStream::Config config_;
rtc::scoped_refptr<webrtc::AudioState> audio_state_;
std::unique_ptr<voe::ChannelProxy> channel_proxy_;
diff --git a/audio/audio_receive_stream_unittest.cc b/audio/audio_receive_stream_unittest.cc
index 4d4af2e..e03fbd5 100644
--- a/audio/audio_receive_stream_unittest.cc
+++ b/audio/audio_receive_stream_unittest.cc
@@ -394,5 +394,46 @@
// Stop stream before it is being destructed.
recv_stream2.Stop();
}
+
+TEST(AudioReceiveStreamTest, ReconfigureWithSameConfig) {
+ ConfigHelper helper;
+ internal::AudioReceiveStream recv_stream(
+ helper.rtp_stream_receiver_controller(),
+ helper.packet_router(),
+ helper.config(), helper.audio_state(), helper.event_log());
+ recv_stream.Reconfigure(helper.config());
+}
+
+TEST(AudioReceiveStreamTest, ReconfigureWithUpdatedConfig) {
+ ConfigHelper helper;
+ internal::AudioReceiveStream recv_stream(
+ helper.rtp_stream_receiver_controller(),
+ helper.packet_router(),
+ helper.config(), helper.audio_state(), helper.event_log());
+
+ auto new_config = helper.config();
+ new_config.rtp.local_ssrc = kLocalSsrc + 1;
+ new_config.rtp.nack.rtp_history_ms = 300 + 20;
+ new_config.rtp.extensions.clear();
+ new_config.rtp.extensions.push_back(
+ RtpExtension(RtpExtension::kAudioLevelUri, kAudioLevelId + 1));
+ new_config.rtp.extensions.push_back(RtpExtension(
+ RtpExtension::kTransportSequenceNumberUri,
+ kTransportSequenceNumberId + 1));
+ new_config.decoder_map.emplace(1, SdpAudioFormat("foo", 8000, 1));
+
+ MockVoEChannelProxy& channel_proxy = *helper.channel_proxy();
+ EXPECT_CALL(channel_proxy, SetLocalSSRC(kLocalSsrc + 1)).Times(1);
+ EXPECT_CALL(channel_proxy, SetNACKStatus(true, 15 + 1)).Times(1);
+ EXPECT_CALL(channel_proxy, SetReceiveCodecs(new_config.decoder_map));
+ EXPECT_CALL(channel_proxy,
+ SetReceiveAudioLevelIndicationStatus(true, kAudioLevelId + 1))
+ .Times(1);
+ EXPECT_CALL(channel_proxy,
+ EnableReceiveTransportSequenceNumber(kTransportSequenceNumberId + 1))
+ .Times(1);
+
+ recv_stream.Reconfigure(new_config);
+}
} // namespace test
} // namespace webrtc
diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h
index 44f093c..505fd08 100644
--- a/call/audio_receive_stream.h
+++ b/call/audio_receive_stream.h
@@ -120,6 +120,9 @@
rtc::scoped_refptr<AudioDecoderFactory> decoder_factory;
};
+ // Reconfigure the stream according to the Configuration.
+ virtual void Reconfigure(const Config& config) = 0;
+
// Starts stream activity.
// When a stream is active, it can receive, process and deliver packets.
virtual void Start() = 0;
diff --git a/media/engine/fakewebrtccall.cc b/media/engine/fakewebrtccall.cc
index c23c299..edc6d8b 100644
--- a/media/engine/fakewebrtccall.cc
+++ b/media/engine/fakewebrtccall.cc
@@ -98,6 +98,11 @@
return true;
}
+void FakeAudioReceiveStream::Reconfigure(
+ const webrtc::AudioReceiveStream::Config& config) {
+ config_ = config;
+}
+
webrtc::AudioReceiveStream::Stats FakeAudioReceiveStream::GetStats() const {
return stats_;
}
diff --git a/media/engine/fakewebrtccall.h b/media/engine/fakewebrtccall.h
index 6fa71db..c355055 100644
--- a/media/engine/fakewebrtccall.h
+++ b/media/engine/fakewebrtccall.h
@@ -57,7 +57,6 @@
private:
// webrtc::AudioSendStream implementation.
void Reconfigure(const webrtc::AudioSendStream::Config& config) override;
-
void Start() override { sending_ = true; }
void Stop() override { sending_ = false; }
void SendAudioData(std::unique_ptr<webrtc::AudioFrame> audio_frame) override {
@@ -96,6 +95,7 @@
private:
// webrtc::AudioReceiveStream implementation.
+ void Reconfigure(const webrtc::AudioReceiveStream::Config& config) override;
void Start() override { started_ = true; }
void Stop() override { started_ = false; }
diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc
index 438a42f..d8fd1e7 100644
--- a/media/engine/webrtcvoiceengine.cc
+++ b/media/engine/webrtcvoiceengine.cc
@@ -1185,37 +1185,38 @@
call_->DestroyAudioReceiveStream(stream_);
}
- void RecreateAudioReceiveStream(uint32_t local_ssrc) {
+ void SetLocalSsrc(uint32_t local_ssrc) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
config_.rtp.local_ssrc = local_ssrc;
- RecreateAudioReceiveStream();
+ ReconfigureAudioReceiveStream();
}
- void RecreateAudioReceiveStream(bool use_transport_cc, bool use_nack) {
+ void SetUseTransportCc(bool use_transport_cc, bool use_nack) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
config_.rtp.transport_cc = use_transport_cc;
config_.rtp.nack.rtp_history_ms = use_nack ? kNackRtpHistoryMs : 0;
- RecreateAudioReceiveStream();
+ ReconfigureAudioReceiveStream();
}
- void RecreateAudioReceiveStream(
- const std::vector<webrtc::RtpExtension>& extensions) {
+ void SetRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
config_.rtp.extensions = extensions;
- RecreateAudioReceiveStream();
+ ReconfigureAudioReceiveStream();
}
// Set a new payload type -> decoder map.
- void RecreateAudioReceiveStream(
- const std::map<int, webrtc::SdpAudioFormat>& decoder_map) {
+ void SetDecoderMap(const std::map<int, webrtc::SdpAudioFormat>& decoder_map) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
config_.decoder_map = decoder_map;
- RecreateAudioReceiveStream();
+ ReconfigureAudioReceiveStream();
}
void MaybeRecreateAudioReceiveStream(const std::string& sync_group) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
if (config_.sync_group != sync_group) {
+ RTC_LOG(LS_INFO) << "Recreating AudioReceiveStream for SSRC="
+ << config_.rtp.remote_ssrc
+ << " because of sync group change.";
config_.sync_group = sync_group;
RecreateAudioReceiveStream();
}
@@ -1278,6 +1279,12 @@
SetPlayout(playout_);
}
+ void ReconfigureAudioReceiveStream() {
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(stream_);
+ stream_->Reconfigure(config_);
+ }
+
rtc::ThreadChecker worker_thread_checker_;
webrtc::Call* call_ = nullptr;
webrtc::AudioReceiveStream::Config config_;
@@ -1373,7 +1380,7 @@
if (recv_rtp_extensions_ != filtered_extensions) {
recv_rtp_extensions_.swap(filtered_extensions);
for (auto& it : recv_streams_) {
- it.second->RecreateAudioReceiveStream(recv_rtp_extensions_);
+ it.second->SetRtpExtensions(recv_rtp_extensions_);
}
}
return true;
@@ -1588,7 +1595,7 @@
decoder_map_ = std::move(decoder_map);
for (auto& kv : recv_streams_) {
- kv.second->RecreateAudioReceiveStream(decoder_map_);
+ kv.second->SetDecoderMap(decoder_map_);
}
recv_codecs_ = codecs;
@@ -1720,8 +1727,8 @@
recv_transport_cc_enabled_ = send_codec_spec_->transport_cc_enabled;
recv_nack_enabled_ = send_codec_spec_->nack_enabled;
for (auto& kv : recv_streams_) {
- kv.second->RecreateAudioReceiveStream(recv_transport_cc_enabled_,
- recv_nack_enabled_);
+ kv.second->SetUseTransportCc(recv_transport_cc_enabled_,
+ recv_nack_enabled_);
}
}
@@ -1846,8 +1853,8 @@
receiver_reports_ssrc_ = ssrc;
for (const auto& kv : recv_streams_) {
// TODO(solenberg): Allow applications to set the RTCP SSRC of receive
- // streams instead, so we can avoid recreating the streams here.
- kv.second->RecreateAudioReceiveStream(ssrc);
+ // streams instead, so we can avoid reconfiguring the streams here.
+ kv.second->SetLocalSsrc(ssrc);
}
}
diff --git a/voice_engine/channel.cc b/voice_engine/channel.cc
index de2d4a7..c77b38b 100644
--- a/voice_engine/channel.cc
+++ b/voice_engine/channel.cc
@@ -1168,9 +1168,11 @@
void Channel::EnableReceiveTransportSequenceNumber(int id) {
rtp_header_parser_->DeregisterRtpHeaderExtension(
kRtpExtensionTransportSequenceNumber);
- bool ret = rtp_header_parser_->RegisterRtpHeaderExtension(
- kRtpExtensionTransportSequenceNumber, id);
- RTC_DCHECK(ret);
+ if (id != 0) {
+ bool ret = rtp_header_parser_->RegisterRtpHeaderExtension(
+ kRtpExtensionTransportSequenceNumber, id);
+ RTC_DCHECK(ret);
+ }
}
void Channel::RegisterSenderCongestionControlObjects(