From bcd39d483db751aec0806262307f4db76d0a9f07 Mon Sep 17 00:00:00 2001 From: Amit Hilbuch Date: Fri, 25 Jan 2019 17:13:56 -0800 Subject: [PATCH] Creating Simulcast offer and answer in Peer Connection. CreateOffer and CreateAnswer will now examine the layers on the transceiver to determine if multiple layers are requested (Simulcast). In this scenario RIDs will be used in the layers (instead of SSRCs). When the offer is created, only RIDs are signalled in the offer. When the offer is set locally SetLocalDescription() SSRCs will be generated for each layer by the Channel and sent downstream to the MediaChannel. The MediaChannel receives configuration that looks identical to that of legacy simulcast, and should be able to integrate the streams correctly regardless of how they were signalled. Setting multiple layers on the transciever is still not supported through the API. Bug: webrtc:10075 Change-Id: Id4ad3637b87b68ef6ca7eec69166fee2d9dfa36f Reviewed-on: https://webrtc-review.googlesource.com/c/119780 Reviewed-by: Seth Hampson Reviewed-by: Steve Anton Commit-Queue: Amit Hilbuch Cr-Commit-Position: refs/heads/master@{#26428} --- media/base/stream_params.cc | 32 ++++ media/base/stream_params.h | 9 ++ media/base/stream_params_unittest.cc | 73 +++++++++ pc/channel.cc | 145 +++++++++++++---- pc/channel.h | 25 ++- pc/channel_interface.h | 5 + pc/channel_manager.cc | 28 ++-- pc/channel_manager.h | 5 +- pc/channel_manager_unittest.cc | 8 +- pc/channel_unittest.cc | 82 ++++++++-- pc/media_session.cc | 105 ++++++------ pc/media_session.h | 19 ++- pc/media_session_unittest.cc | 29 +++- pc/peer_connection.cc | 195 ++++++++++++++++++++--- pc/peer_connection.h | 6 + pc/rtp_sender_receiver_unittest.cc | 7 +- pc/test/fake_peer_connection_for_stats.h | 8 +- pc/test/mock_channel_interface.h | 3 + pc/webrtc_session_description_factory.cc | 8 +- pc/webrtc_session_description_factory.h | 4 +- 20 files changed, 625 insertions(+), 171 deletions(-) diff --git a/media/base/stream_params.cc b/media/base/stream_params.cc index f89e56537d..329aa30574 100644 --- a/media/base/stream_params.cc +++ b/media/base/stream_params.cc @@ -195,6 +195,38 @@ std::string StreamParams::ToString() const { sb << "}"; return sb.str(); } + +void StreamParams::GenerateSsrcs(int num_layers, + bool generate_fid, + bool generate_fec_fr, + rtc::UniqueRandomIdGenerator* ssrc_generator) { + RTC_DCHECK_GE(num_layers, 0); + RTC_DCHECK(ssrc_generator); + std::vector primary_ssrcs; + for (int i = 0; i < num_layers; ++i) { + uint32_t ssrc = ssrc_generator->GenerateId(); + primary_ssrcs.push_back(ssrc); + add_ssrc(ssrc); + } + + if (num_layers > 1) { + SsrcGroup simulcast(kSimSsrcGroupSemantics, primary_ssrcs); + ssrc_groups.push_back(simulcast); + } + + if (generate_fid) { + for (uint32_t ssrc : primary_ssrcs) { + AddFidSsrc(ssrc, ssrc_generator->GenerateId()); + } + } + + if (generate_fec_fr) { + for (uint32_t ssrc : primary_ssrcs) { + AddFecFrSsrc(ssrc, ssrc_generator->GenerateId()); + } + } +} + void StreamParams::GetPrimarySsrcs(std::vector* ssrcs) const { const SsrcGroup* sim_group = get_ssrc_group(kSimSsrcGroupSemantics); if (sim_group == NULL) { diff --git a/media/base/stream_params.h b/media/base/stream_params.h index b53e04772c..de567d0b4d 100644 --- a/media/base/stream_params.h +++ b/media/base/stream_params.h @@ -54,6 +54,7 @@ #include "media/base/rid_description.h" #include "rtc_base/constructor_magic.h" +#include "rtc_base/unique_id_generator.h" namespace cricket { @@ -154,6 +155,14 @@ struct StreamParams { return GetSecondarySsrc(kFecFrSsrcGroupSemantics, primary_ssrc, fecfr_ssrc); } + // Convenience function to populate the StreamParams with the requested number + // of SSRCs along with accompanying FID and FEC-FR ssrcs if requested. + // SSRCs are generated using the given generator. + void GenerateSsrcs(int num_layers, + bool generate_fid, + bool generate_fec_fr, + rtc::UniqueRandomIdGenerator* ssrc_generator); + // Convenience to get all the SIM SSRCs if there are SIM ssrcs, or // the first SSRC otherwise. void GetPrimarySsrcs(std::vector* ssrcs) const; diff --git a/media/base/stream_params_unittest.cc b/media/base/stream_params_unittest.cc index f0041332c8..53d355af6b 100644 --- a/media/base/stream_params_unittest.cc +++ b/media/base/stream_params_unittest.cc @@ -14,8 +14,12 @@ #include "media/base/test_utils.h" #include "rtc_base/arraysize.h" +#include "test/gmock.h" #include "test/gtest.h" +using ::testing::Each; +using ::testing::Ne; + static const uint32_t kSsrcs1[] = {1}; static const uint32_t kSsrcs2[] = {1, 2}; static const uint32_t kSsrcs3[] = {1, 2, 3}; @@ -321,3 +325,72 @@ TEST(StreamParams, TestIsSimulcastStream_InvalidStreams) { stream3.ssrc_groups.push_back(sg); EXPECT_FALSE(cricket::IsSimulcastStream(stream3)); } + +TEST(StreamParams, TestGenerateSsrcs_SingleStreamWithRtxAndFlex) { + rtc::UniqueRandomIdGenerator generator; + cricket::StreamParams stream; + stream.GenerateSsrcs(1, true, true, &generator); + uint32_t primary_ssrc = stream.first_ssrc(); + ASSERT_NE(0u, primary_ssrc); + uint32_t rtx_ssrc = 0; + uint32_t flex_ssrc = 0; + EXPECT_EQ(3u, stream.ssrcs.size()); + EXPECT_TRUE(stream.GetFidSsrc(primary_ssrc, &rtx_ssrc)); + EXPECT_NE(0u, rtx_ssrc); + EXPECT_TRUE(stream.GetFecFrSsrc(primary_ssrc, &flex_ssrc)); + EXPECT_NE(0u, flex_ssrc); + EXPECT_FALSE(stream.has_ssrc_group(cricket::kSimSsrcGroupSemantics)); + EXPECT_TRUE(stream.has_ssrc_group(cricket::kFidSsrcGroupSemantics)); + EXPECT_TRUE(stream.has_ssrc_group(cricket::kFecFrSsrcGroupSemantics)); +} + +TEST(StreamParams, TestGenerateSsrcs_SingleStreamWithRtx) { + rtc::UniqueRandomIdGenerator generator; + cricket::StreamParams stream; + stream.GenerateSsrcs(1, true, false, &generator); + uint32_t primary_ssrc = stream.first_ssrc(); + ASSERT_NE(0u, primary_ssrc); + uint32_t rtx_ssrc = 0; + uint32_t flex_ssrc = 0; + EXPECT_EQ(2u, stream.ssrcs.size()); + EXPECT_TRUE(stream.GetFidSsrc(primary_ssrc, &rtx_ssrc)); + EXPECT_NE(0u, rtx_ssrc); + EXPECT_FALSE(stream.GetFecFrSsrc(primary_ssrc, &flex_ssrc)); + EXPECT_EQ(0u, flex_ssrc); + EXPECT_FALSE(stream.has_ssrc_group(cricket::kSimSsrcGroupSemantics)); + EXPECT_TRUE(stream.has_ssrc_group(cricket::kFidSsrcGroupSemantics)); +} + +TEST(StreamParams, TestGenerateSsrcs_SingleStreamWithFlex) { + rtc::UniqueRandomIdGenerator generator; + cricket::StreamParams stream; + stream.GenerateSsrcs(1, false, true, &generator); + uint32_t primary_ssrc = stream.first_ssrc(); + ASSERT_NE(0u, primary_ssrc); + uint32_t rtx_ssrc = 0; + uint32_t flex_ssrc = 0; + EXPECT_EQ(2u, stream.ssrcs.size()); + EXPECT_FALSE(stream.GetFidSsrc(primary_ssrc, &rtx_ssrc)); + EXPECT_EQ(0u, rtx_ssrc); + EXPECT_TRUE(stream.GetFecFrSsrc(primary_ssrc, &flex_ssrc)); + EXPECT_NE(0u, flex_ssrc); + EXPECT_FALSE(stream.has_ssrc_group(cricket::kSimSsrcGroupSemantics)); + EXPECT_TRUE(stream.has_ssrc_group(cricket::kFecFrSsrcGroupSemantics)); +} + +TEST(StreamParams, TestGenerateSsrcs_SimulcastLayersAndRtx) { + const size_t kNumStreams = 3; + rtc::UniqueRandomIdGenerator generator; + cricket::StreamParams stream; + stream.GenerateSsrcs(kNumStreams, true, false, &generator); + EXPECT_EQ(kNumStreams * 2, stream.ssrcs.size()); + std::vector primary_ssrcs, rtx_ssrcs; + stream.GetPrimarySsrcs(&primary_ssrcs); + EXPECT_EQ(kNumStreams, primary_ssrcs.size()); + EXPECT_THAT(primary_ssrcs, Each(Ne(0u))); + stream.GetFidSsrcs(primary_ssrcs, &rtx_ssrcs); + EXPECT_EQ(kNumStreams, rtx_ssrcs.size()); + EXPECT_THAT(rtx_ssrcs, Each(Ne(0u))); + EXPECT_TRUE(stream.has_ssrc_group(cricket::kSimSsrcGroupSemantics)); + EXPECT_TRUE(stream.has_ssrc_group(cricket::kFidSsrcGroupSemantics)); +} diff --git a/pc/channel.cc b/pc/channel.cc index d26a5113d4..049e9506ce 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -37,6 +37,7 @@ namespace cricket { using rtc::Bind; +using rtc::UniqueRandomIdGenerator; using webrtc::SdpType; namespace { @@ -46,6 +47,39 @@ struct SendPacketMessageData : public rtc::MessageData { rtc::PacketOptions options; }; +// Finds a stream based on target's Primary SSRC or RIDs. +// This struct is used in BaseChannel::UpdateLocalStreams_w. +struct StreamFinder { + explicit StreamFinder(const StreamParams* target) : target_(target) { + RTC_DCHECK(target); + } + + bool operator()(const StreamParams& sp) const { + if (target_->has_ssrcs() && sp.has_ssrcs()) { + return sp.has_ssrc(target_->first_ssrc()); + } + + if (!target_->has_rids() && !sp.has_rids()) { + return false; + } + + const std::vector& target_rids = target_->rids(); + const std::vector& source_rids = sp.rids(); + if (source_rids.size() != target_rids.size()) { + return false; + } + + // Check that all RIDs match. + return std::equal(source_rids.begin(), source_rids.end(), + target_rids.begin(), + [](const RidDescription& lhs, const RidDescription& rhs) { + return lhs.rid == rhs.rid; + }); + } + + const StreamParams* target_; +}; + } // namespace enum { @@ -102,15 +136,18 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, std::unique_ptr media_channel, const std::string& content_name, bool srtp_required, - webrtc::CryptoOptions crypto_options) + webrtc::CryptoOptions crypto_options, + UniqueRandomIdGenerator* ssrc_generator) : worker_thread_(worker_thread), network_thread_(network_thread), signaling_thread_(signaling_thread), content_name_(content_name), srtp_required_(srtp_required), crypto_options_(crypto_options), - media_channel_(std::move(media_channel)) { + media_channel_(std::move(media_channel)), + ssrc_generator_(ssrc_generator) { RTC_DCHECK_RUN_ON(worker_thread_); + RTC_DCHECK(ssrc_generator_); demuxer_criteria_.mid = content_name; RTC_LOG(LS_INFO) << "Created channel for " << content_name; } @@ -581,35 +618,77 @@ bool BaseChannel::RemoveRecvStream_w(uint32_t ssrc) { bool BaseChannel::UpdateLocalStreams_w(const std::vector& streams, SdpType type, std::string* error_desc) { + // In the case of RIDs (where SSRCs are not negotiated), this method will + // generate an SSRC for each layer in StreamParams. That representation will + // be stored internally in |local_streams_|. + // In subsequent offers, the same stream can appear in |streams| again + // (without the SSRCs), so it should be looked up using RIDs (if available) + // and then by primary SSRC. + // In both scenarios, it is safe to assume that the media channel will be + // created with a StreamParams object with SSRCs. However, it is not safe to + // assume that |local_streams_| will always have SSRCs as there are scenarios + // in which niether SSRCs or RIDs are negotiated. + // Check for streams that have been removed. bool ret = true; for (const StreamParams& old_stream : local_streams_) { - if (old_stream.has_ssrcs() && - !GetStreamBySsrc(streams, old_stream.first_ssrc())) { - if (!media_channel()->RemoveSendStream(old_stream.first_ssrc())) { - rtc::StringBuilder desc; - desc << "Failed to remove send stream with ssrc " - << old_stream.first_ssrc() << "."; - SafeSetError(desc.str(), error_desc); - ret = false; - } + if (!old_stream.has_ssrcs() || + GetStream(streams, StreamFinder(&old_stream))) { + continue; + } + if (!media_channel()->RemoveSendStream(old_stream.first_ssrc())) { + rtc::StringBuilder desc; + desc << "Failed to remove send stream with ssrc " + << old_stream.first_ssrc() << "."; + SafeSetError(desc.str(), error_desc); + ret = false; } } // Check for new streams. - for (const StreamParams& new_stream : streams) { - if (new_stream.has_ssrcs() && - !GetStreamBySsrc(local_streams_, new_stream.first_ssrc())) { - if (media_channel()->AddSendStream(new_stream)) { - RTC_LOG(LS_INFO) << "Add send stream ssrc: " << new_stream.ssrcs[0]; - } else { - rtc::StringBuilder desc; - desc << "Failed to add send stream ssrc: " << new_stream.first_ssrc(); - SafeSetError(desc.str(), error_desc); - ret = false; - } + std::vector all_streams; + for (const StreamParams& stream : streams) { + StreamParams* existing = GetStream(local_streams_, StreamFinder(&stream)); + if (existing) { + // Parameters cannot change for an existing stream. + all_streams.push_back(*existing); + continue; + } + + all_streams.push_back(stream); + StreamParams& new_stream = all_streams.back(); + + if (!new_stream.has_ssrcs() && !new_stream.has_rids()) { + continue; + } + + RTC_DCHECK(new_stream.has_ssrcs() || new_stream.has_rids()); + if (new_stream.has_ssrcs() && new_stream.has_rids()) { + rtc::StringBuilder desc; + desc << "Failed to add send stream: " << new_stream.first_ssrc() + << ". Stream has both SSRCs and RIDs."; + SafeSetError(desc.str(), error_desc); + ret = false; + continue; + } + + // At this point we use the legacy simulcast group in StreamParams to + // indicate that we want multiple layers to the media channel. + if (!new_stream.has_ssrcs()) { + // TODO(bugs.webrtc.org/10250): Indicate if flex is desired here. + new_stream.GenerateSsrcs(new_stream.rids().size(), /* rtx = */ true, + /* flex_fec = */ false, ssrc_generator_); + } + + if (media_channel()->AddSendStream(new_stream)) { + RTC_LOG(LS_INFO) << "Add send stream ssrc: " << new_stream.ssrcs[0]; + } else { + rtc::StringBuilder desc; + desc << "Failed to add send stream ssrc: " << new_stream.first_ssrc(); + SafeSetError(desc.str(), error_desc); + ret = false; } } - local_streams_ = streams; + local_streams_ = all_streams; return ret; } @@ -729,19 +808,19 @@ void BaseChannel::SignalSentPacket_w(const rtc::SentPacket& sent_packet) { VoiceChannel::VoiceChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, - // TODO(nisse): Delete unused argument. - MediaEngineInterface* /* media_engine */, std::unique_ptr media_channel, const std::string& content_name, bool srtp_required, - webrtc::CryptoOptions crypto_options) + webrtc::CryptoOptions crypto_options, + UniqueRandomIdGenerator* ssrc_generator) : BaseChannel(worker_thread, network_thread, signaling_thread, std::move(media_channel), content_name, srtp_required, - crypto_options) {} + crypto_options, + ssrc_generator) {} VoiceChannel::~VoiceChannel() { if (media_transport()) { @@ -895,14 +974,16 @@ VideoChannel::VideoChannel(rtc::Thread* worker_thread, std::unique_ptr media_channel, const std::string& content_name, bool srtp_required, - webrtc::CryptoOptions crypto_options) + webrtc::CryptoOptions crypto_options, + UniqueRandomIdGenerator* ssrc_generator) : BaseChannel(worker_thread, network_thread, signaling_thread, std::move(media_channel), content_name, srtp_required, - crypto_options) {} + crypto_options, + ssrc_generator) {} VideoChannel::~VideoChannel() { TRACE_EVENT0("webrtc", "VideoChannel::~VideoChannel"); @@ -1034,14 +1115,16 @@ RtpDataChannel::RtpDataChannel(rtc::Thread* worker_thread, std::unique_ptr media_channel, const std::string& content_name, bool srtp_required, - webrtc::CryptoOptions crypto_options) + webrtc::CryptoOptions crypto_options, + UniqueRandomIdGenerator* ssrc_generator) : BaseChannel(worker_thread, network_thread, signaling_thread, std::move(media_channel), content_name, srtp_required, - crypto_options) {} + crypto_options, + ssrc_generator) {} RtpDataChannel::~RtpDataChannel() { TRACE_EVENT0("webrtc", "RtpDataChannel::~RtpDataChannel"); diff --git a/pc/channel.h b/pc/channel.h index 227c593312..1a4cc72201 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -41,6 +41,7 @@ #include "rtc_base/critical_section.h" #include "rtc_base/network.h" #include "rtc_base/third_party/sigslot/sigslot.h" +#include "rtc_base/unique_id_generator.h" namespace webrtc { class AudioSinkInterface; @@ -78,6 +79,8 @@ class BaseChannel : public ChannelInterface, 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). + // The BaseChannel does not own the UniqueRandomIdGenerator so it is the + // responsibility of the user to ensure it outlives this object. // TODO(zhihuang:) Create a BaseChannel::Config struct for the parameter lists // which will make it easier to change the constructor. BaseChannel(rtc::Thread* worker_thread, @@ -86,7 +89,8 @@ class BaseChannel : public ChannelInterface, std::unique_ptr media_channel, const std::string& content_name, bool srtp_required, - webrtc::CryptoOptions crypto_options); + webrtc::CryptoOptions crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator); virtual ~BaseChannel(); virtual void Init_w(webrtc::RtpTransportInternal* rtp_transport, webrtc::MediaTransportInterface* media_transport); @@ -125,10 +129,10 @@ class BaseChannel : public ChannelInterface, bool Enable(bool enable) override; - const std::vector& local_streams() const { + const std::vector& local_streams() const override { return local_streams_; } - const std::vector& remote_streams() const { + const std::vector& remote_streams() const override { return remote_streams_; } @@ -345,6 +349,11 @@ class BaseChannel : public ChannelInterface, webrtc::RtpTransceiverDirection::kInactive; webrtc::RtpDemuxerCriteria demuxer_criteria_; + // This generator is used to generate SSRCs for local streams. + // This is needed in cases where SSRCs are not negotiated or set explicitly + // like in Simulcast. + // This object is not owned by the channel so it must outlive it. + rtc::UniqueRandomIdGenerator* const ssrc_generator_; }; // VoiceChannel is a specialization that adds support for early media, DTMF, @@ -355,11 +364,11 @@ class VoiceChannel : public BaseChannel, VoiceChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, - MediaEngineInterface* media_engine, std::unique_ptr channel, const std::string& content_name, bool srtp_required, - webrtc::CryptoOptions crypto_options); + webrtc::CryptoOptions crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator); ~VoiceChannel(); // downcasts a MediaChannel @@ -402,7 +411,8 @@ class VideoChannel : public BaseChannel { std::unique_ptr media_channel, const std::string& content_name, bool srtp_required, - webrtc::CryptoOptions crypto_options); + webrtc::CryptoOptions crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator); ~VideoChannel(); // downcasts a MediaChannel @@ -443,7 +453,8 @@ class RtpDataChannel : public BaseChannel { std::unique_ptr channel, const std::string& content_name, bool srtp_required, - webrtc::CryptoOptions crypto_options); + webrtc::CryptoOptions crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator); ~RtpDataChannel(); // TODO(zhihuang): Remove this once the RtpTransport can be shared between // BaseChannels. diff --git a/pc/channel_interface.h b/pc/channel_interface.h index b623254b1b..cd29ed4f84 100644 --- a/pc/channel_interface.h +++ b/pc/channel_interface.h @@ -12,6 +12,7 @@ #define PC_CHANNEL_INTERFACE_H_ #include +#include #include "api/jsep.h" #include "api/media_types.h" @@ -52,6 +53,10 @@ class ChannelInterface { webrtc::SdpType type, std::string* error_desc) = 0; + // Access to the local and remote streams that were set on the channel. + virtual const std::vector& local_streams() const = 0; + virtual const std::vector& remote_streams() const = 0; + // Set an RTP level transport. // Some examples: // * An RtpTransport without encryption. diff --git a/pc/channel_manager.cc b/pc/channel_manager.cc index c936caf16b..f5798a253d 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc @@ -163,12 +163,13 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( const std::string& content_name, bool srtp_required, const webrtc::CryptoOptions& crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator, const AudioOptions& options) { if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - return CreateVoiceChannel(call, media_config, rtp_transport, - media_transport, signaling_thread, content_name, - srtp_required, crypto_options, options); + return CreateVoiceChannel( + call, media_config, rtp_transport, media_transport, signaling_thread, + content_name, srtp_required, crypto_options, ssrc_generator, options); }); } @@ -186,9 +187,9 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( } auto voice_channel = absl::make_unique( - worker_thread_, network_thread_, signaling_thread, media_engine_.get(), + worker_thread_, network_thread_, signaling_thread, absl::WrapUnique(media_channel), content_name, srtp_required, - crypto_options); + crypto_options, ssrc_generator); voice_channel->Init_w(rtp_transport, media_transport); @@ -231,12 +232,13 @@ VideoChannel* ChannelManager::CreateVideoChannel( const std::string& content_name, bool srtp_required, const webrtc::CryptoOptions& crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator, const VideoOptions& options) { if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - return CreateVideoChannel(call, media_config, rtp_transport, - media_transport, signaling_thread, content_name, - srtp_required, crypto_options, options); + return CreateVideoChannel( + call, media_config, rtp_transport, media_transport, signaling_thread, + content_name, srtp_required, crypto_options, ssrc_generator, options); }); } @@ -256,7 +258,7 @@ VideoChannel* ChannelManager::CreateVideoChannel( auto video_channel = absl::make_unique( worker_thread_, network_thread_, signaling_thread, absl::WrapUnique(media_channel), content_name, srtp_required, - crypto_options); + crypto_options, ssrc_generator); video_channel->Init_w(rtp_transport, media_transport); @@ -296,11 +298,13 @@ RtpDataChannel* ChannelManager::CreateRtpDataChannel( rtc::Thread* signaling_thread, const std::string& content_name, bool srtp_required, - const webrtc::CryptoOptions& crypto_options) { + const webrtc::CryptoOptions& crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator) { if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke(RTC_FROM_HERE, [&] { return CreateRtpDataChannel(media_config, rtp_transport, signaling_thread, - content_name, srtp_required, crypto_options); + content_name, srtp_required, crypto_options, + ssrc_generator); }); } @@ -315,7 +319,7 @@ RtpDataChannel* ChannelManager::CreateRtpDataChannel( auto data_channel = absl::make_unique( worker_thread_, network_thread_, signaling_thread, absl::WrapUnique(media_channel), content_name, srtp_required, - crypto_options); + crypto_options, ssrc_generator); data_channel->Init_w(rtp_transport); RtpDataChannel* data_channel_ptr = data_channel.get(); diff --git a/pc/channel_manager.h b/pc/channel_manager.h index 38d79ee878..b70ed2e3cd 100644 --- a/pc/channel_manager.h +++ b/pc/channel_manager.h @@ -100,6 +100,7 @@ class ChannelManager final { const std::string& content_name, bool srtp_required, const webrtc::CryptoOptions& crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator, const AudioOptions& options); // Destroys a voice channel created by CreateVoiceChannel. void DestroyVoiceChannel(VoiceChannel* voice_channel); @@ -116,6 +117,7 @@ class ChannelManager final { const std::string& content_name, bool srtp_required, const webrtc::CryptoOptions& crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator, const VideoOptions& options); // Destroys a video channel created by CreateVideoChannel. void DestroyVideoChannel(VideoChannel* video_channel); @@ -126,7 +128,8 @@ class ChannelManager final { rtc::Thread* signaling_thread, const std::string& content_name, bool srtp_required, - const webrtc::CryptoOptions& crypto_options); + const webrtc::CryptoOptions& crypto_options, + rtc::UniqueRandomIdGenerator* ssrc_generator); // Destroys a data channel created by CreateRtpDataChannel. void DestroyRtpDataChannel(RtpDataChannel* data_channel); diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index 999e36eff0..36b798181b 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -83,16 +83,17 @@ class ChannelManagerTest : public testing::Test { cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( &fake_call_, cricket::MediaConfig(), rtp_transport, media_transport, rtc::Thread::Current(), cricket::CN_AUDIO, kDefaultSrtpRequired, - webrtc::CryptoOptions(), AudioOptions()); + webrtc::CryptoOptions(), &ssrc_generator_, AudioOptions()); EXPECT_TRUE(voice_channel != nullptr); cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( &fake_call_, cricket::MediaConfig(), rtp_transport, media_transport, rtc::Thread::Current(), cricket::CN_VIDEO, kDefaultSrtpRequired, - webrtc::CryptoOptions(), VideoOptions()); + webrtc::CryptoOptions(), &ssrc_generator_, VideoOptions()); EXPECT_TRUE(video_channel != nullptr); cricket::RtpDataChannel* rtp_data_channel = cm_->CreateRtpDataChannel( cricket::MediaConfig(), rtp_transport, rtc::Thread::Current(), - cricket::CN_DATA, kDefaultSrtpRequired, webrtc::CryptoOptions()); + cricket::CN_DATA, kDefaultSrtpRequired, webrtc::CryptoOptions(), + &ssrc_generator_); EXPECT_TRUE(rtp_data_channel != nullptr); cm_->DestroyVideoChannel(video_channel); cm_->DestroyVoiceChannel(voice_channel); @@ -109,6 +110,7 @@ class ChannelManagerTest : public testing::Test { std::unique_ptr cm_; cricket::FakeCall fake_call_; webrtc::FakeMediaTransportFactory fake_media_transport_factory_; + rtc::UniqueRandomIdGenerator ssrc_generator_; }; // Test that we startup/shutdown properly. diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 1ead1359b0..6e3aa194f2 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -39,6 +39,8 @@ using cricket::DtlsTransportInternal; using cricket::FakeVoiceMediaChannel; +using cricket::RidDescription; +using cricket::RidDirection; using cricket::StreamParams; using webrtc::RtpTransceiverDirection; using webrtc::SdpType; @@ -223,10 +225,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { fake_rtp_dtls_transport2_.get(), fake_rtcp_dtls_transport2_.get(), flags2); - channel1_ = CreateChannel(worker_thread, network_thread_, &media_engine_, - std::move(ch1), rtp_transport1_.get(), flags1); - channel2_ = CreateChannel(worker_thread, network_thread_, &media_engine_, - std::move(ch2), rtp_transport2_.get(), flags2); + channel1_ = CreateChannel(worker_thread, network_thread_, std::move(ch1), + rtp_transport1_.get(), flags1); + channel2_ = CreateChannel(worker_thread, network_thread_, std::move(ch2), + rtp_transport2_.get(), flags2); channel1_->SignalRtcpMuxFullyActive.connect( this, &ChannelTest::OnRtcpMuxFullyActive1); channel2_->SignalRtcpMuxFullyActive.connect( @@ -253,14 +255,14 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { std::unique_ptr CreateChannel( rtc::Thread* worker_thread, rtc::Thread* network_thread, - cricket::MediaEngineInterface* engine, std::unique_ptr ch, webrtc::RtpTransportInternal* rtp_transport, int flags) { rtc::Thread* signaling_thread = rtc::Thread::Current(); auto channel = absl::make_unique( - worker_thread, network_thread, signaling_thread, engine, std::move(ch), - cricket::CN_AUDIO, (flags & DTLS) != 0, webrtc::CryptoOptions()); + worker_thread, network_thread, signaling_thread, std::move(ch), + cricket::CN_AUDIO, (flags & DTLS) != 0, webrtc::CryptoOptions(), + &ssrc_generator_); channel->Init_w(rtp_transport, /*media_transport=*/nullptr); return channel; } @@ -1434,6 +1436,59 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_EQ(kRcvBufSize, option_val); } + void CreateSimulcastContent(const std::vector& rids, + typename T::Content* content) { + std::vector rid_descriptions; + for (const std::string name : rids) { + rid_descriptions.push_back(RidDescription(name, RidDirection::kSend)); + } + + StreamParams stream; + stream.set_rids(rid_descriptions); + CreateContent(0, kPcmuCodec, kH264Codec, content); + // This is for unified plan, so there can be only one StreamParams. + content->mutable_streams().clear(); + content->AddStream(stream); + } + + void VerifySimulcastStreamParams(const StreamParams& expected, + const typename T::Channel* channel) { + const std::vector& streams = channel->local_streams(); + ASSERT_EQ(1u, streams.size()); + const StreamParams& result = streams[0]; + EXPECT_EQ(expected.rids(), result.rids()); + EXPECT_TRUE(result.has_ssrcs()); + EXPECT_EQ(expected.rids().size() * 2, result.ssrcs.size()); + std::vector primary_ssrcs; + result.GetPrimarySsrcs(&primary_ssrcs); + EXPECT_EQ(expected.rids().size(), primary_ssrcs.size()); + } + + void TestUpdateLocalStreamsWithSimulcast() { + CreateChannels(0, 0); + typename T::Content content1, content2, content3; + CreateSimulcastContent({"f", "h", "q"}, &content1); + EXPECT_TRUE( + channel1_->SetLocalContent(&content1, SdpType::kOffer, nullptr)); + VerifySimulcastStreamParams(content1.streams()[0], channel1_.get()); + StreamParams stream1 = channel1_->local_streams()[0]; + + // Create a similar offer. SetLocalContent should not remove and add. + CreateSimulcastContent({"f", "h", "q"}, &content2); + EXPECT_TRUE( + channel1_->SetLocalContent(&content2, SdpType::kOffer, nullptr)); + VerifySimulcastStreamParams(content2.streams()[0], channel1_.get()); + StreamParams stream2 = channel1_->local_streams()[0]; + // Check that the streams are identical (SSRCs didn't change). + EXPECT_EQ(stream1, stream2); + + // Create third offer that has same RIDs in different order. + CreateSimulcastContent({"f", "q", "h"}, &content3); + EXPECT_TRUE( + channel1_->SetLocalContent(&content3, SdpType::kOffer, nullptr)); + VerifySimulcastStreamParams(content3.streams()[0], channel1_.get()); + } + protected: void WaitForThreads() { WaitForThreads(rtc::ArrayView()); } static void ProcessThreadQueue(rtc::Thread* thread) { @@ -1489,6 +1544,7 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { int rtcp_mux_activated_callbacks1_ = 0; int rtcp_mux_activated_callbacks2_ = 0; cricket::CandidatePairInterface* last_selected_candidate_pair_; + rtc::UniqueRandomIdGenerator ssrc_generator_; }; template <> @@ -1562,14 +1618,14 @@ template <> std::unique_ptr ChannelTest::CreateChannel( rtc::Thread* worker_thread, rtc::Thread* network_thread, - cricket::MediaEngineInterface* engine, std::unique_ptr ch, webrtc::RtpTransportInternal* rtp_transport, int flags) { rtc::Thread* signaling_thread = rtc::Thread::Current(); auto channel = absl::make_unique( worker_thread, network_thread, signaling_thread, std::move(ch), - cricket::CN_VIDEO, (flags & DTLS) != 0, webrtc::CryptoOptions()); + cricket::CN_VIDEO, (flags & DTLS) != 0, webrtc::CryptoOptions(), + &ssrc_generator_); channel->Init_w(rtp_transport, /*media_transport=*/nullptr); return channel; } @@ -2063,6 +2119,10 @@ TEST_F(VideoChannelSingleThreadTest, SocketOptionsMergedOnSetTransport) { Base::SocketOptionsMergedOnSetTransport(); } +TEST_F(VideoChannelSingleThreadTest, UpdateLocalStreamsWithSimulcast) { + Base::TestUpdateLocalStreamsWithSimulcast(); +} + // VideoChannelDoubleThreadTest TEST_F(VideoChannelDoubleThreadTest, TestInit) { Base::TestInit(); @@ -2231,14 +2291,14 @@ template <> std::unique_ptr ChannelTest::CreateChannel( rtc::Thread* worker_thread, rtc::Thread* network_thread, - cricket::MediaEngineInterface* engine, std::unique_ptr ch, webrtc::RtpTransportInternal* rtp_transport, int flags) { rtc::Thread* signaling_thread = rtc::Thread::Current(); auto channel = absl::make_unique( worker_thread, network_thread, signaling_thread, std::move(ch), - cricket::CN_DATA, (flags & DTLS) != 0, webrtc::CryptoOptions()); + cricket::CN_DATA, (flags & DTLS) != 0, webrtc::CryptoOptions(), + &ssrc_generator_); channel->Init_w(rtp_transport); return channel; } diff --git a/pc/media_session.cc b/pc/media_session.cc index 3c98117d71..2e2f0cdc36 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -390,52 +390,24 @@ class UsedRtpHeaderExtensionIds : public UsedIds { static StreamParams CreateStreamParamsForNewSenderWithSsrcs( const SenderOptions& sender, const std::string& rtcp_cname, - const StreamParamsVec& current_streams, bool include_rtx_streams, - bool include_flexfec_stream) { + bool include_flexfec_stream, + UniqueRandomIdGenerator* ssrc_generator) { StreamParams result; result.id = sender.track_id; - std::vector known_ssrcs; - for (const StreamParams& params : current_streams) { - for (uint32_t ssrc : params.ssrcs) { - known_ssrcs.push_back(ssrc); - } + // TODO(brandtr): Update when we support multistream protection. + if (include_flexfec_stream && sender.num_sim_layers > 1) { + include_flexfec_stream = false; + RTC_LOG(LS_WARNING) + << "Our FlexFEC implementation only supports protecting " + "a single media streams. This session has multiple " + "media streams however, so no FlexFEC SSRC will be generated."; } - UniqueRandomIdGenerator ssrc_generator(known_ssrcs); - // We need to keep |primary_ssrcs| separate from |result.ssrcs| because - // iterators are invalidated when rtx and flexfec ssrcs are added to the list. - std::vector primary_ssrcs; - for (int i = 0; i < sender.num_sim_layers; ++i) { - primary_ssrcs.push_back(ssrc_generator()); - } - result.ssrcs = primary_ssrcs; + result.GenerateSsrcs(sender.num_sim_layers, include_rtx_streams, + include_flexfec_stream, ssrc_generator); - if (sender.num_sim_layers > 1) { - SsrcGroup group(kSimSsrcGroupSemantics, result.ssrcs); - result.ssrc_groups.push_back(group); - } - // Generate an RTX ssrc for every ssrc in the group. - if (include_rtx_streams) { - for (uint32_t ssrc : primary_ssrcs) { - result.AddFidSsrc(ssrc, ssrc_generator()); - } - } - // Generate extra ssrc for include_flexfec_stream case. - if (include_flexfec_stream) { - // TODO(brandtr): Update when we support multistream protection. - if (primary_ssrcs.size() == 1) { - for (uint32_t ssrc : primary_ssrcs) { - result.AddFecFrSsrc(ssrc, ssrc_generator()); - } - } else if (!primary_ssrcs.empty()) { - RTC_LOG(LS_WARNING) - << "Our FlexFEC implementation only supports protecting " - "a single media streams. This session has multiple " - "media streams however, so no FlexFEC SSRC will be generated."; - } - } result.cname = rtcp_cname; result.set_stream_ids(sender.stream_ids); @@ -523,6 +495,7 @@ template static bool AddStreamParams( const std::vector& sender_options, const std::string& rtcp_cname, + UniqueRandomIdGenerator* ssrc_generator, StreamParamsVec* current_streams, MediaContentDescriptionImpl* content_description) { // SCTP streams are not negotiated using SDP/ContentDescriptions. @@ -548,8 +521,8 @@ static bool AddStreamParams( ? // Signal SSRCs and legacy simulcast (if requested). CreateStreamParamsForNewSenderWithSsrcs( - sender, rtcp_cname, *current_streams, include_rtx_streams, - include_flexfec_stream) + sender, rtcp_cname, include_rtx_streams, + include_flexfec_stream, ssrc_generator) : // Signal RIDs and spec-compliant simulcast (if requested). CreateStreamParamsForNewSenderWithRids(sender, rtcp_cname); @@ -787,6 +760,7 @@ static bool CreateMediaContentOffer( const CryptoParamsVec* current_cryptos, const std::vector& crypto_suites, const RtpHeaderExtensions& rtp_extensions, + UniqueRandomIdGenerator* ssrc_generator, StreamParamsVec* current_streams, MediaContentDescriptionImpl* offer) { offer->AddCodecs(codecs); @@ -798,7 +772,8 @@ static bool CreateMediaContentOffer( offer->set_rtp_header_extensions(rtp_extensions); if (!AddStreamParams(media_description_options.sender_options, - session_options.rtcp_cname, current_streams, offer)) { + session_options.rtcp_cname, ssrc_generator, + current_streams, offer)) { return false; } @@ -1166,6 +1141,7 @@ static bool CreateMediaContentAnswer( const SecurePolicy& sdes_policy, const CryptoParamsVec* current_cryptos, const RtpHeaderExtensions& local_rtp_extenstions, + UniqueRandomIdGenerator* ssrc_generator, bool enable_encrypted_rtp_header_extensions, StreamParamsVec* current_streams, bool bundle_enabled, @@ -1203,7 +1179,8 @@ static bool CreateMediaContentAnswer( } if (!AddStreamParams(media_description_options.sender_options, - session_options.rtcp_cname, current_streams, answer)) { + session_options.rtcp_cname, ssrc_generator, + current_streams, answer)) { return false; // Something went seriously wrong. } @@ -1343,13 +1320,18 @@ bool MediaSessionOptions::HasMediaDescription(MediaType type) const { } MediaSessionDescriptionFactory::MediaSessionDescriptionFactory( - const TransportDescriptionFactory* transport_desc_factory) - : transport_desc_factory_(transport_desc_factory) {} + const TransportDescriptionFactory* transport_desc_factory, + rtc::UniqueRandomIdGenerator* ssrc_generator) + : ssrc_generator_(ssrc_generator), + transport_desc_factory_(transport_desc_factory) { + RTC_DCHECK(ssrc_generator_); +} MediaSessionDescriptionFactory::MediaSessionDescriptionFactory( ChannelManager* channel_manager, - const TransportDescriptionFactory* transport_desc_factory) - : transport_desc_factory_(transport_desc_factory) { + const TransportDescriptionFactory* transport_desc_factory, + rtc::UniqueRandomIdGenerator* ssrc_generator) + : MediaSessionDescriptionFactory(transport_desc_factory, ssrc_generator) { channel_manager->GetSupportedAudioSendCodecs(&audio_send_codecs_); channel_manager->GetSupportedAudioReceiveCodecs(&audio_recv_codecs_); channel_manager->GetSupportedAudioRtpHeaderExtensions(&audio_rtp_extensions_); @@ -2039,10 +2021,11 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( std::vector crypto_suites; GetSupportedAudioSdesCryptoSuiteNames(session_options.crypto_options, &crypto_suites); - if (!CreateMediaContentOffer( - media_description_options, session_options, filtered_codecs, - sdes_policy, GetCryptos(current_content), crypto_suites, - audio_rtp_extensions, current_streams, audio.get())) { + if (!CreateMediaContentOffer(media_description_options, session_options, + filtered_codecs, sdes_policy, + GetCryptos(current_content), crypto_suites, + audio_rtp_extensions, ssrc_generator_, + current_streams, audio.get())) { return false; } @@ -2109,10 +2092,11 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( } } - if (!CreateMediaContentOffer( - media_description_options, session_options, filtered_codecs, - sdes_policy, GetCryptos(current_content), crypto_suites, - video_rtp_extensions, current_streams, video.get())) { + if (!CreateMediaContentOffer(media_description_options, session_options, + filtered_codecs, sdes_policy, + GetCryptos(current_content), crypto_suites, + video_rtp_extensions, ssrc_generator_, + current_streams, video.get())) { return false; } @@ -2180,7 +2164,7 @@ bool MediaSessionDescriptionFactory::AddDataContentForOffer( if (!CreateMediaContentOffer( media_description_options, session_options, data_codecs, sdes_policy, GetCryptos(current_content), crypto_suites, RtpHeaderExtensions(), - current_streams, data.get())) { + ssrc_generator_, current_streams, data.get())) { return false; } @@ -2283,7 +2267,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( if (!CreateMediaContentAnswer( offer_audio_description, media_description_options, session_options, filtered_codecs, sdes_policy, GetCryptos(current_content), - audio_rtp_header_extensions(), + audio_rtp_header_extensions(), ssrc_generator_, enable_encrypted_rtp_header_extensions_, current_streams, bundle_enabled, audio_answer.get())) { return false; // Fails the session setup. @@ -2372,7 +2356,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( if (!CreateMediaContentAnswer( offer_video_description, media_description_options, session_options, filtered_codecs, sdes_policy, GetCryptos(current_content), - video_rtp_header_extensions(), + video_rtp_header_extensions(), ssrc_generator_, enable_encrypted_rtp_header_extensions_, current_streams, bundle_enabled, video_answer.get())) { return false; // Failed the sessin setup. @@ -2432,8 +2416,9 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer( if (!CreateMediaContentAnswer( offer_data_description, media_description_options, session_options, data_codecs, sdes_policy, GetCryptos(current_content), - RtpHeaderExtensions(), enable_encrypted_rtp_header_extensions_, - current_streams, bundle_enabled, data_answer.get())) { + RtpHeaderExtensions(), ssrc_generator_, + enable_encrypted_rtp_header_extensions_, current_streams, + bundle_enabled, data_answer.get())) { return false; // Fails the session setup. } diff --git a/pc/media_session.h b/pc/media_session.h index 2251b5b98b..538e2195bb 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -26,6 +26,7 @@ #include "p2p/base/transport_description_factory.h" #include "pc/jsep_transport.h" #include "pc/session_description.h" +#include "rtc_base/unique_id_generator.h" namespace cricket { @@ -127,15 +128,19 @@ struct MediaSessionOptions { // of the various fields to determine the proper result. class MediaSessionDescriptionFactory { public: - // Default ctor; use methods below to set configuration. - // The TransportDescriptionFactory is not owned by MediaSessionDescFactory, - // so it must be kept alive by the user of this class. - explicit MediaSessionDescriptionFactory( - const TransportDescriptionFactory* factory); + // Simple constructor that does not set any configuration for the factory. + // When using this constructor, the methods below can be used to set the + // configuration. + // The TransportDescriptionFactory and the UniqueRandomIdGenerator are not + // owned by MediaSessionDescriptionFactory, so they must be kept alive by the + // user of this class. + MediaSessionDescriptionFactory(const TransportDescriptionFactory* factory, + rtc::UniqueRandomIdGenerator* ssrc_generator); // This helper automatically sets up the factory to get its configuration // from the specified ChannelManager. MediaSessionDescriptionFactory(ChannelManager* cmanager, - const TransportDescriptionFactory* factory); + const TransportDescriptionFactory* factory, + rtc::UniqueRandomIdGenerator* ssrc_generator); const AudioCodecs& audio_sendrecv_codecs() const; const AudioCodecs& audio_send_codecs() const; @@ -300,6 +305,8 @@ class MediaSessionDescriptionFactory { VideoCodecs video_codecs_; RtpHeaderExtensions video_rtp_extensions_; DataCodecs data_codecs_; + // This object is not owned by the channel so it must outlive it. + rtc::UniqueRandomIdGenerator* const ssrc_generator_; bool enable_encrypted_rtp_header_extensions_ = false; // TODO(zhihuang): Rename secure_ to sdec_policy_; rename the related getter // and setter. diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index b6be353c36..8525d61525 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -28,6 +28,7 @@ #include "rtc_base/message_digest.h" #include "rtc_base/ssl_adapter.h" #include "rtc_base/strings/string_builder.h" +#include "rtc_base/unique_id_generator.h" #include "test/gmock.h" #define ASSERT_CRYPTO(cd, s, cs) \ @@ -79,6 +80,7 @@ using rtc::CS_AEAD_AES_128_GCM; using rtc::CS_AEAD_AES_256_GCM; using rtc::CS_AES_CM_128_HMAC_SHA1_32; using rtc::CS_AES_CM_128_HMAC_SHA1_80; +using rtc::UniqueRandomIdGenerator; using testing::Each; using testing::ElementsAreArray; using testing::Eq; @@ -382,7 +384,8 @@ static MediaSessionOptions CreatePlanBMediaSessionOptions() { // these tests may be obsolete as a result, and should be refactored or removed. class MediaSessionDescriptionFactoryTest : public testing::Test { public: - MediaSessionDescriptionFactoryTest() : f1_(&tdf1_), f2_(&tdf2_) { + MediaSessionDescriptionFactoryTest() + : f1_(&tdf1_, &ssrc_generator1), f2_(&tdf2_, &ssrc_generator2) { f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1), MAKE_VECTOR(kAudioCodecs1)); f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1)); @@ -688,6 +691,8 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { } protected: + UniqueRandomIdGenerator ssrc_generator1; + UniqueRandomIdGenerator ssrc_generator2; MediaSessionDescriptionFactory f1_; MediaSessionDescriptionFactory f2_; TransportDescriptionFactory tdf1_; @@ -4016,7 +4021,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, class MediaProtocolTest : public ::testing::TestWithParam { public: - MediaProtocolTest() : f1_(&tdf1_), f2_(&tdf2_) { + MediaProtocolTest() + : f1_(&tdf1_, &ssrc_generator1), f2_(&tdf2_, &ssrc_generator2) { f1_.set_audio_codecs(MAKE_VECTOR(kAudioCodecs1), MAKE_VECTOR(kAudioCodecs1)); f1_.set_video_codecs(MAKE_VECTOR(kVideoCodecs1)); @@ -4040,6 +4046,8 @@ class MediaProtocolTest : public ::testing::TestWithParam { MediaSessionDescriptionFactory f2_; TransportDescriptionFactory tdf1_; TransportDescriptionFactory tdf2_; + UniqueRandomIdGenerator ssrc_generator1; + UniqueRandomIdGenerator ssrc_generator2; }; TEST_P(MediaProtocolTest, TestAudioVideoAcceptance) { @@ -4074,7 +4082,8 @@ INSTANTIATE_TEST_CASE_P(MediaProtocolDtlsPatternTest, TEST_F(MediaSessionDescriptionFactoryTest, TestSetAudioCodecs) { TransportDescriptionFactory tdf; - MediaSessionDescriptionFactory sf(&tdf); + UniqueRandomIdGenerator ssrc_generator; + MediaSessionDescriptionFactory sf(&tdf, &ssrc_generator); std::vector send_codecs = MAKE_VECTOR(kAudioCodecs1); std::vector recv_codecs = MAKE_VECTOR(kAudioCodecs2); @@ -4130,7 +4139,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestSetAudioCodecs) { // an object that is not semantically equivalent to the set object. TEST_F(MediaSessionDescriptionFactoryTest, VideoHasRidExtensionsInUnifiedPlan) { TransportDescriptionFactory tdf; - MediaSessionDescriptionFactory sf(&tdf); + UniqueRandomIdGenerator ssrc_generator; + MediaSessionDescriptionFactory sf(&tdf, &ssrc_generator); sf.set_is_unified_plan(true); cricket::RtpHeaderExtensions extensions; sf.set_video_rtp_header_extensions(extensions); @@ -4155,7 +4165,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, VideoHasRidExtensionsInUnifiedPlan) { // an object that is not semantically equivalent to the set object. TEST_F(MediaSessionDescriptionFactoryTest, AudioHasRidExtensionsInUnifiedPlan) { TransportDescriptionFactory tdf; - MediaSessionDescriptionFactory sf(&tdf); + UniqueRandomIdGenerator ssrc_generator; + MediaSessionDescriptionFactory sf(&tdf, &ssrc_generator); sf.set_is_unified_plan(true); cricket::RtpHeaderExtensions extensions; sf.set_audio_rtp_header_extensions(extensions); @@ -4193,7 +4204,8 @@ bool CodecsMatch(const std::vector& codecs1, void TestAudioCodecsOffer(RtpTransceiverDirection direction) { TransportDescriptionFactory tdf; - MediaSessionDescriptionFactory sf(&tdf); + UniqueRandomIdGenerator ssrc_generator; + MediaSessionDescriptionFactory sf(&tdf, &ssrc_generator); const std::vector send_codecs = MAKE_VECTOR(kAudioCodecs1); const std::vector recv_codecs = MAKE_VECTOR(kAudioCodecs2); const std::vector sendrecv_codecs = @@ -4290,8 +4302,9 @@ void TestAudioCodecsAnswer(RtpTransceiverDirection offer_direction, bool add_legacy_stream) { TransportDescriptionFactory offer_tdf; TransportDescriptionFactory answer_tdf; - MediaSessionDescriptionFactory offer_factory(&offer_tdf); - MediaSessionDescriptionFactory answer_factory(&answer_tdf); + UniqueRandomIdGenerator ssrc_generator1, ssrc_generator2; + MediaSessionDescriptionFactory offer_factory(&offer_tdf, &ssrc_generator1); + MediaSessionDescriptionFactory answer_factory(&answer_tdf, &ssrc_generator2); offer_factory.set_audio_codecs( VectorFromIndices(kOfferAnswerCodecs, kOfferSendCodecs), VectorFromIndices(kOfferAnswerCodecs, kOfferRecvCodecs)); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 3be5ca3fa3..33f4ce166a 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -59,8 +59,13 @@ using cricket::ContentInfo; using cricket::ContentInfos; using cricket::MediaContentDescription; using cricket::MediaProtocolType; +using cricket::RidDescription; +using cricket::RidDirection; using cricket::SessionDescription; +using cricket::SimulcastDescription; +using cricket::SimulcastLayer; using cricket::SimulcastLayerList; +using cricket::StreamParams; using cricket::TransportInfo; using cricket::LOCAL_PORT_TYPE; @@ -1092,7 +1097,7 @@ bool PeerConnection::Initialize( webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory( signaling_thread(), channel_manager(), this, session_id(), - std::move(dependencies.cert_generator), certificate)); + std::move(dependencies.cert_generator), certificate, &ssrc_generator_)); webrtc_session_desc_factory_->SignalCertificateReady.connect( this, &PeerConnection::OnCertificateReady); @@ -2198,18 +2203,20 @@ RTCError PeerConnection::ApplyLocalDescription( if (!content) { continue; } - const auto& streams = content->media_description()->streams(); - if (!content->rejected && !streams.empty()) { - transceiver->internal()->sender_internal()->set_stream_ids( - streams[0].stream_ids()); - transceiver->internal()->sender_internal()->SetSsrc( - streams[0].first_ssrc()); - } else { + cricket::ChannelInterface* channel = transceiver->internal()->channel(); + if (content->rejected || !channel || channel->local_streams().empty()) { // 0 is a special value meaning "this sender has no associated send // stream". Need to call this so the sender won't attempt to configure // a no longer existing stream and run into DCHECKs in the lower // layers. transceiver->internal()->sender_internal()->SetSsrc(0); + } else { + // Get the StreamParams from the channel which could generate SSRCs. + const std::vector& streams = channel->local_streams(); + transceiver->internal()->sender_internal()->set_stream_ids( + streams[0].stream_ids()); + transceiver->internal()->sender_internal()->SetSsrc( + streams[0].first_ssrc()); } } } else { @@ -2911,6 +2918,72 @@ RTCError PeerConnection::UpdateDataChannel( return RTCError::OK(); } +// This method will extract any send encodings that were sent by the remote +// connection. This is currently only relevant for Simulcast scenario (where +// the number of layers may be communicated by the server). +static std::vector GetSendEncodingsFromRemoteDescription( + const MediaContentDescription& desc) { + if (!desc.HasSimulcast()) { + return {}; + } + std::vector result; + const SimulcastDescription& simulcast = desc.simulcast_description(); + + // This is a remote description, the parameters we are after should appear + // as receive streams. + for (const auto& alternatives : simulcast.receive_layers()) { + RTC_DCHECK(!alternatives.empty()); + // There is currently no way to specify or choose from alternatives. + // We will always use the first alternative, which is the most preferred. + const SimulcastLayer& layer = alternatives[0]; + RtpEncodingParameters parameters; + parameters.rid = layer.rid; + parameters.active = !layer.is_paused; + result.push_back(parameters); + } + + return result; +} + +static RTCError UpdateSimulcastLayerStatusInSender( + const std::vector& layers, + RtpSenderInterface* sender) { + RTC_DCHECK(sender); + RtpParameters parameters = sender->GetParameters(); + + // The simulcast envelope cannot be changed, only the status of the streams. + // So we will iterate over the send encodings rather than the layers. + for (RtpEncodingParameters& encoding : parameters.encodings) { + auto iter = std::find_if(layers.begin(), layers.end(), + [&encoding](const SimulcastLayer& layer) { + return layer.rid == encoding.rid; + }); + // A layer that cannot be found may have been removed by the remote party. + encoding.active = iter != layers.end() && !iter->is_paused; + } + + return sender->SetParameters(parameters); +} + +static RTCError DisableSimulcastInSender(RtpSenderInterface* sender) { + RTC_DCHECK(sender); + RtpParameters parameters = sender->GetParameters(); + if (parameters.encodings.empty()) { + return RTCError::OK(); + } + + bool first_layer_status = parameters.encodings[0].active; + for (RtpEncodingParameters& encoding : parameters.encodings) { + // TODO(bugs.webrtc.org/10251): Active does not really disable the layer. + // The user might enable it at a later point in time even though it was + // rejected. + encoding.active = false; + } + parameters.encodings[0].active = first_layer_status; + + return sender->SetParameters(parameters); +} + RTCErrorOr>> PeerConnection::AssociateTransceiver(cricket::ContentSource source, SdpType type, @@ -2969,8 +3042,10 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, << " at i=" << mline_index << " in response to the remote description."; std::string sender_id = rtc::CreateRandomUuid(); - auto sender = - CreateSender(media_desc->type(), sender_id, nullptr, {}, {}); + std::vector send_encodings = + GetSendEncodingsFromRemoteDescription(*media_desc); + auto sender = CreateSender(media_desc->type(), sender_id, nullptr, {}, + send_encodings); std::string receiver_id; if (!media_desc->streams().empty()) { receiver_id = media_desc->streams()[0].id; @@ -2982,6 +3057,22 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, transceiver->internal()->set_direction( RtpTransceiverDirection::kRecvOnly); } + + // Check if the offer indicated simulcast but the answer rejected it. + // This can happen when simulcast is not supported on the remote party. + // This check can be simplified to comparing the number of send encodings, + // but that might break legacy implementation in which simulcast is not + // signaled in the remote description. + if (old_local_content && old_local_content->media_description() && + old_local_content->media_description()->HasSimulcast() && + !media_desc->HasSimulcast()) { + RTCError error = + DisableSimulcastInSender(transceiver->internal()->sender()); + if (!error.ok()) { + RTC_LOG(LS_ERROR) << "Failed to remove rejected simulcast."; + return std::move(error); + } + } } RTC_DCHECK(transceiver); if (transceiver->media_type() != media_desc->type()) { @@ -2989,6 +3080,20 @@ PeerConnection::AssociateTransceiver(cricket::ContentSource source, RTCErrorType::INVALID_PARAMETER, "Transceiver type does not match media description type."); } + if (media_desc->HasSimulcast()) { + std::vector layers = + source == cricket::CS_LOCAL + ? media_desc->simulcast_description().send_layers().GetAllLayers() + : media_desc->simulcast_description() + .receive_layers() + .GetAllLayers(); + RTCError error = UpdateSimulcastLayerStatusInSender( + layers, transceiver->internal()->sender()); + if (!error.ok()) { + RTC_LOG(LS_ERROR) << "Failed updating status for simulcast layers."; + return std::move(error); + } + } // Associate the found or created RtpTransceiver with the m= section by // setting the value of the RtpTransceiver's mid property to the MID of the m= // section, and establish a mapping between the transceiver and the index of @@ -4035,6 +4140,20 @@ void PeerConnection::GetOptionsForPlanBOffer( offer_answer_options.num_simulcast_layers); } +static void GetSimulcastInformationFromRtpParameters( + const RtpParameters& parameters, + RidDirection direction, + std::vector* rids, + SimulcastLayerList* layers) { + for (const RtpEncodingParameters& encoding : parameters.encodings) { + if (encoding.rid.empty()) { + continue; + } + rids->push_back(RidDescription(encoding.rid, direction)); + layers->AddLayer(SimulcastLayer(encoding.rid, !encoding.active)); + } +} + static cricket::MediaDescriptionOptions GetMediaDescriptionOptionsForTransceiver( rtc::scoped_refptr> @@ -4048,18 +4167,46 @@ GetMediaDescriptionOptionsForTransceiver( // sendrecv. // 2. If the MSID is included, then it must be included in any subsequent // offer/answer exactly the same until the RtpTransceiver is stopped. - if (!transceiver->stopped() && - (RtpTransceiverDirectionHasSend(transceiver->direction()) || - transceiver->internal()->has_ever_been_used_to_send())) { - cricket::SenderOptions sender_options; - sender_options.track_id = transceiver->sender()->id(); - sender_options.stream_ids = transceiver->sender()->stream_ids(); - int num_send_encoding_layers = - transceiver->sender()->init_send_encodings().size(); - sender_options.num_sim_layers = - !num_send_encoding_layers ? 1 : num_send_encoding_layers; - media_description_options.sender_options.push_back(sender_options); + if (transceiver->stopped() || + (!RtpTransceiverDirectionHasSend(transceiver->direction()) && + !transceiver->internal()->has_ever_been_used_to_send())) { + return media_description_options; } + + cricket::SenderOptions sender_options; + sender_options.track_id = transceiver->sender()->id(); + sender_options.stream_ids = transceiver->sender()->stream_ids(); + + // The following sets up RIDs and Simulcast. + // RIDs are included if Simulcast is requested or if any RID was specified. + RtpParameters send_parameters = transceiver->sender()->GetParameters(); + RtpParameters receive_parameters = transceiver->receiver()->GetParameters(); + bool has_rids = std::any_of(send_parameters.encodings.begin(), + send_parameters.encodings.end(), + [](const RtpEncodingParameters& encoding) { + return !encoding.rid.empty(); + }); + + std::vector send_rids, receive_rids; + SimulcastLayerList send_layers, receive_layers; + GetSimulcastInformationFromRtpParameters(send_parameters, RidDirection::kSend, + &send_rids, &send_layers); + GetSimulcastInformationFromRtpParameters(receive_parameters, + RidDirection::kReceive, + &receive_rids, &receive_layers); + if (has_rids) { + sender_options.rids = send_rids; + } + + sender_options.simulcast_layers = send_layers; + media_description_options.receive_simulcast_layers = receive_layers; + media_description_options.receive_rids = receive_rids; + // When RIDs are configured, we must set num_sim_layers to 0 to. + // Otherwise, num_sim_layers must be 1 because either there is no + // simulcast, or simulcast is acheived by munging the SDP. + sender_options.num_sim_layers = has_rids ? 0 : 1; + media_description_options.sender_options.push_back(sender_options); + return media_description_options; } @@ -5930,7 +6077,7 @@ cricket::VoiceChannel* PeerConnection::CreateVoiceChannel( cricket::VoiceChannel* voice_channel = channel_manager()->CreateVoiceChannel( call_.get(), configuration_.media_config, rtp_transport, media_transport, signaling_thread(), mid, SrtpRequired(), GetCryptoOptions(), - audio_options_); + &ssrc_generator_, audio_options_); if (!voice_channel) { return nullptr; } @@ -5955,7 +6102,7 @@ cricket::VideoChannel* PeerConnection::CreateVideoChannel( cricket::VideoChannel* video_channel = channel_manager()->CreateVideoChannel( call_.get(), configuration_.media_config, rtp_transport, media_transport, signaling_thread(), mid, SrtpRequired(), GetCryptoOptions(), - video_options_); + &ssrc_generator_, video_options_); if (!video_channel) { return nullptr; } @@ -6002,7 +6149,7 @@ bool PeerConnection::CreateDataChannel(const std::string& mid) { RtpTransportInternal* rtp_transport = GetRtpTransport(mid); rtp_data_channel_ = channel_manager()->CreateRtpDataChannel( configuration_.media_config, rtp_transport, signaling_thread(), mid, - SrtpRequired(), GetCryptoOptions()); + SrtpRequired(), GetCryptoOptions(), &ssrc_generator_); if (!rtp_data_channel_) { return false; } diff --git a/pc/peer_connection.h b/pc/peer_connection.h index dde19bacfa..7c144ce3bc 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -1149,6 +1149,12 @@ class PeerConnection : public PeerConnectionInternal, int usage_event_accumulator_ = 0; bool return_histogram_very_quickly_ = false; + + // This object should be used to generate any SSRC that is not explicitly + // specified by the user (or by the remote party). + // The generator is not used directly, instead it is passed on to the + // channel manager and the session description factory. + rtc::UniqueRandomIdGenerator ssrc_generator_; }; } // namespace webrtc diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index 34092fb4eb..f3fb63ad48 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -105,11 +105,13 @@ class RtpSenderReceiverTest : public testing::Test, voice_channel_ = channel_manager_.CreateVoiceChannel( &fake_call_, cricket::MediaConfig(), rtp_transport_.get(), /*media_transport=*/nullptr, rtc::Thread::Current(), cricket::CN_AUDIO, - srtp_required, webrtc::CryptoOptions(), cricket::AudioOptions()); + srtp_required, webrtc::CryptoOptions(), &ssrc_generator_, + cricket::AudioOptions()); video_channel_ = channel_manager_.CreateVideoChannel( &fake_call_, cricket::MediaConfig(), rtp_transport_.get(), /*media_transport=*/nullptr, rtc::Thread::Current(), cricket::CN_VIDEO, - srtp_required, webrtc::CryptoOptions(), cricket::VideoOptions()); + srtp_required, webrtc::CryptoOptions(), &ssrc_generator_, + cricket::VideoOptions()); voice_channel_->Enable(true); video_channel_->Enable(true); voice_media_channel_ = media_engine_->GetVoiceChannel(0); @@ -360,6 +362,7 @@ class RtpSenderReceiverTest : public testing::Test, rtc::scoped_refptr video_track_; rtc::scoped_refptr audio_track_; bool audio_sender_destroyed_signal_fired_ = false; + rtc::UniqueRandomIdGenerator ssrc_generator_; }; // Test that |voice_channel_| is updated when an audio track is associated diff --git a/pc/test/fake_peer_connection_for_stats.h b/pc/test/fake_peer_connection_for_stats.h index e33514aef2..b954fc58dc 100644 --- a/pc/test/fake_peer_connection_for_stats.h +++ b/pc/test/fake_peer_connection_for_stats.h @@ -125,9 +125,9 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { absl::make_unique(); auto* voice_media_channel_ptr = voice_media_channel.get(); voice_channel_ = absl::make_unique( - worker_thread_, network_thread_, signaling_thread_, nullptr, + worker_thread_, network_thread_, signaling_thread_, std::move(voice_media_channel), mid, kDefaultSrtpRequired, - webrtc::CryptoOptions()); + webrtc::CryptoOptions(), &ssrc_generator_); voice_channel_->set_transport_name_for_testing(transport_name); GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_AUDIO) ->internal() @@ -145,7 +145,7 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { video_channel_ = absl::make_unique( worker_thread_, network_thread_, signaling_thread_, std::move(video_media_channel), mid, kDefaultSrtpRequired, - webrtc::CryptoOptions()); + webrtc::CryptoOptions(), &ssrc_generator_); video_channel_->set_transport_name_for_testing(transport_name); GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_VIDEO) ->internal() @@ -380,6 +380,8 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { local_certificates_by_transport_; std::map> remote_cert_chains_by_transport_; + + rtc::UniqueRandomIdGenerator ssrc_generator_; }; } // namespace webrtc diff --git a/pc/test/mock_channel_interface.h b/pc/test/mock_channel_interface.h index b559e846ec..255bd2fcee 100644 --- a/pc/test/mock_channel_interface.h +++ b/pc/test/mock_channel_interface.h @@ -12,6 +12,7 @@ #define PC_TEST_MOCK_CHANNEL_INTERFACE_H_ #include +#include #include "pc/channel_interface.h" #include "test/gmock.h" @@ -39,6 +40,8 @@ class MockChannelInterface : public cricket::ChannelInterface { bool(const cricket::MediaContentDescription*, webrtc::SdpType, std::string*)); + MOCK_CONST_METHOD0(local_streams, const std::vector&()); + MOCK_CONST_METHOD0(remote_streams, const std::vector&()); MOCK_METHOD1(SetRtpTransport, bool(webrtc::RtpTransportInternal*)); }; diff --git a/pc/webrtc_session_description_factory.cc b/pc/webrtc_session_description_factory.cc index 563632c434..d859559f48 100644 --- a/pc/webrtc_session_description_factory.cc +++ b/pc/webrtc_session_description_factory.cc @@ -31,6 +31,7 @@ #include "rtc_base/string_encode.h" using cricket::MediaSessionOptions; +using rtc::UniqueRandomIdGenerator; namespace webrtc { namespace { @@ -131,9 +132,12 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( PeerConnectionInternal* pc, const std::string& session_id, std::unique_ptr cert_generator, - const rtc::scoped_refptr& certificate) + const rtc::scoped_refptr& certificate, + UniqueRandomIdGenerator* ssrc_generator) : signaling_thread_(signaling_thread), - session_desc_factory_(channel_manager, &transport_desc_factory_), + session_desc_factory_(channel_manager, + &transport_desc_factory_, + ssrc_generator), // RFC 4566 suggested a Network Time Protocol (NTP) format timestamp // as the session id and session version. To simplify, it should be fine // to just use a random number as session id and start version from diff --git a/pc/webrtc_session_description_factory.h b/pc/webrtc_session_description_factory.h index af689a990f..b94ddbfc88 100644 --- a/pc/webrtc_session_description_factory.h +++ b/pc/webrtc_session_description_factory.h @@ -30,6 +30,7 @@ #include "rtc_base/rtc_certificate_generator.h" #include "rtc_base/third_party/sigslot/sigslot.h" #include "rtc_base/thread.h" +#include "rtc_base/unique_id_generator.h" namespace webrtc { @@ -82,7 +83,8 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, PeerConnectionInternal* pc, const std::string& session_id, std::unique_ptr cert_generator, - const rtc::scoped_refptr& certificate); + const rtc::scoped_refptr& certificate, + rtc::UniqueRandomIdGenerator* ssrc_generator); virtual ~WebRtcSessionDescriptionFactory(); static void CopyCandidatesFromSessionDescription(