diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index fad0f4fc55..b97e829f3e 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -356,8 +356,11 @@ static bool ValidateStreamParams(const StreamParams& sp) { } } for (const auto& group : sp.ssrc_groups) { - if (group.semantics != kSimSsrcGroupSemantics) + if (!(group.semantics == kFidSsrcGroupSemantics || + group.semantics == kSimSsrcGroupSemantics || + group.semantics == kFecFrSsrcGroupSemantics)) { continue; + } for (uint32_t group_ssrc : group.ssrcs) { auto it = absl::c_find_if(sp.ssrcs, [&group_ssrc](uint32_t ssrc) { return ssrc == group_ssrc; @@ -365,7 +368,7 @@ static bool ValidateStreamParams(const StreamParams& sp) { if (it == sp.ssrcs.end()) { RTC_LOG(LS_ERROR) << "SSRC '" << group_ssrc << "' missing from StreamParams ssrcs with semantics " - << kSimSsrcGroupSemantics << ": " << sp.ToString(); + << group.semantics << ": " << sp.ToString(); return false; } } diff --git a/pc/peer_connection_bundle_unittest.cc b/pc/peer_connection_bundle_unittest.cc index e5ef16ff8a..0db401276a 100644 --- a/pc/peer_connection_bundle_unittest.cc +++ b/pc/peer_connection_bundle_unittest.cc @@ -60,6 +60,7 @@ #include "pc/rtp_transport_internal.h" #include "pc/sdp_utils.h" #include "pc/session_description.h" +#include "pc/test/integration_test_helpers.h" #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -90,8 +91,6 @@ using ::testing::ElementsAre; using ::testing::UnorderedElementsAre; using ::testing::Values; -constexpr int kDefaultTimeout = 10000; - // TODO(steveanton): These tests should be rewritten to use the standard // RtpSenderInterface/DtlsTransportInterface objects once they're available in // the API. The RtpSender can be used to determine which transport a given media @@ -743,18 +742,18 @@ TEST_P(PeerConnectionBundleTest, ApplyDescriptionWithSameSsrcsBundledFails) { caller->SetLocalDescription(CloneSessionDescription(offer.get()))); // Modify the remote SDP to make two m= sections have the same SSRC. ASSERT_GE(offer->description()->contents().size(), 2U); - offer->description() - ->contents()[0] - .media_description() - ->mutable_streams()[0] - .ssrcs[0] = 1111222; - offer->description() - ->contents()[1] - .media_description() - ->mutable_streams()[0] - .ssrcs[0] = 1111222; - EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer))); + ReplaceFirstSsrc(offer->description() + ->contents()[0] + .media_description() + ->mutable_streams()[0], + 1111222); + ReplaceFirstSsrc(offer->description() + ->contents()[1] + .media_description() + ->mutable_streams()[0], + 1111222); + EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer))); // When BUNDLE is enabled, applying the description is expected to fail // because the demuxing criteria can not be satisfied. auto answer = callee->CreateAnswer(options); @@ -774,16 +773,16 @@ TEST_P(PeerConnectionBundleTest, caller->SetLocalDescription(CloneSessionDescription(offer.get()))); // Modify the remote SDP to make two m= sections have the same SSRC. ASSERT_GE(offer->description()->contents().size(), 2U); - offer->description() - ->contents()[0] - .media_description() - ->mutable_streams()[0] - .ssrcs[0] = 1111222; - offer->description() - ->contents()[1] - .media_description() - ->mutable_streams()[0] - .ssrcs[0] = 1111222; + ReplaceFirstSsrc(offer->description() + ->contents()[0] + .media_description() + ->mutable_streams()[0], + 1111222); + ReplaceFirstSsrc(offer->description() + ->contents()[1] + .media_description() + ->mutable_streams()[0], + 1111222); EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer))); // Without BUNDLE, demuxing is done per-transport. diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index 3023be1493..1f5ab2f449 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -3004,21 +3004,20 @@ TEST_P(PeerConnectionInterfaceTest, EXPECT_TRUE(ContainsSender(senders, kVideoTracks[0])); // Change the ssrc of the audio and video track. - cricket::MediaContentDescription* desc = - cricket::GetFirstAudioContentDescription(modified_offer->description()); - ASSERT_TRUE(desc != nullptr); - for (StreamParams& stream : desc->mutable_streams()) { - for (unsigned int& ssrc : stream.ssrcs) { - ++ssrc; - } - } - - desc = - cricket::GetFirstVideoContentDescription(modified_offer->description()); - ASSERT_TRUE(desc != nullptr); - for (StreamParams& stream : desc->mutable_streams()) { - for (unsigned int& ssrc : stream.ssrcs) { - ++ssrc; + for (auto content : modified_offer->description()->contents()) { + cricket::MediaContentDescription* desc = content.media_description(); + ASSERT_TRUE(desc); + for (StreamParams& stream : desc->mutable_streams()) { + for (unsigned int& ssrc : stream.ssrcs) { + unsigned int old_ssrc = ssrc++; + for (auto& group : stream.ssrc_groups) { + for (unsigned int& secondary_ssrc : group.ssrcs) { + if (secondary_ssrc == old_ssrc) { + secondary_ssrc = ssrc; + } + } + } + } } } diff --git a/pc/peer_connection_rtp_unittest.cc b/pc/peer_connection_rtp_unittest.cc index 4bdb7f1ea6..b93e5923bb 100644 --- a/pc/peer_connection_rtp_unittest.cc +++ b/pc/peer_connection_rtp_unittest.cc @@ -53,6 +53,7 @@ #include "pc/sdp_utils.h" #include "pc/session_description.h" #include "pc/test/fake_audio_capture_module.h" +#include "pc/test/integration_test_helpers.h" #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/checks.h" #include "rtc_base/gunit.h" @@ -73,8 +74,6 @@ using ::testing::Pair; using ::testing::UnorderedElementsAre; using ::testing::Values; -const uint32_t kDefaultTimeout = 10000u; - template class OnSuccessObserver : public webrtc::SetRemoteDescriptionObserverInterface { public: @@ -826,7 +825,8 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan, TracksDoNotEndWhenSsrcChanges) { for (size_t i = 0; i < contents.size(); ++i) { auto& mutable_streams = contents[i].media_description()->mutable_streams(); ASSERT_EQ(mutable_streams.size(), 1u); - mutable_streams[0].ssrcs[0] = kFirstMungedSsrc + static_cast(i); + ReplaceFirstSsrc(mutable_streams[0], + kFirstMungedSsrc + static_cast(i)); } ASSERT_TRUE( callee->SetLocalDescription(CloneSessionDescription(answer.get()))); diff --git a/pc/test/integration_test_helpers.cc b/pc/test/integration_test_helpers.cc index 3f07c9e826..ede159d744 100644 --- a/pc/test/integration_test_helpers.cc +++ b/pc/test/integration_test_helpers.cc @@ -55,6 +55,13 @@ int FindFirstMediaStatsIndexByKind( return -1; } +void ReplaceFirstSsrc(StreamParams& stream, uint32_t ssrc) { + stream.ssrcs[0] = ssrc; + for (auto& group : stream.ssrc_groups) { + group.ssrcs[0] = ssrc; + } +} + TaskQueueMetronome::TaskQueueMetronome(TimeDelta tick_period) : tick_period_(tick_period) {} diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index 43033d2a5c..36b2111324 100644 --- a/pc/test/integration_test_helpers.h +++ b/pc/test/integration_test_helpers.h @@ -173,6 +173,10 @@ void RemoveSsrcsAndMsids(cricket::SessionDescription* desc); // endpoint that only signals a=msid lines to convey stream_ids. void RemoveSsrcsAndKeepMsids(cricket::SessionDescription* desc); +// Replaces the stream's primary SSRC and updates the first SSRC of all +// ssrc-groups. +void ReplaceFirstSsrc(StreamParams& stream, uint32_t ssrc); + int FindFirstMediaStatsIndexByKind( const std::string& kind, const std::vector& inbound_rtps);