Generalize ssrc-group check to apply to groups other than SIM
BUG=chromium:1477075
Change-Id: I20f094dee11ea26a180471ce52d78d916f922f29
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/322440
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40888}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index fad0f4f..b97e829 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -356,8 +356,11 @@
}
}
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 @@
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 e5ef16f..0db4012 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::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 @@
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 @@
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 3023be1..1f5ab2f 100644
--- a/pc/peer_connection_interface_unittest.cc
+++ b/pc/peer_connection_interface_unittest.cc
@@ -3004,21 +3004,20 @@
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 4bdb7f1..b93e592 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::UnorderedElementsAre;
using ::testing::Values;
-const uint32_t kDefaultTimeout = 10000u;
-
template <typename MethodFunctor>
class OnSuccessObserver : public webrtc::SetRemoteDescriptionObserverInterface {
public:
@@ -826,7 +825,8 @@
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<uint32_t>(i);
+ ReplaceFirstSsrc(mutable_streams[0],
+ kFirstMungedSsrc + static_cast<uint32_t>(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 3f07c9e..ede159d 100644
--- a/pc/test/integration_test_helpers.cc
+++ b/pc/test/integration_test_helpers.cc
@@ -55,6 +55,13 @@
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 43033d2..36b2111 100644
--- a/pc/test/integration_test_helpers.h
+++ b/pc/test/integration_test_helpers.h
@@ -173,6 +173,10 @@
// 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<const webrtc::RTCInboundRtpStreamStats*>& inbound_rtps);
