Cleanup: Using RtpRtcp directly from AudioSendStream
This reduces indirection and makes it easier to follow code. It also
fits into a long term strategy of reducing the scope of ChannelSend.
Bug: webrtc:9883
Change-Id: I2661c4aa6c561f7691beaaa289636254f7a58b72
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166042
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30273}
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 90d72c4..5e3b9ff 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -32,6 +32,7 @@
#include "logging/rtc_event_log/rtc_stream_config.h"
#include "modules/audio_coding/codecs/cng/audio_encoder_cng.h"
#include "modules/audio_processing/include/audio_processing.h"
+#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "rtc_base/checks.h"
#include "rtc_base/event.h"
#include "rtc_base/logging.h"
@@ -156,7 +157,7 @@
!field_trial::IsDisabled("WebRTC-Audio-LegacyOverhead")),
bitrate_allocator_(bitrate_allocator),
rtp_transport_(rtp_transport),
- rtp_rtcp_module_(nullptr),
+ rtp_rtcp_module_(channel_send_->GetRtpRtcp()),
suspended_rtp_state_(suspended_rtp_state) {
RTC_LOG(LS_INFO) << "AudioSendStream: " << config.rtp.ssrc;
RTC_DCHECK(worker_queue_);
@@ -165,7 +166,6 @@
RTC_DCHECK(bitrate_allocator_);
RTC_DCHECK(rtp_transport);
- rtp_rtcp_module_ = channel_send_->GetRtpRtcp();
RTC_DCHECK(rtp_rtcp_module_);
ConfigureStream(config, true);
@@ -249,7 +249,7 @@
if (first_time ||
new_config.rtp.extmap_allow_mixed != old_config.rtp.extmap_allow_mixed) {
- channel_send_->SetExtmapAllowMixed(new_config.rtp.extmap_allow_mixed);
+ rtp_rtcp_module_->SetExtmapAllowMixed(new_config.rtp.extmap_allow_mixed);
}
const ExtensionIds old_ids = FindExtensionIds(old_config.rtp.extensions);
@@ -262,7 +262,7 @@
}
if (first_time || new_ids.abs_send_time != old_ids.abs_send_time) {
- channel_send_->GetRtpRtcp()->DeregisterSendRtpHeaderExtension(
+ rtp_rtcp_module_->DeregisterSendRtpHeaderExtension(
kRtpExtensionAbsoluteSendTime);
if (new_ids.abs_send_time) {
rtp_rtcp_module_->RegisterRtpHeaderExtension(AbsoluteSendTime::kUri,
@@ -282,8 +282,8 @@
if (audio_send_side_bwe_ && !allocate_audio_without_feedback_ &&
new_ids.transport_sequence_number != 0) {
- channel_send_->EnableSendTransportSequenceNumber(
- new_ids.transport_sequence_number);
+ rtp_rtcp_module_->RegisterRtpHeaderExtension(
+ TransportSequenceNumber::kUri, new_ids.transport_sequence_number);
// Probing in application limited region is only used in combination with
// send side congestion control, wich depends on feedback packets which
// requires transport sequence numbers to be enabled.
@@ -301,15 +301,26 @@
if ((first_time || new_ids.mid != old_ids.mid ||
new_config.rtp.mid != old_config.rtp.mid) &&
new_ids.mid != 0 && !new_config.rtp.mid.empty()) {
- channel_send_->SetMid(new_config.rtp.mid, new_ids.mid);
+ rtp_rtcp_module_->RegisterRtpHeaderExtension(RtpMid::kUri, new_ids.mid);
+ rtp_rtcp_module_->SetMid(new_config.rtp.mid);
}
// RID RTP header extension
if ((first_time || new_ids.rid != old_ids.rid ||
new_ids.repaired_rid != old_ids.repaired_rid ||
new_config.rtp.rid != old_config.rtp.rid)) {
- channel_send_->SetRid(new_config.rtp.rid, new_ids.rid,
- new_ids.repaired_rid);
+ if (new_ids.rid != 0 || new_ids.repaired_rid != 0) {
+ if (new_config.rtp.rid.empty()) {
+ rtp_rtcp_module_->DeregisterSendRtpHeaderExtension(RtpStreamId::kUri);
+ } else if (new_ids.repaired_rid != 0) {
+ rtp_rtcp_module_->RegisterRtpHeaderExtension(RtpStreamId::kUri,
+ new_ids.repaired_rid);
+ } else {
+ rtp_rtcp_module_->RegisterRtpHeaderExtension(RtpStreamId::kUri,
+ new_ids.rid);
+ }
+ }
+ rtp_rtcp_module_->SetRid(new_config.rtp.rid);
}
if (!ReconfigureSendCodec(new_config)) {
diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h
index fff3ee1..6875915 100644
--- a/audio/audio_send_stream.h
+++ b/audio/audio_send_stream.h
@@ -181,7 +181,7 @@
RTC_GUARDED_BY(worker_queue_);
RtpTransportControllerSendInterface* const rtp_transport_;
- RtpRtcp* rtp_rtcp_module_;
+ RtpRtcp* const rtp_rtcp_module_;
absl::optional<RtpState> const suspended_rtp_state_;
// RFC 5285: Each distinct extension MUST have a unique ID. The value 0 is
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc
index 95d7f73..0472366 100644
--- a/audio/audio_send_stream_unittest.cc
+++ b/audio/audio_send_stream_unittest.cc
@@ -195,6 +195,7 @@
return *static_cast<MockAudioEncoderFactory*>(
stream_config_.encoder_factory.get());
}
+ MockRtpRtcp* rtp_rtcp() { return &rtp_rtcp_; }
MockChannelSend* channel_send() { return channel_send_; }
RtpTransportControllerSendInterface* transport() { return &rtp_transport_; }
@@ -213,15 +214,16 @@
EXPECT_CALL(rtp_rtcp_, SSRC).WillRepeatedly(Return(kSsrc));
EXPECT_CALL(*channel_send_, SetRTCP_CNAME(StrEq(kCName))).Times(1);
EXPECT_CALL(*channel_send_, SetFrameEncryptor(_)).Times(1);
- EXPECT_CALL(*channel_send_, SetExtmapAllowMixed(false)).Times(1);
+ EXPECT_CALL(rtp_rtcp_, SetExtmapAllowMixed(false)).Times(1);
EXPECT_CALL(*channel_send_,
SetSendAudioLevelIndicationStatus(true, kAudioLevelId))
.Times(1);
EXPECT_CALL(rtp_transport_, GetBandwidthObserver())
.WillRepeatedly(Return(&bandwidth_observer_));
if (audio_bwe_enabled) {
- EXPECT_CALL(*channel_send_,
- EnableSendTransportSequenceNumber(kTransportSequenceNumberId))
+ EXPECT_CALL(rtp_rtcp_,
+ RegisterRtpHeaderExtension(TransportSequenceNumber::kUri,
+ kTransportSequenceNumberId))
.Times(1);
EXPECT_CALL(*channel_send_,
RegisterSenderCongestionControlObjects(
@@ -233,7 +235,7 @@
.Times(1);
}
EXPECT_CALL(*channel_send_, ResetSenderCongestionControlObjects()).Times(1);
- EXPECT_CALL(*channel_send_, SetRid(std::string(), 0, 0)).Times(1);
+ EXPECT_CALL(rtp_rtcp_, SetRid(std::string())).Times(1);
}
void SetupMockForSetupSendCodec(bool expect_set_encoder_call) {
@@ -705,8 +707,10 @@
auto send_stream = helper.CreateAudioSendStream();
auto new_config = helper.config();
ConfigHelper::AddBweToConfig(&new_config);
- EXPECT_CALL(*helper.channel_send(),
- EnableSendTransportSequenceNumber(kTransportSequenceNumberId))
+
+ EXPECT_CALL(*helper.rtp_rtcp(),
+ RegisterRtpHeaderExtension(TransportSequenceNumber::kUri,
+ kTransportSequenceNumberId))
.Times(1);
{
::testing::InSequence seq;
diff --git a/audio/channel_send.cc b/audio/channel_send.cc
index de77158..2fa0706 100644
--- a/audio/channel_send.cc
+++ b/audio/channel_send.cc
@@ -115,13 +115,7 @@
int payload_frequency) override;
// RTP+RTCP
- void SetRid(const std::string& rid,
- int extension_id,
- int repaired_extension_id) override;
- void SetMid(const std::string& mid, int extension_id) override;
- void SetExtmapAllowMixed(bool extmap_allow_mixed) override;
void SetSendAudioLevelIndicationStatus(bool enable, int id) override;
- void EnableSendTransportSequenceNumber(int id) override;
void RegisterSenderCongestionControlObjects(
RtpTransportControllerSendInterface* transport,
@@ -159,8 +153,6 @@
void OnUplinkPacketLossRate(float packet_loss_rate);
bool InputMute() const;
- void SetSendRtpHeaderExtension(bool enable, absl::string_view uri, int id);
-
int32_t SendRtpAudio(AudioFrameType frameType,
uint8_t payloadType,
uint32_t timeStamp,
@@ -698,40 +690,14 @@
payload_frequency, 0, 0);
}
-void ChannelSend::SetRid(const std::string& rid,
- int extension_id,
- int repaired_extension_id) {
- RTC_DCHECK_RUN_ON(&worker_thread_checker_);
- if (extension_id != 0) {
- SetSendRtpHeaderExtension(!rid.empty(), RtpStreamId::kUri, extension_id);
- }
- if (repaired_extension_id != 0) {
- SetSendRtpHeaderExtension(!rid.empty(), RtpStreamId::kUri,
- repaired_extension_id);
- }
- _rtpRtcpModule->SetRid(rid);
-}
-
-void ChannelSend::SetMid(const std::string& mid, int extension_id) {
- RTC_DCHECK_RUN_ON(&worker_thread_checker_);
- SetSendRtpHeaderExtension(true, RtpMid::kUri, extension_id);
- _rtpRtcpModule->SetMid(mid);
-}
-
-void ChannelSend::SetExtmapAllowMixed(bool extmap_allow_mixed) {
- RTC_DCHECK_RUN_ON(&worker_thread_checker_);
- _rtpRtcpModule->SetExtmapAllowMixed(extmap_allow_mixed);
-}
-
void ChannelSend::SetSendAudioLevelIndicationStatus(bool enable, int id) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
_includeAudioLevelIndication = enable;
- SetSendRtpHeaderExtension(enable, AudioLevel::kUri, id);
-}
-
-void ChannelSend::EnableSendTransportSequenceNumber(int id) {
- RTC_DCHECK_RUN_ON(&worker_thread_checker_);
- SetSendRtpHeaderExtension(true, TransportSequenceNumber::kUri, id);
+ if (enable) {
+ _rtpRtcpModule->RegisterRtpHeaderExtension(AudioLevel::kUri, id);
+ } else {
+ _rtpRtcpModule->DeregisterSendRtpHeaderExtension(AudioLevel::kUri);
+ }
}
void ChannelSend::RegisterSenderCongestionControlObjects(
@@ -895,15 +861,6 @@
return _rtpRtcpModule.get();
}
-void ChannelSend::SetSendRtpHeaderExtension(bool enable,
- absl::string_view uri,
- int id) {
- _rtpRtcpModule->DeregisterSendRtpHeaderExtension(uri);
- if (enable) {
- _rtpRtcpModule->RegisterRtpHeaderExtension(uri, id);
- }
-}
-
int64_t ChannelSend::GetRTT() const {
std::vector<RTCPReportBlock> report_blocks;
_rtpRtcpModule->RemoteRTCPStat(&report_blocks);
diff --git a/audio/channel_send.h b/audio/channel_send.h
index 6f73c2b..0fe434b 100644
--- a/audio/channel_send.h
+++ b/audio/channel_send.h
@@ -77,14 +77,8 @@
virtual void CallEncoder(rtc::FunctionView<void(AudioEncoder*)> modifier) = 0;
// Use 0 to indicate that the extension should not be registered.
- virtual void SetRid(const std::string& rid,
- int extension_id,
- int repaired_extension_id) = 0;
- virtual void SetMid(const std::string& mid, int extension_id) = 0;
virtual void SetRTCP_CNAME(absl::string_view c_name) = 0;
- virtual void SetExtmapAllowMixed(bool extmap_allow_mixed) = 0;
virtual void SetSendAudioLevelIndicationStatus(bool enable, int id) = 0;
- virtual void EnableSendTransportSequenceNumber(int id) = 0;
virtual void RegisterSenderCongestionControlObjects(
RtpTransportControllerSendInterface* transport,
RtcpBandwidthObserver* bandwidth_observer) = 0;
diff --git a/audio/mock_voe_channel_proxy.h b/audio/mock_voe_channel_proxy.h
index d61bc89..e4c60a1 100644
--- a/audio/mock_voe_channel_proxy.h
+++ b/audio/mock_voe_channel_proxy.h
@@ -82,15 +82,8 @@
void(rtc::FunctionView<void(std::unique_ptr<AudioEncoder>*)> modifier));
MOCK_METHOD1(CallEncoder,
void(rtc::FunctionView<void(AudioEncoder*)> modifier));
- MOCK_METHOD3(SetRid,
- void(const std::string& rid,
- int extension_id,
- int repaired_extension_id));
- MOCK_METHOD2(SetMid, void(const std::string& mid, int extension_id));
MOCK_METHOD1(SetRTCP_CNAME, void(absl::string_view c_name));
- MOCK_METHOD1(SetExtmapAllowMixed, void(bool extmap_allow_mixed));
MOCK_METHOD2(SetSendAudioLevelIndicationStatus, void(bool enable, int id));
- MOCK_METHOD1(EnableSendTransportSequenceNumber, void(int id));
MOCK_METHOD2(RegisterSenderCongestionControlObjects,
void(RtpTransportControllerSendInterface* transport,
RtcpBandwidthObserver* bandwidth_observer));