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}
This commit is contained in:
Jiawei Ou 2018-11-09 13:17:39 -08:00 committed by Commit Bot
parent 4aeb35b6d0
commit 55718120e6
23 changed files with 72 additions and 53 deletions

View file

@ -340,6 +340,22 @@ class PeerConnectionInterface : public rtc::RefCountInterface {
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;

View file

@ -66,11 +66,13 @@ std::unique_ptr<voe::ChannelSendProxy> CreateChannelAndProxy(
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 @@ AudioSendStream::AudioSendStream(
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,

View file

@ -334,11 +334,12 @@ TEST(AudioSendStreamTest, ConfigToString) {
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, "

View file

@ -454,7 +454,8 @@ ChannelSend::ChannelSend(rtc::TaskQueue* encoder_queue,
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 @@ ChannelSend::ChannelSend(rtc::TaskQueue* encoder_queue,
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);

View file

@ -125,7 +125,8 @@ class ChannelSend
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();

View file

@ -34,6 +34,7 @@ std::string AudioSendStream::Config::ToString() const {
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;

View file

@ -96,6 +96,9 @@ class AudioSendStream {
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;

View file

@ -112,18 +112,4 @@ std::string RtpConfig::Rtx::ToString() const {
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

View file

@ -134,17 +134,5 @@ struct RtpConfig {
// 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_

View file

@ -85,15 +85,15 @@ RtpVideoSenderInterface* RtpTransportControllerSend::CreateRtpVideoSender(
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),

View file

@ -54,7 +54,7 @@ class RtpTransportControllerSend final
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,

View file

@ -104,7 +104,7 @@ class RtpTransportControllerSendInterface {
// 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,

View file

@ -41,7 +41,7 @@ static const size_t kPathMTU = 1500;
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 @@ std::vector<std::unique_ptr<RtpRtcp>> CreateRtpRtcpModules(
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 @@ RtpVideoSender::RtpVideoSender(
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 @@ RtpVideoSender::RtpVideoSender(
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(),

View file

@ -54,7 +54,7 @@ class RtpVideoSender : public RtpVideoSenderInterface,
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,

View file

@ -102,7 +102,7 @@ class RtpVideoSenderTestFixture {
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_),

View file

@ -38,7 +38,7 @@ class MockRtpTransportControllerSend
std::map<uint32_t, RtpState>,
const std::map<uint32_t, RtpPayloadState>&,
const RtpConfig&,
const RtcpConfig&,
int rtcp_report_interval_ms,
Transport*,
const RtpSenderObservers&,
RtcEventLog*,

View file

@ -79,7 +79,7 @@ std::string VideoSendStream::Config::ToString() const {
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;

View file

@ -113,7 +113,8 @@ class VideoSendStream {
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;

View file

@ -59,8 +59,17 @@ struct MediaConfig {
// 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 @@ struct MediaConfig {
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); }

View file

@ -1082,6 +1082,7 @@ bool WebRtcVideoChannel::AddSendStream(const StreamParams& sp) {
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_,

View file

@ -708,6 +708,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
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 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
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 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(
: 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 @@ bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) {
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));

View file

@ -276,6 +276,8 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel,
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_;

View file

@ -251,7 +251,7 @@ VideoSendStreamImpl::VideoSendStreamImpl(
suspended_ssrcs,
suspended_payload_states,
config_->rtp,
config_->rtcp,
config_->rtcp_report_interval_ms,
config_->send_transport,
CreateObservers(call_stats,
&encoder_feedback_,