Adding rtcp report interval into RTCConfiguration.
This is a follow up of https://webrtc-review.googlesource.com/c/src/+/43201.
Issue 43201 didn't do the job properly.
1. The audio rtcp report interval is not properly hooked up.
2. We don't need to propagate audio rtcp interval into video send stream or vice versa.
3. We don't need to propagate rtcp report interval to any receiving streams.
Bug: webrtc:8789
Change-Id: I1f637d6e5173608564ef0702d7eda6fc93b3200f
Reviewed-on: https://webrtc-review.googlesource.com/c/110105
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Magnus Flodman <mflodman@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Jiawei Ou <ouj@fb.com>
Cr-Commit-Position: refs/heads/master@{#25610}diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h
index 044a480..3f01760 100644
--- a/api/peerconnectioninterface.h
+++ b/api/peerconnectioninterface.h
@@ -340,6 +340,22 @@
media_config.video.experiment_cpu_load_estimator = enable;
}
+ int audio_rtcp_report_interval_ms() const {
+ return media_config.audio.rtcp_report_interval_ms;
+ }
+ void set_audio_rtcp_report_interval_ms(int audio_rtcp_report_interval_ms) {
+ media_config.audio.rtcp_report_interval_ms =
+ audio_rtcp_report_interval_ms;
+ }
+
+ int video_rtcp_report_interval_ms() const {
+ return media_config.video.rtcp_report_interval_ms;
+ }
+ void set_video_rtcp_report_interval_ms(int video_rtcp_report_interval_ms) {
+ media_config.video.rtcp_report_interval_ms =
+ video_rtcp_report_interval_ms;
+ }
+
static const int kUndefined = -1;
// Default maximum number of packets in the audio jitter buffer.
static const int kAudioJitterBufferMaxPackets = 50;
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 37f89c5..7824653 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -66,11 +66,13 @@
RtcEventLog* event_log,
FrameEncryptorInterface* frame_encryptor,
const webrtc::CryptoOptions& crypto_options,
- bool extmap_allow_mixed) {
+ bool extmap_allow_mixed,
+ int rtcp_report_interval_ms) {
return absl::make_unique<voe::ChannelSendProxy>(
absl::make_unique<voe::ChannelSend>(
worker_queue, module_process_thread, media_transport, rtcp_rtt_stats,
- event_log, frame_encryptor, crypto_options, extmap_allow_mixed));
+ event_log, frame_encryptor, crypto_options, extmap_allow_mixed,
+ rtcp_report_interval_ms));
}
void UpdateEventLogStreamConfig(RtcEventLog* event_log,
@@ -157,7 +159,8 @@
event_log,
config.frame_encryptor,
config.crypto_options,
- config.rtp.extmap_allow_mixed)) {}
+ config.rtp.extmap_allow_mixed,
+ config.rtcp_report_interval_ms)) {}
AudioSendStream::AudioSendStream(
const webrtc::AudioSendStream::Config& config,
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc
index 6a92329..ed94283 100644
--- a/audio/audio_send_stream_unittest.cc
+++ b/audio/audio_send_stream_unittest.cc
@@ -334,11 +334,12 @@
config.rtp.extmap_allow_mixed = true;
config.rtp.extensions.push_back(
RtpExtension(RtpExtension::kAudioLevelUri, kAudioLevelId));
+ config.rtcp_report_interval_ms = 2500;
EXPECT_EQ(
"{rtp: {ssrc: 1234, extmap-allow-mixed: true, extensions: [{uri: "
"urn:ietf:params:rtp-hdrext:ssrc-audio-level, id: 2}], nack: "
- "{rtp_history_ms: 0}, c_name: foo_name}, send_transport: null, "
- "media_transport: null, "
+ "{rtp_history_ms: 0}, c_name: foo_name}, rtcp_report_interval_ms: 2500, "
+ "send_transport: null, media_transport: null, "
"min_bitrate_bps: 12000, max_bitrate_bps: 34000, "
"send_codec_spec: {nack_enabled: true, transport_cc_enabled: false, "
"cng_payload_type: 42, payload_type: 103, "
diff --git a/audio/channel_send.cc b/audio/channel_send.cc
index c0de939..a4a4977 100644
--- a/audio/channel_send.cc
+++ b/audio/channel_send.cc
@@ -454,7 +454,8 @@
RtcEventLog* rtc_event_log,
FrameEncryptorInterface* frame_encryptor,
const webrtc::CryptoOptions& crypto_options,
- bool extmap_allow_mixed)
+ bool extmap_allow_mixed,
+ int rtcp_report_interval_ms)
: event_log_(rtc_event_log),
_timeStamp(0), // This is just an offset, RTP module will add it's own
// random offset
@@ -498,6 +499,8 @@
configuration.retransmission_rate_limiter =
retransmission_rate_limiter_.get();
configuration.extmap_allow_mixed = extmap_allow_mixed;
+ configuration.rtcp_interval_config.audio_interval_ms =
+ rtcp_report_interval_ms;
_rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration));
_rtpRtcpModule->SetSendingMediaStatus(false);
diff --git a/audio/channel_send.h b/audio/channel_send.h
index 407303f..6013d1e 100644
--- a/audio/channel_send.h
+++ b/audio/channel_send.h
@@ -125,7 +125,8 @@
RtcEventLog* rtc_event_log,
FrameEncryptorInterface* frame_encryptor,
const webrtc::CryptoOptions& crypto_options,
- bool extmap_allow_mixed);
+ bool extmap_allow_mixed,
+ int rtcp_report_interval_ms);
virtual ~ChannelSend();
diff --git a/call/audio_send_stream.cc b/call/audio_send_stream.cc
index 3721268..7fc6fa3 100644
--- a/call/audio_send_stream.cc
+++ b/call/audio_send_stream.cc
@@ -34,6 +34,7 @@
char buf[1024];
rtc::SimpleStringBuilder ss(buf);
ss << "{rtp: " << rtp.ToString();
+ ss << ", rtcp_report_interval_ms: " << rtcp_report_interval_ms;
ss << ", send_transport: " << (send_transport ? "(Transport)" : "null");
ss << ", media_transport: " << (media_transport ? "(Transport)" : "null");
ss << ", min_bitrate_bps: " << min_bitrate_bps;
diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h
index ba0d652..0cd0894 100644
--- a/call/audio_send_stream.h
+++ b/call/audio_send_stream.h
@@ -96,6 +96,9 @@
std::string c_name;
} rtp;
+ // Time interval between RTCP report for audio
+ int rtcp_report_interval_ms = 5000;
+
// Transport for outgoing packets. The transport is expected to exist for
// the entire life of the AudioSendStream and is owned by the API client.
Transport* send_transport = nullptr;
diff --git a/call/rtp_config.cc b/call/rtp_config.cc
index 30631f5..cce78ad 100644
--- a/call/rtp_config.cc
+++ b/call/rtp_config.cc
@@ -112,18 +112,4 @@
ss << '}';
return ss.str();
}
-
-RtcpConfig::RtcpConfig() = default;
-RtcpConfig::RtcpConfig(const RtcpConfig&) = default;
-RtcpConfig::~RtcpConfig() = default;
-
-std::string RtcpConfig::ToString() const {
- char buf[1024];
- rtc::SimpleStringBuilder ss(buf);
- ss << "{video_report_interval_ms: " << video_report_interval_ms;
- ss << ", audio_report_interval_ms: " << audio_report_interval_ms;
- ss << '}';
- return ss.str();
-}
-
} // namespace webrtc
diff --git a/call/rtp_config.h b/call/rtp_config.h
index 5d22182..97ae951 100644
--- a/call/rtp_config.h
+++ b/call/rtp_config.h
@@ -134,17 +134,5 @@
// RTCP CNAME, see RFC 3550.
std::string c_name;
};
-
-struct RtcpConfig {
- RtcpConfig();
- RtcpConfig(const RtcpConfig&);
- ~RtcpConfig();
- std::string ToString() const;
-
- // Time interval between RTCP report for video
- int64_t video_report_interval_ms = 1000;
- // Time interval between RTCP report for audio
- int64_t audio_report_interval_ms = 5000;
-};
} // namespace webrtc
#endif // CALL_RTP_CONFIG_H_
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index f225bed..5265c95 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -85,15 +85,15 @@
std::map<uint32_t, RtpState> suspended_ssrcs,
const std::map<uint32_t, RtpPayloadState>& states,
const RtpConfig& rtp_config,
- const RtcpConfig& rtcp_config,
+ int rtcp_report_interval_ms,
Transport* send_transport,
const RtpSenderObservers& observers,
RtcEventLog* event_log,
std::unique_ptr<FecController> fec_controller,
const RtpSenderFrameEncryptionConfig& frame_encryption_config) {
video_rtp_senders_.push_back(absl::make_unique<RtpVideoSender>(
- ssrcs, suspended_ssrcs, states, rtp_config, rtcp_config, send_transport,
- observers,
+ ssrcs, suspended_ssrcs, states, rtp_config, rtcp_report_interval_ms,
+ send_transport, observers,
// TODO(holmer): Remove this circular dependency by injecting
// the parts of RtpTransportControllerSendInterface that are really used.
this, event_log, &retransmission_rate_limiter_, std::move(fec_controller),
diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h
index c6b2362..d62cfb6 100644
--- a/call/rtp_transport_controller_send.h
+++ b/call/rtp_transport_controller_send.h
@@ -54,7 +54,7 @@
const std::map<uint32_t, RtpPayloadState>&
states, // move states into RtpTransportControllerSend
const RtpConfig& rtp_config,
- const RtcpConfig& rtcp_config,
+ int rtcp_report_interval_ms,
Transport* send_transport,
const RtpSenderObservers& observers,
RtcEventLog* event_log,
diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h
index e93e6ca..219c36d 100644
--- a/call/rtp_transport_controller_send_interface.h
+++ b/call/rtp_transport_controller_send_interface.h
@@ -104,7 +104,7 @@
// TODO(holmer): Move states into RtpTransportControllerSend.
const std::map<uint32_t, RtpPayloadState>& states,
const RtpConfig& rtp_config,
- const RtcpConfig& rtcp_config,
+ int rtcp_report_interval_ms,
Transport* send_transport,
const RtpSenderObservers& observers,
RtcEventLog* event_log,
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 214fe96..c93f93d 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -41,7 +41,7 @@
std::vector<std::unique_ptr<RtpRtcp>> CreateRtpRtcpModules(
const std::vector<uint32_t>& ssrcs,
const RtpConfig& rtp_config,
- const RtcpConfig& rtcp_config,
+ int rtcp_report_interval_ms,
Transport* send_transport,
RtcpIntraFrameObserver* intra_frame_callback,
RtcpBandwidthObserver* bandwidth_callback,
@@ -82,14 +82,12 @@
configuration.retransmission_rate_limiter = retransmission_rate_limiter;
configuration.overhead_observer = overhead_observer;
configuration.keepalive_config = keepalive_config;
- configuration.rtcp_interval_config.video_interval_ms =
- rtcp_config.video_report_interval_ms;
- configuration.rtcp_interval_config.audio_interval_ms =
- rtcp_config.audio_report_interval_ms;
configuration.frame_encryptor = frame_encryptor;
configuration.require_frame_encryption =
crypto_options.sframe.require_frame_encryption;
configuration.extmap_allow_mixed = rtp_config.extmap_allow_mixed;
+ configuration.rtcp_interval_config.video_interval_ms =
+ rtcp_report_interval_ms;
std::vector<std::unique_ptr<RtpRtcp>> modules;
const std::vector<uint32_t>& flexfec_protected_ssrcs =
@@ -186,7 +184,7 @@
std::map<uint32_t, RtpState> suspended_ssrcs,
const std::map<uint32_t, RtpPayloadState>& states,
const RtpConfig& rtp_config,
- const RtcpConfig& rtcp_config,
+ int rtcp_report_interval_ms,
Transport* send_transport,
const RtpSenderObservers& observers,
RtpTransportControllerSendInterface* transport,
@@ -204,7 +202,7 @@
fec_controller_(std::move(fec_controller)),
rtp_modules_(CreateRtpRtcpModules(ssrcs,
rtp_config,
- rtcp_config,
+ rtcp_report_interval_ms,
send_transport,
observers.intra_frame_callback,
transport->GetBandwidthObserver(),
diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h
index 91e5fd6..d0965fd 100644
--- a/call/rtp_video_sender.h
+++ b/call/rtp_video_sender.h
@@ -54,7 +54,7 @@
std::map<uint32_t, RtpState> suspended_ssrcs,
const std::map<uint32_t, RtpPayloadState>& states,
const RtpConfig& rtp_config,
- const RtcpConfig& rtcp_config,
+ int rtcp_report_interval_ms,
Transport* send_transport,
const RtpSenderObservers& observers,
RtpTransportControllerSendInterface* transport,
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index 00b61fe..8ab89d6 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -102,7 +102,7 @@
std::map<uint32_t, RtpState> suspended_ssrcs;
router_ = absl::make_unique<RtpVideoSender>(
config_.rtp.ssrcs, suspended_ssrcs, suspended_payload_states,
- config_.rtp, config_.rtcp, &transport_,
+ config_.rtp, config_.rtcp_report_interval_ms, &transport_,
CreateObservers(&call_stats_, &encoder_feedback_, &stats_proxy_,
&stats_proxy_, &stats_proxy_, &stats_proxy_,
&stats_proxy_, &stats_proxy_, &send_delay_stats_),
diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h
index 621447d..4d34d06 100644
--- a/call/test/mock_rtp_transport_controller_send.h
+++ b/call/test/mock_rtp_transport_controller_send.h
@@ -38,7 +38,7 @@
std::map<uint32_t, RtpState>,
const std::map<uint32_t, RtpPayloadState>&,
const RtpConfig&,
- const RtcpConfig&,
+ int rtcp_report_interval_ms,
Transport*,
const RtpSenderObservers&,
RtcEventLog*,
diff --git a/call/video_send_stream.cc b/call/video_send_stream.cc
index 08bede5..c8ba308 100644
--- a/call/video_send_stream.cc
+++ b/call/video_send_stream.cc
@@ -79,7 +79,7 @@
ss << "{encoder_settings: { experiment_cpu_load_estimator: "
<< (encoder_settings.experiment_cpu_load_estimator ? "on" : "off") << "}}";
ss << ", rtp: " << rtp.ToString();
- ss << ", rtcp: " << rtcp.ToString();
+ ss << ", rtcp_report_interval_ms: " << rtcp_report_interval_ms;
ss << ", pre_encode_callback: "
<< (pre_encode_callback ? "(VideoSinkInterface)" : "nullptr");
ss << ", render_delay_ms: " << render_delay_ms;
diff --git a/call/video_send_stream.h b/call/video_send_stream.h
index 162622e..e91882c 100644
--- a/call/video_send_stream.h
+++ b/call/video_send_stream.h
@@ -113,7 +113,8 @@
RtpConfig rtp;
- RtcpConfig rtcp;
+ // Time interval between RTCP report for video
+ int rtcp_report_interval_ms = 1000;
// Transport for outgoing packets.
Transport* send_transport = nullptr;
diff --git a/media/base/mediaconfig.h b/media/base/mediaconfig.h
index eda387e..c8fc521 100644
--- a/media/base/mediaconfig.h
+++ b/media/base/mediaconfig.h
@@ -59,8 +59,17 @@
// TODO(bugs.webrtc.org/8504): If all goes well, the flag will be removed
// together with the old method of estimation.
bool experiment_cpu_load_estimator = false;
+
+ // Time interval between RTCP report for video
+ int rtcp_report_interval_ms = 1000;
} video;
+ // Audio-specific config.
+ struct Audio {
+ // Time interval between RTCP report for audio
+ int rtcp_report_interval_ms = 5000;
+ } audio;
+
bool operator==(const MediaConfig& o) const {
return enable_dscp == o.enable_dscp &&
video.enable_cpu_adaptation == o.video.enable_cpu_adaptation &&
@@ -71,7 +80,9 @@
video.periodic_alr_bandwidth_probing ==
o.video.periodic_alr_bandwidth_probing &&
video.experiment_cpu_load_estimator ==
- o.video.experiment_cpu_load_estimator;
+ o.video.experiment_cpu_load_estimator &&
+ video.rtcp_report_interval_ms == o.video.rtcp_report_interval_ms &&
+ audio.rtcp_report_interval_ms == o.audio.rtcp_report_interval_ms;
}
bool operator!=(const MediaConfig& o) const { return !(*this == o); }
diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc
index c796b4f..74aa55b 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -1082,6 +1082,7 @@
bitrate_allocator_factory_;
config.crypto_options = crypto_options_;
config.rtp.extmap_allow_mixed = ExtmapAllowMixed();
+ config.rtcp_report_interval_ms = video_config_.rtcp_report_interval_ms;
WebRtcVideoSendStream* stream = new WebRtcVideoSendStream(
call_, sp, std::move(config), default_send_options_,
diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc
index b753a42..eaf8746 100644
--- a/media/engine/webrtcvoiceengine.cc
+++ b/media/engine/webrtcvoiceengine.cc
@@ -708,6 +708,7 @@
bool extmap_allow_mixed,
const std::vector<webrtc::RtpExtension>& extensions,
int max_send_bitrate_bps,
+ int rtcp_report_interval_ms,
const absl::optional<std::string>& audio_network_adaptor_config,
webrtc::Call* call,
webrtc::Transport* send_transport,
@@ -737,6 +738,7 @@
config_.track_id = track_id;
config_.frame_encryptor = frame_encryptor;
config_.crypto_options = crypto_options;
+ config_.rtcp_report_interval_ms = rtcp_report_interval_ms;
rtp_parameters_.encodings[0].ssrc = ssrc;
rtp_parameters_.rtcp.cname = c_name;
rtp_parameters_.header_extensions = extensions;
@@ -1258,6 +1260,7 @@
: VoiceMediaChannel(config),
engine_(engine),
call_(call),
+ audio_config_(config.audio),
crypto_options_(crypto_options) {
RTC_LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel";
RTC_DCHECK(call);
@@ -1810,7 +1813,8 @@
GetAudioNetworkAdaptorConfig(options_);
WebRtcAudioSendStream* stream = new WebRtcAudioSendStream(
ssrc, mid_, sp.cname, sp.id, send_codec_spec_, ExtmapAllowMixed(),
- send_rtp_extensions_, max_send_bitrate_bps_, audio_network_adaptor_config,
+ send_rtp_extensions_, max_send_bitrate_bps_,
+ audio_config_.rtcp_report_interval_ms, audio_network_adaptor_config,
call_, this, media_transport(), engine()->encoder_factory_,
codec_pair_id_, nullptr, crypto_options_);
send_streams_.insert(std::make_pair(ssrc, stream));
diff --git a/media/engine/webrtcvoiceengine.h b/media/engine/webrtcvoiceengine.h
index d8eff15..7a5343e 100644
--- a/media/engine/webrtcvoiceengine.h
+++ b/media/engine/webrtcvoiceengine.h
@@ -276,6 +276,8 @@
bool send_ = false;
webrtc::Call* const call_ = nullptr;
+ const MediaConfig::Audio audio_config_;
+
// Queue of unsignaled SSRCs; oldest at the beginning.
std::vector<uint32_t> unsignaled_recv_ssrcs_;
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index e6d97e9..1cb4dbc 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -251,7 +251,7 @@
suspended_ssrcs,
suspended_payload_states,
config_->rtp,
- config_->rtcp,
+ config_->rtcp_report_interval_ms,
config_->send_transport,
CreateObservers(call_stats,
&encoder_feedback_,
