Generalize stream parameter primary/secondary ssrc checks
to ensure consistency for both FID and FEC-FR ssrc-groups.
BUG=chromium:1454860
Change-Id: I61277e73e0a28f5773260ec62c268bdc8c2cd738
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/309760
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#40347}
diff --git a/media/base/stream_params.cc b/media/base/stream_params.cc
index 0fe1be6..ac9daee 100644
--- a/media/base/stream_params.cc
+++ b/media/base/stream_params.cc
@@ -183,16 +183,23 @@
}
}
-void StreamParams::GetFidSsrcs(const std::vector<uint32_t>& primary_ssrcs,
- std::vector<uint32_t>* fid_ssrcs) const {
+void StreamParams::GetSecondarySsrcs(
+ const std::string& semantics,
+ const std::vector<uint32_t>& primary_ssrcs,
+ std::vector<uint32_t>* secondary_ssrcs) const {
for (uint32_t primary_ssrc : primary_ssrcs) {
- uint32_t fid_ssrc;
- if (GetFidSsrc(primary_ssrc, &fid_ssrc)) {
- fid_ssrcs->push_back(fid_ssrc);
+ uint32_t secondary_ssrc;
+ if (GetSecondarySsrc(semantics, primary_ssrc, &secondary_ssrc)) {
+ secondary_ssrcs->push_back(secondary_ssrc);
}
}
}
+void StreamParams::GetFidSsrcs(const std::vector<uint32_t>& primary_ssrcs,
+ std::vector<uint32_t>* fid_ssrcs) const {
+ return GetSecondarySsrcs(kFidSsrcGroupSemantics, primary_ssrcs, fid_ssrcs);
+}
+
bool StreamParams::AddSecondarySsrc(const std::string& semantics,
uint32_t primary_ssrc,
uint32_t secondary_ssrc) {
diff --git a/media/base/stream_params.h b/media/base/stream_params.h
index 60c67a1..89fc155 100644
--- a/media/base/stream_params.h
+++ b/media/base/stream_params.h
@@ -166,6 +166,14 @@
// the first SSRC otherwise.
void GetPrimarySsrcs(std::vector<uint32_t>* ssrcs) const;
+ // Convenience to get all the secondary SSRCs for the given primary ssrcs
+ // of a particular semantic.
+ // If a given primary SSRC does not have a secondary SSRC, the list of
+ // secondary SSRCS will be smaller than the list of primary SSRCs.
+ void GetSecondarySsrcs(const std::string& semantic,
+ const std::vector<uint32_t>& primary_ssrcs,
+ std::vector<uint32_t>* fid_ssrcs) const;
+
// Convenience to get all the FID SSRCs for the given primary ssrcs.
// If a given primary SSRC does not have a FID SSRC, the list of FID
// SSRCS will be smaller than the list of primary SSRCs.
diff --git a/media/base/test_utils.cc b/media/base/test_utils.cc
index a6d5f61..1b28873 100644
--- a/media/base/test_utils.cc
+++ b/media/base/test_utils.cc
@@ -35,26 +35,20 @@
const std::vector<uint32_t>& rtx_ssrcs) {
cricket::StreamParams sp = CreateSimStreamParams(cname, ssrcs);
for (size_t i = 0; i < ssrcs.size(); ++i) {
- sp.ssrcs.push_back(rtx_ssrcs[i]);
- std::vector<uint32_t> fid_ssrcs;
- fid_ssrcs.push_back(ssrcs[i]);
- fid_ssrcs.push_back(rtx_ssrcs[i]);
- cricket::SsrcGroup fid_group(cricket::kFidSsrcGroupSemantics, fid_ssrcs);
- sp.ssrc_groups.push_back(fid_group);
+ sp.AddFidSsrc(ssrcs[i], rtx_ssrcs[i]);
}
return sp;
}
+// There should be one fec ssrc per ssrc.
cricket::StreamParams CreatePrimaryWithFecFrStreamParams(
const std::string& cname,
uint32_t primary_ssrc,
uint32_t flexfec_ssrc) {
cricket::StreamParams sp;
- cricket::SsrcGroup sg(cricket::kFecFrSsrcGroupSemantics,
- {primary_ssrc, flexfec_ssrc});
sp.ssrcs = {primary_ssrc};
- sp.ssrc_groups.push_back(sg);
sp.cname = cname;
+ sp.AddFecFrSsrc(primary_ssrc, flexfec_ssrc);
return sp;
}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index a8309f5..83bb39d 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -311,30 +311,37 @@
std::vector<uint32_t> primary_ssrcs;
sp.GetPrimarySsrcs(&primary_ssrcs);
- std::vector<uint32_t> rtx_ssrcs;
- sp.GetFidSsrcs(primary_ssrcs, &rtx_ssrcs);
- for (uint32_t rtx_ssrc : rtx_ssrcs) {
- bool rtx_ssrc_present = false;
- for (uint32_t sp_ssrc : sp.ssrcs) {
- if (sp_ssrc == rtx_ssrc) {
- rtx_ssrc_present = true;
- break;
+ for (const auto& semantic :
+ {kFidSsrcGroupSemantics, kFecFrSsrcGroupSemantics}) {
+ if (!sp.has_ssrc_group(semantic)) {
+ continue;
+ }
+ std::vector<uint32_t> secondary_ssrcs;
+ sp.GetSecondarySsrcs(semantic, primary_ssrcs, &secondary_ssrcs);
+ for (uint32_t secondary_ssrc : secondary_ssrcs) {
+ bool secondary_ssrc_present = false;
+ for (uint32_t sp_ssrc : sp.ssrcs) {
+ if (sp_ssrc == secondary_ssrc) {
+ secondary_ssrc_present = true;
+ break;
+ }
+ }
+ if (!secondary_ssrc_present) {
+ RTC_LOG(LS_ERROR) << "SSRC '" << secondary_ssrc
+ << "' missing from StreamParams ssrcs with semantics "
+ << semantic << ": " << sp.ToString();
+ return false;
}
}
- if (!rtx_ssrc_present) {
- RTC_LOG(LS_ERROR) << "RTX SSRC '" << rtx_ssrc
- << "' missing from StreamParams ssrcs: "
- << sp.ToString();
+ if (!secondary_ssrcs.empty() &&
+ primary_ssrcs.size() != secondary_ssrcs.size()) {
+ RTC_LOG(LS_ERROR)
+ << semantic
+ << " secondary SSRCs exist, but don't cover all SSRCs (unsupported): "
+ << sp.ToString();
return false;
}
}
- if (!rtx_ssrcs.empty() && primary_ssrcs.size() != rtx_ssrcs.size()) {
- RTC_LOG(LS_ERROR)
- << "RTX SSRCs exist, but don't cover all SSRCs (unsupported): "
- << sp.ToString();
- return false;
- }
-
return true;
}
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 6596d82..fa16ec6 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -491,6 +491,25 @@
return RTCError::OK();
}
+RTCError ValidateSsrcGroups(const cricket::SessionDescription& description) {
+ // https://www.rfc-editor.org/rfc/rfc5576#section-4.2
+ // Every <ssrc-id> listed in an "ssrc-group" attribute MUST be
+ // defined by a corresponding "ssrc:" line in the same media
+ // description.
+ for (const ContentInfo& content : description.contents()) {
+ if (!content.media_description()) {
+ continue;
+ }
+ for (const StreamParams& sp : content.media_description()->streams()) {
+ for (const cricket::SsrcGroup& group : sp.ssrc_groups) {
+ RTC_LOG(LS_ERROR) << "YO " << group.semantics << " #"
+ << group.ssrcs.size();
+ }
+ }
+ }
+ return RTCError::OK();
+}
+
RTCError FindDuplicateHeaderExtensionIds(
const RtpExtension extension,
std::map<int, RtpExtension>& id_to_extension) {
@@ -3567,6 +3586,11 @@
return RTCError(RTCErrorType::INVALID_PARAMETER, kBundleWithoutRtcpMux);
}
+ error = ValidateSsrcGroups(*sdesc->description());
+ if (!error.ok()) {
+ return error;
+ }
+
// TODO(skvlad): When the local rtcp-mux policy is Require, reject any
// m-lines that do not rtcp-mux enabled.
diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index e9ec17b..604ad27 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -599,4 +599,64 @@
EXPECT_TRUE(pc->SetRemoteDescription(std::move(answer_with_extensions)));
}
+TEST_F(SdpOfferAnswerTest, ExpectAllSsrcsSpecifiedInSsrcGroupFid) {
+ auto pc = CreatePeerConnection();
+ std::string sdp =
+ "v=0\r\n"
+ "o=- 0 3 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "a=group:BUNDLE 0\r\n"
+ "a=fingerprint:sha-1 "
+ "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
+ "a=setup:actpass\r\n"
+ "a=ice-ufrag:ETEn\r\n"
+ "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
+ "m=video 9 UDP/TLS/RTP/SAVPF 96 97\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:0\r\n"
+ "a=rtpmap:96 H264/90000\r\n"
+ "a=fmtp:96 "
+ "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id="
+ "42e01f\r\n"
+ "a=rtpmap:97 rtx/90000\r\n"
+ "a=fmtp:97 apt=96\r\n"
+ "a=ssrc-group:FID 1 2\r\n"
+ "a=ssrc:1 cname:test\r\n";
+ auto offer = CreateSessionDescription(SdpType::kOffer, sdp);
+ EXPECT_FALSE(pc->SetRemoteDescription(std::move(offer)));
+}
+
+TEST_F(SdpOfferAnswerTest, ExpectAllSsrcsSpecifiedInSsrcGroupFecFr) {
+ auto pc = CreatePeerConnection();
+ std::string sdp =
+ "v=0\r\n"
+ "o=- 0 3 IN IP4 127.0.0.1\r\n"
+ "s=-\r\n"
+ "t=0 0\r\n"
+ "a=group:BUNDLE 0\r\n"
+ "a=fingerprint:sha-1 "
+ "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
+ "a=setup:actpass\r\n"
+ "a=ice-ufrag:ETEn\r\n"
+ "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n"
+ "m=video 9 UDP/TLS/RTP/SAVPF 96 98\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "a=rtcp-mux\r\n"
+ "a=sendonly\r\n"
+ "a=mid:0\r\n"
+ "a=rtpmap:96 H264/90000\r\n"
+ "a=fmtp:96 "
+ "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id="
+ "42e01f\r\n"
+ "a=rtpmap:98 flexfec-03/90000\r\n"
+ "a=fmtp:98 repair-window=10000000\r\n"
+ "a=ssrc-group:FEC-FR 1 2\r\n"
+ "a=ssrc:1 cname:test\r\n";
+ auto offer = CreateSessionDescription(SdpType::kOffer, sdp);
+ EXPECT_FALSE(pc->SetRemoteDescription(std::move(offer)));
+}
+
} // namespace webrtc
