Adding reduced size RTCP configuration down to the video stream level.
Still waiting to turn on negotiation (in mediasession.cc)
until we verify it's working as expected.
BUG=webrtc:4868
Review URL: https://codereview.webrtc.org/1418123003
Cr-Commit-Position: refs/heads/master@{#10958}
diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc
index fceaaf4..bc5dc1d 100644
--- a/talk/app/webrtc/webrtcsdp.cc
+++ b/talk/app/webrtc/webrtcsdp.cc
@@ -122,6 +122,7 @@
static const char kAttributeGroup[] = "group";
static const char kAttributeMid[] = "mid";
static const char kAttributeRtcpMux[] = "rtcp-mux";
+static const char kAttributeRtcpReducedSize[] = "rtcp-rsize";
static const char kAttributeSsrc[] = "ssrc";
static const char kSsrcAttributeCname[] = "cname";
static const char kAttributeExtmap[] = "extmap";
@@ -1400,6 +1401,13 @@
AddLine(os.str(), message);
}
+ // RFC 5506
+ // a=rtcp-rsize
+ if (media_desc->rtcp_reduced_size()) {
+ InitAttrLine(kAttributeRtcpReducedSize, &os);
+ AddLine(os.str(), message);
+ }
+
// RFC 4568
// a=crypto:<tag> <crypto-suite> <key-params> [<session-params>]
for (std::vector<CryptoParams>::const_iterator it =
@@ -2553,6 +2561,8 @@
//
if (HasAttribute(line, kAttributeRtcpMux)) {
media_desc->set_rtcp_mux(true);
+ } else if (HasAttribute(line, kAttributeRtcpReducedSize)) {
+ media_desc->set_rtcp_reduced_size(true);
} else if (HasAttribute(line, kAttributeSsrcGroup)) {
if (!ParseSsrcGroupAttribute(line, &ssrc_groups, error)) {
return false;
diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc
index cb6a392..fb55e31 100644
--- a/talk/app/webrtc/webrtcsdp_unittest.cc
+++ b/talk/app/webrtc/webrtcsdp_unittest.cc
@@ -153,6 +153,7 @@
"a=mid:audio_content_name\r\n"
"a=sendrecv\r\n"
"a=rtcp-mux\r\n"
+ "a=rtcp-rsize\r\n"
"a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
"inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 "
"dummy_session_params\r\n"
@@ -220,6 +221,7 @@
"a=mid:audio_content_name\r\n"
"a=sendrecv\r\n"
"a=rtcp-mux\r\n"
+ "a=rtcp-rsize\r\n"
"a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
"inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 "
"dummy_session_params\r\n"
@@ -704,6 +706,7 @@
AudioContentDescription* CreateAudioContentDescription() {
AudioContentDescription* audio = new AudioContentDescription();
audio->set_rtcp_mux(true);
+ audio->set_rtcp_reduced_size(true);
StreamParams audio_stream1;
audio_stream1.id = kAudioTrackId1;
audio_stream1.cname = kStream1Cname;
@@ -735,6 +738,9 @@
// rtcp_mux
EXPECT_EQ(cd1->rtcp_mux(), cd2->rtcp_mux());
+ // rtcp_reduced_size
+ EXPECT_EQ(cd1->rtcp_reduced_size(), cd2->rtcp_reduced_size());
+
// cryptos
EXPECT_EQ(cd1->cryptos().size(), cd2->cryptos().size());
if (cd1->cryptos().size() != cd2->cryptos().size()) {
diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h
index 92d0fcb..fe223bb 100644
--- a/talk/media/base/mediachannel.h
+++ b/talk/media/base/mediachannel.h
@@ -927,6 +927,10 @@
std::vector<DataReceiverInfo> receivers;
};
+struct RtcpParameters {
+ bool reduced_size = false;
+};
+
template <class Codec>
struct RtpParameters {
virtual std::string ToString() const {
@@ -941,6 +945,7 @@
std::vector<Codec> codecs;
std::vector<RtpHeaderExtension> extensions;
// TODO(pthatcher): Add streams.
+ RtcpParameters rtcp;
};
template <class Codec, class Options>
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
index 3f9e8ce..6b699fe 100644
--- a/talk/media/webrtc/webrtcvideoengine2.cc
+++ b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -792,10 +792,20 @@
LOG(LS_INFO) << "SetSendParameters: " << params.ToString();
// TODO(pbos): Refactor this to only recreate the send streams once
// instead of 4 times.
- return (SetSendCodecs(params.codecs) &&
- SetSendRtpHeaderExtensions(params.extensions) &&
- SetMaxSendBandwidth(params.max_bandwidth_bps) &&
- SetOptions(params.options));
+ if (!SetSendCodecs(params.codecs) ||
+ !SetSendRtpHeaderExtensions(params.extensions) ||
+ !SetMaxSendBandwidth(params.max_bandwidth_bps) ||
+ !SetOptions(params.options)) {
+ return false;
+ }
+ if (send_params_.rtcp.reduced_size != params.rtcp.reduced_size) {
+ rtc::CritScope stream_lock(&stream_crit_);
+ for (auto& kv : send_streams_) {
+ kv.second->SetSendParameters(params);
+ }
+ }
+ send_params_ = params;
+ return true;
}
bool WebRtcVideoChannel2::SetRecvParameters(const VideoRecvParameters& params) {
@@ -803,8 +813,18 @@
LOG(LS_INFO) << "SetRecvParameters: " << params.ToString();
// TODO(pbos): Refactor this to only recreate the recv streams once
// instead of twice.
- return (SetRecvCodecs(params.codecs) &&
- SetRecvRtpHeaderExtensions(params.extensions));
+ if (!SetRecvCodecs(params.codecs) ||
+ !SetRecvRtpHeaderExtensions(params.extensions)) {
+ return false;
+ }
+ if (recv_params_.rtcp.reduced_size != params.rtcp.reduced_size) {
+ rtc::CritScope stream_lock(&stream_crit_);
+ for (auto& kv : receive_streams_) {
+ kv.second->SetRecvParameters(params);
+ }
+ }
+ recv_params_ = params;
+ return true;
}
std::string WebRtcVideoChannel2::CodecSettingsVectorToString(
@@ -1025,15 +1045,10 @@
webrtc::VideoSendStream::Config config(this);
config.overuse_callback = this;
- WebRtcVideoSendStream* stream =
- new WebRtcVideoSendStream(call_,
- sp,
- config,
- external_encoder_factory_,
- options_,
- bitrate_config_.max_bitrate_bps,
- send_codec_,
- send_rtp_extensions_);
+ WebRtcVideoSendStream* stream = new WebRtcVideoSendStream(
+ call_, sp, config, external_encoder_factory_, options_,
+ bitrate_config_.max_bitrate_bps, send_codec_, send_rtp_extensions_,
+ send_params_);
uint32_t ssrc = sp.first_ssrc();
RTC_DCHECK(ssrc != 0);
@@ -1175,6 +1190,9 @@
config->rtp.local_ssrc = rtcp_receiver_report_ssrc_;
config->rtp.extensions = recv_rtp_extensions_;
+ config->rtp.rtcp_mode = recv_params_.rtcp.reduced_size
+ ? webrtc::RtcpMode::kReducedSize
+ : webrtc::RtcpMode::kCompound;
// TODO(pbos): This protection is against setting the same local ssrc as
// remote which is not permitted by the lower-level API. RTCP requires a
@@ -1661,7 +1679,10 @@
const VideoOptions& options,
int max_bitrate_bps,
const rtc::Optional<VideoCodecSettings>& codec_settings,
- const std::vector<webrtc::RtpExtension>& rtp_extensions)
+ const std::vector<webrtc::RtpExtension>& rtp_extensions,
+ // TODO(deadbeef): Don't duplicate information between send_params,
+ // rtp_extensions, options, etc.
+ const VideoSendParameters& send_params)
: ssrcs_(sp.ssrcs),
ssrc_groups_(sp.ssrc_groups),
call_(call),
@@ -1682,6 +1703,9 @@
¶meters_.config.rtp.rtx.ssrcs);
parameters_.config.rtp.c_name = sp.cname;
parameters_.config.rtp.extensions = rtp_extensions;
+ parameters_.config.rtp.rtcp_mode = send_params.rtcp.reduced_size
+ ? webrtc::RtcpMode::kReducedSize
+ : webrtc::RtcpMode::kCompound;
if (codec_settings) {
SetCodec(*codec_settings);
@@ -1998,6 +2022,18 @@
}
}
+void WebRtcVideoChannel2::WebRtcVideoSendStream::SetSendParameters(
+ const VideoSendParameters& send_params) {
+ rtc::CritScope cs(&lock_);
+ parameters_.config.rtp.rtcp_mode = send_params.rtcp.reduced_size
+ ? webrtc::RtcpMode::kReducedSize
+ : webrtc::RtcpMode::kCompound;
+ if (stream_ != nullptr) {
+ LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetSendParameters";
+ RecreateWebRtcStream();
+ }
+}
+
webrtc::VideoEncoderConfig
WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig(
const Dimensions& dimensions,
@@ -2435,6 +2471,15 @@
RecreateWebRtcStream();
}
+void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(
+ const VideoRecvParameters& recv_params) {
+ config_.rtp.rtcp_mode = recv_params.rtcp.reduced_size
+ ? webrtc::RtcpMode::kReducedSize
+ : webrtc::RtcpMode::kCompound;
+ LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetRecvParameters";
+ RecreateWebRtcStream();
+}
+
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {
if (stream_ != NULL) {
call_->DestroyVideoReceiveStream(stream_);
diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h
index e6e26e7..155790c 100644
--- a/talk/media/webrtc/webrtcvideoengine2.h
+++ b/talk/media/webrtc/webrtcvideoengine2.h
@@ -249,13 +249,17 @@
const VideoOptions& options,
int max_bitrate_bps,
const rtc::Optional<VideoCodecSettings>& codec_settings,
- const std::vector<webrtc::RtpExtension>& rtp_extensions);
+ const std::vector<webrtc::RtpExtension>& rtp_extensions,
+ const VideoSendParameters& send_params);
~WebRtcVideoSendStream();
void SetOptions(const VideoOptions& options);
void SetCodec(const VideoCodecSettings& codec);
void SetRtpExtensions(
const std::vector<webrtc::RtpExtension>& rtp_extensions);
+ // TODO(deadbeef): Move logic from SetCodec/SetRtpExtensions/etc.
+ // into this method. Currently this method only sets the RTCP mode.
+ void SetSendParameters(const VideoSendParameters& send_params);
void InputFrame(VideoCapturer* capturer, const VideoFrame* frame);
bool SetCapturer(VideoCapturer* capturer);
@@ -405,6 +409,9 @@
bool transport_cc_enabled);
void SetRecvCodecs(const std::vector<VideoCodecSettings>& recv_codecs);
void SetRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions);
+ // TODO(deadbeef): Move logic from SetRecvCodecs/SetRtpExtensions/etc.
+ // into this method. Currently this method only sets the RTCP mode.
+ void SetRecvParameters(const VideoRecvParameters& recv_params);
void RenderFrame(const webrtc::VideoFrame& frame,
int time_to_render_ms) override;
@@ -525,6 +532,10 @@
std::vector<webrtc::RtpExtension> recv_rtp_extensions_;
webrtc::Call::Config::BitrateConfig bitrate_config_;
VideoOptions options_;
+ // TODO(deadbeef): Don't duplicate information between
+ // send_params/recv_params, rtp_extensions, options, etc.
+ VideoSendParameters send_params_;
+ VideoRecvParameters recv_params_;
};
} // namespace cricket
diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
index 1de2754..24e028e 100644
--- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
@@ -2418,6 +2418,42 @@
channel_->SetInterface(NULL);
}
+// This test verifies that the RTCP reduced size mode is properly applied to
+// send video streams.
+TEST_F(WebRtcVideoChannel2Test, TestSetSendRtcpReducedSize) {
+ // Create stream, expecting that default mode is "compound".
+ FakeVideoSendStream* stream1 = AddSendStream();
+ EXPECT_EQ(webrtc::RtcpMode::kCompound, stream1->GetConfig().rtp.rtcp_mode);
+
+ // Now enable reduced size mode.
+ send_parameters_.rtcp.reduced_size = true;
+ EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
+ stream1 = fake_call_->GetVideoSendStreams()[0];
+ EXPECT_EQ(webrtc::RtcpMode::kReducedSize, stream1->GetConfig().rtp.rtcp_mode);
+
+ // Create a new stream and ensure it picks up the reduced size mode.
+ FakeVideoSendStream* stream2 = AddSendStream();
+ EXPECT_EQ(webrtc::RtcpMode::kReducedSize, stream2->GetConfig().rtp.rtcp_mode);
+}
+
+// This test verifies that the RTCP reduced size mode is properly applied to
+// receive video streams.
+TEST_F(WebRtcVideoChannel2Test, TestSetRecvRtcpReducedSize) {
+ // Create stream, expecting that default mode is "compound".
+ FakeVideoReceiveStream* stream1 = AddRecvStream();
+ EXPECT_EQ(webrtc::RtcpMode::kCompound, stream1->GetConfig().rtp.rtcp_mode);
+
+ // Now enable reduced size mode.
+ recv_parameters_.rtcp.reduced_size = true;
+ EXPECT_TRUE(channel_->SetRecvParameters(recv_parameters_));
+ stream1 = fake_call_->GetVideoReceiveStreams()[0];
+ EXPECT_EQ(webrtc::RtcpMode::kReducedSize, stream1->GetConfig().rtp.rtcp_mode);
+
+ // Create a new stream and ensure it picks up the reduced size mode.
+ FakeVideoReceiveStream* stream2 = AddRecvStream();
+ EXPECT_EQ(webrtc::RtcpMode::kReducedSize, stream2->GetConfig().rtp.rtcp_mode);
+}
+
TEST_F(WebRtcVideoChannel2Test, OnReadyToSendSignalsNetworkState) {
EXPECT_EQ(webrtc::kNetworkUp, fake_call_->GetNetworkState());
diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc
index f4c0e81..588a036 100644
--- a/talk/session/media/channel.cc
+++ b/talk/session/media/channel.cc
@@ -152,6 +152,7 @@
if (desc->rtp_header_extensions_set()) {
params->extensions = desc->rtp_header_extensions();
}
+ params->rtcp.reduced_size = desc->rtcp_reduced_size();
}
template <class Codec, class Options>
diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc
index 0e9730c..8db37d0 100644
--- a/talk/session/media/mediasession.cc
+++ b/talk/session/media/mediasession.cc
@@ -760,6 +760,11 @@
offer->set_crypto_required(CT_SDES);
}
offer->set_rtcp_mux(options.rtcp_mux_enabled);
+ // TODO(deadbeef): Once we're sure this works correctly, enable it in
+ // CreateOffer.
+ // if (offer->type() == cricket::MEDIA_TYPE_VIDEO) {
+ // offer->set_rtcp_reduced_size(true);
+ // }
offer->set_multistream(options.is_muc);
offer->set_rtp_header_extensions(rtp_extensions);
@@ -1038,6 +1043,11 @@
answer->set_rtp_header_extensions(negotiated_rtp_extensions);
answer->set_rtcp_mux(options.rtcp_mux_enabled && offer->rtcp_mux());
+ // TODO(deadbeef): Once we're sure this works correctly, enable it in
+ // CreateAnswer.
+ // if (answer->type() == cricket::MEDIA_TYPE_VIDEO) {
+ // answer->set_rtcp_reduced_size(offer->rtcp_reduced_size());
+ // }
if (sdes_policy != SEC_DISABLED) {
CryptoParams crypto;
diff --git a/talk/session/media/mediasession.h b/talk/session/media/mediasession.h
index 25ea017..8a7f4cc 100644
--- a/talk/session/media/mediasession.h
+++ b/talk/session/media/mediasession.h
@@ -167,17 +167,7 @@
// "content" (as used in XEP-0166) descriptions for voice and video.
class MediaContentDescription : public ContentDescription {
public:
- MediaContentDescription()
- : rtcp_mux_(false),
- bandwidth_(kAutoBandwidth),
- crypto_required_(CT_NONE),
- rtp_header_extensions_set_(false),
- multistream_(false),
- conference_mode_(false),
- partial_(false),
- buffered_mode_latency_(kBufferedModeDisabled),
- direction_(MD_SENDRECV) {
- }
+ MediaContentDescription() {}
virtual MediaType type() const = 0;
virtual bool has_codecs() const = 0;
@@ -195,6 +185,11 @@
bool rtcp_mux() const { return rtcp_mux_; }
void set_rtcp_mux(bool mux) { rtcp_mux_ = mux; }
+ bool rtcp_reduced_size() const { return rtcp_reduced_size_; }
+ void set_rtcp_reduced_size(bool reduced_size) {
+ rtcp_reduced_size_ = reduced_size;
+ }
+
int bandwidth() const { return bandwidth_; }
void set_bandwidth(int bandwidth) { bandwidth_ = bandwidth; }
@@ -291,19 +286,20 @@
int buffered_mode_latency() const { return buffered_mode_latency_; }
protected:
- bool rtcp_mux_;
- int bandwidth_;
+ bool rtcp_mux_ = false;
+ bool rtcp_reduced_size_ = false;
+ int bandwidth_ = kAutoBandwidth;
std::string protocol_;
std::vector<CryptoParams> cryptos_;
- CryptoType crypto_required_;
+ CryptoType crypto_required_ = CT_NONE;
std::vector<RtpHeaderExtension> rtp_header_extensions_;
- bool rtp_header_extensions_set_;
- bool multistream_;
+ bool rtp_header_extensions_set_ = false;
+ bool multistream_ = false;
StreamParamsVec streams_;
- bool conference_mode_;
- bool partial_;
- int buffered_mode_latency_;
- MediaContentDirection direction_;
+ bool conference_mode_ = false;
+ bool partial_ = false;
+ int buffered_mode_latency_ = kBufferedModeDisabled;
+ MediaContentDirection direction_ = MD_SENDRECV;
};
template <class C>
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index d73ef9c..656d551 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -516,7 +516,7 @@
// When it goes down, disable RTCP afterwards. This ensures that any packets
// sent due to the network state changed will not be dropped.
if (state == kNetworkUp)
- vie_channel_->SetRTCPMode(RtcpMode::kCompound);
+ vie_channel_->SetRTCPMode(config_.rtp.rtcp_mode);
vie_encoder_->SetNetworkTransmissionState(state == kNetworkUp);
if (state == kNetworkDown)
vie_channel_->SetRTCPMode(RtcpMode::kOff);
diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h
index 0c0af80..d0e0c93 100644
--- a/webrtc/video_send_stream.h
+++ b/webrtc/video_send_stream.h
@@ -100,6 +100,9 @@
std::vector<uint32_t> ssrcs;
+ // See RtcpMode for description.
+ RtcpMode rtcp_mode = RtcpMode::kCompound;
+
// Max RTP packet size delivered to send transport from VideoEngine.
size_t max_packet_size = kDefaultMaxPacketSize;
