Implement TargetBitrate, NetworkRoute and overhead features of media transport interface.
So far ANA was not available for media transport interface. With recent changes to media transport, we can now account for packet overhead, network route (ip/tcp/udp/turn overheads) and we can also use bandwidth estimate from the media transport.
Bug: webrtc:9719
Change-Id: I98c9a09dd418b763c339ee2ee05592e164cf9199
Reviewed-on: https://webrtc-review.googlesource.com/c/110367
Commit-Queue: Peter Slatala <psla@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Anton Sukhanov <sukhanov@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25677}diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc
index 9c100fb..e2ce753 100644
--- a/audio/channel_receive.cc
+++ b/audio/channel_receive.cc
@@ -1004,6 +1004,15 @@
}
int64_t ChannelReceive::GetRTT() const {
+ if (media_transport_) {
+ auto target_rate = media_transport_->GetLatestTargetTransferRate();
+ if (target_rate.has_value()) {
+ return target_rate->network_estimate.round_trip_time.ms();
+ }
+
+ return 0;
+ }
+
RtcpMode method = _rtpRtcpModule->RTCP();
if (method == RtcpMode::kOff) {
return 0;
diff --git a/audio/channel_send.cc b/audio/channel_send.cc
index a8b93cc..e96ef4a 100644
--- a/audio/channel_send.cc
+++ b/audio/channel_send.cc
@@ -479,15 +479,23 @@
audio_coding_.reset(AudioCodingModule::Create(AudioCodingModule::Config()));
RtpRtcp::Configuration configuration;
+
+ // We gradually remove codepaths that depend on RTP when using media
+ // transport. All of this logic should be moved to the future
+ // RTPMediaTransport. In this case it means that overhead and bandwidth
+ // observers should not be called when using media transport.
+ if (!media_transport_) {
+ configuration.overhead_observer = this;
+ configuration.bandwidth_callback = rtcp_observer_.get();
+ configuration.transport_feedback_callback = feedback_observer_proxy_.get();
+ }
+
configuration.audio = true;
configuration.outgoing_transport = this;
- configuration.overhead_observer = this;
- configuration.bandwidth_callback = rtcp_observer_.get();
configuration.paced_sender = rtp_packet_sender_proxy_.get();
configuration.transport_sequence_number_allocator =
seq_num_allocator_proxy_.get();
- configuration.transport_feedback_callback = feedback_observer_proxy_.get();
configuration.event_log = event_log_;
configuration.rtt_stats = rtcp_rtt_stats;
@@ -500,6 +508,16 @@
_rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration));
_rtpRtcpModule->SetSendingMediaStatus(false);
+ // We want to invoke the 'TargetRateObserver' and |OnOverheadChanged|
+ // callbacks after the audio_coding_ is fully initialized.
+ if (media_transport_) {
+ RTC_DLOG(LS_INFO) << "Setting media_transport_ rate observers.";
+ media_transport_->AddTargetTransferRateObserver(this);
+ OnOverheadChanged(media_transport_->GetAudioPacketOverhead());
+ } else {
+ RTC_DLOG(LS_INFO) << "Not setting media_transport_ rate observers.";
+ }
+
channel_state_.Reset();
_moduleProcessThreadPtr->RegisterModule(_rtpRtcpModule.get(), RTC_FROM_HERE);
@@ -522,6 +540,10 @@
ChannelSend::~ChannelSend() {
RTC_DCHECK(construction_thread_.CalledOnValidThread());
+ if (media_transport_) {
+ media_transport_->RemoveTargetTransferRateObserver(this);
+ }
+
StopSend();
int error = audio_coding_->RegisterTransportCallback(NULL);
@@ -693,6 +715,12 @@
// TODO(nisse): Delete always-true return value.
bool ChannelSend::ReceivedRTCPPacket(const uint8_t* data, size_t length) {
+ if (media_transport_) {
+ // Ignore RTCP packets while media transport is used.
+ // Those packets should not arrive, but we are seeing occasional packets.
+ return 0;
+ }
+
// Deliver RTCP packet to RTP/RTCP module for parsing
_rtpRtcpModule->IncomingRtcpPacket(data, length);
@@ -710,11 +738,7 @@
}
retransmission_rate_limiter_->SetWindowSize(nack_window_ms);
- // Invoke audio encoders OnReceivedRtt().
- audio_coding_->ModifyEncoder([&](std::unique_ptr<AudioEncoder>* encoder) {
- if (*encoder)
- (*encoder)->OnReceivedRtt(rtt);
- });
+ OnReceivedRtt(rtt);
return true;
}
@@ -1010,6 +1034,19 @@
}
int64_t ChannelSend::GetRTT() const {
+ if (media_transport_) {
+ // GetRTT is generally used in the RTCP codepath, where media transport is
+ // not present and so it shouldn't be needed. But it's also invoked in
+ // 'GetStats' method, and for now returning media transport RTT here gives
+ // us "free" rtt stats for media transport.
+ auto target_rate = media_transport_->GetLatestTargetTransferRate();
+ if (target_rate.has_value()) {
+ return target_rate.value().network_estimate.round_trip_time.ms();
+ }
+
+ return 0;
+ }
+
RtcpMode method = _rtpRtcpModule->RTCP();
if (method == RtcpMode::kOff) {
return 0;
@@ -1046,5 +1083,20 @@
}
}
+void ChannelSend::OnTargetTransferRate(TargetTransferRate rate) {
+ RTC_DCHECK(media_transport_);
+ OnReceivedRtt(rate.network_estimate.round_trip_time.ms());
+}
+
+void ChannelSend::OnReceivedRtt(int64_t rtt_ms) {
+ // Invoke audio encoders OnReceivedRtt().
+ audio_coding_->ModifyEncoder(
+ [rtt_ms](std::unique_ptr<AudioEncoder>* encoder) {
+ if (*encoder) {
+ (*encoder)->OnReceivedRtt(rtt_ms);
+ }
+ });
+}
+
} // namespace voe
} // namespace webrtc
diff --git a/audio/channel_send.h b/audio/channel_send.h
index 63e8d04..3d194a7 100644
--- a/audio/channel_send.h
+++ b/audio/channel_send.h
@@ -108,11 +108,11 @@
State state_;
};
-class ChannelSend
- : public Transport,
- public AudioPacketizationCallback, // receive encoded packets from the
- // ACM
- public OverheadObserver {
+class ChannelSend : public Transport,
+ public AudioPacketizationCallback, // receive encoded
+ // packets from the ACM
+ public OverheadObserver,
+ public TargetTransferRateObserver {
public:
// TODO(nisse): Make OnUplinkPacketLossRate public, and delete friend
// declaration.
@@ -202,12 +202,24 @@
void OnRecoverableUplinkPacketLossRate(float recoverable_packet_loss_rate);
+ // Returns the RTT in milliseconds.
int64_t GetRTT() const;
// E2EE Custom Audio Frame Encryption
void SetFrameEncryptor(
rtc::scoped_refptr<FrameEncryptorInterface> frame_encryptor);
+ // In RTP we currently rely on RTCP packets (|ReceivedRTCPPacket|) to inform
+ // about RTT.
+ // In media transport we rely on the TargetTransferRateObserver instead.
+ // In other words, if you are using RTP, you should expect
+ // |ReceivedRTCPPacket| to be called, if you are using media transport,
+ // |OnTargetTransferRate| will be called.
+ //
+ // In future, RTP media will move to the media transport implementation and
+ // these conditions will be removed.
+ void OnTargetTransferRate(TargetTransferRate rate) override;
+
private:
class ProcessAndEncodeAudioTask;
@@ -261,6 +273,8 @@
// for encoding.
void ProcessAndEncodeAudioOnTaskQueue(AudioFrame* audio_input);
+ void OnReceivedRtt(int64_t rtt_ms);
+
rtc::CriticalSection _callbackCritSect;
rtc::CriticalSection volume_settings_critsect_;
diff --git a/call/BUILD.gn b/call/BUILD.gn
index 34c16ef..50decd1 100644
--- a/call/BUILD.gn
+++ b/call/BUILD.gn
@@ -333,6 +333,7 @@
"..:webrtc_common",
"../api:array_view",
"../api:fake_media_transport",
+ "../api:fake_media_transport",
"../api:libjingle_peerconnection_api",
"../api:mock_audio_mixer",
"../api/audio_codecs:builtin_audio_decoder_factory",
diff --git a/call/call_unittest.cc b/call/call_unittest.cc
index 43c3355..770f255 100644
--- a/call/call_unittest.cc
+++ b/call/call_unittest.cc
@@ -290,4 +290,27 @@
EXPECT_EQ(rtp_state1.media_has_been_sent, rtp_state2.media_has_been_sent);
}
+TEST(CallTest, RegisterMediaTransportBitrateCallbacksInCreateStream) {
+ CallHelper call;
+ MediaTransportSettings settings;
+ webrtc::FakeMediaTransport fake_media_transport(settings);
+
+ EXPECT_EQ(0, fake_media_transport.target_rate_observers_size());
+ AudioSendStream::Config config(/*send_transport=*/nullptr,
+ /*media_transport=*/&fake_media_transport);
+
+ call->MediaTransportChange(&fake_media_transport);
+ AudioSendStream* stream = call->CreateAudioSendStream(config);
+
+ // We get 2 subscribers: one subscriber from call.cc, and one from
+ // ChannelSend.
+ EXPECT_EQ(2, fake_media_transport.target_rate_observers_size());
+
+ call->DestroyAudioSendStream(stream);
+ EXPECT_EQ(1, fake_media_transport.target_rate_observers_size());
+
+ call->MediaTransportChange(nullptr);
+ EXPECT_EQ(0, fake_media_transport.target_rate_observers_size());
+}
+
} // namespace webrtc
diff --git a/pc/channel.cc b/pc/channel.cc
index 0f857cd..67d6984 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -118,6 +118,11 @@
BaseChannel::~BaseChannel() {
TRACE_EVENT0("webrtc", "BaseChannel::~BaseChannel");
RTC_DCHECK_RUN_ON(worker_thread_);
+
+ if (media_transport_) {
+ media_transport_->SetNetworkChangeCallback(nullptr);
+ }
+
// Eats any outstanding messages or packets.
worker_thread_->Clear(&invoker_);
worker_thread_->Clear(this);
@@ -137,8 +142,15 @@
this, &BaseChannel::OnTransportReadyToSend);
rtp_transport_->SignalRtcpPacketReceived.connect(
this, &BaseChannel::OnRtcpPacketReceived);
- rtp_transport_->SignalNetworkRouteChanged.connect(
- this, &BaseChannel::OnNetworkRouteChanged);
+
+ // If media transport is used, it's responsible for providing network
+ // route changed callbacks.
+ if (!media_transport_) {
+ rtp_transport_->SignalNetworkRouteChanged.connect(
+ this, &BaseChannel::OnNetworkRouteChanged);
+ }
+ // TODO(bugs.webrtc.org/9719): Media transport should also be used to provide
+ // 'writable' state here.
rtp_transport_->SignalWritableState.connect(this,
&BaseChannel::OnWritableState);
rtp_transport_->SignalSentPacket.connect(this,
@@ -159,12 +171,20 @@
void BaseChannel::Init_w(webrtc::RtpTransportInternal* rtp_transport,
webrtc::MediaTransportInterface* media_transport) {
RTC_DCHECK_RUN_ON(worker_thread_);
+ media_transport_ = media_transport;
+
network_thread_->Invoke<void>(
RTC_FROM_HERE, [this, rtp_transport] { SetRtpTransport(rtp_transport); });
// Both RTP and RTCP channels should be set, we can call SetInterface on
// the media channel and it can set network options.
media_channel_->SetInterface(this, media_transport);
+
+ RTC_LOG(LS_INFO) << "BaseChannel::Init_w, media_transport="
+ << (media_transport_ != nullptr);
+ if (media_transport_) {
+ media_transport_->SetNetworkChangeCallback(this);
+ }
}
void BaseChannel::Deinit() {
@@ -353,6 +373,8 @@
void BaseChannel::OnNetworkRouteChanged(
absl::optional<rtc::NetworkRoute> network_route) {
+ RTC_LOG(LS_INFO) << "Network route was changed.";
+
RTC_DCHECK(network_thread_->IsCurrent());
rtc::NetworkRoute new_route;
if (network_route) {
@@ -767,6 +789,11 @@
Bind(&BaseChannel::UpdateMediaSendRecvState_w, this));
}
+void BaseChannel::OnNetworkRouteChanged(
+ const rtc::NetworkRoute& network_route) {
+ OnNetworkRouteChanged(absl::make_optional(network_route));
+}
+
void VoiceChannel::UpdateMediaSendRecvState_w() {
// Render incoming data if we're the active call, and we have the local
// content. We receive data on the default channel and multiplexed streams.
diff --git a/pc/channel.h b/pc/channel.h
index 5644bb4..6264bb3 100644
--- a/pc/channel.h
+++ b/pc/channel.h
@@ -72,7 +72,8 @@
public rtc::MessageHandler,
public sigslot::has_slots<>,
public MediaChannel::NetworkInterface,
- public webrtc::RtpPacketSinkInterface {
+ public webrtc::RtpPacketSinkInterface,
+ public webrtc::MediaTransportNetworkChangeCallback {
public:
// If |srtp_required| is true, the channel will not send or receive any
// RTP/RTCP packets without using SRTP (either using SDES or DTLS-SRTP).
@@ -303,6 +304,9 @@
void SignalSentPacket_n(const rtc::SentPacket& sent_packet);
void SignalSentPacket_w(const rtc::SentPacket& sent_packet);
bool IsReadyToSendMedia_n() const;
+
+ // MediaTransportNetworkChangeCallback override.
+ void OnNetworkRouteChanged(const rtc::NetworkRoute& network_route) override;
rtc::Thread* const worker_thread_;
rtc::Thread* const network_thread_;
rtc::Thread* const signaling_thread_;
