From 9688e384e6ae8f214624d9dc0d869152b9e86eb3 Mon Sep 17 00:00:00 2001 From: brandtr Date: Tue, 22 Nov 2016 00:59:48 -0800 Subject: [PATCH] Add support for FEC-FR semantics in StreamParams. This allows us to associate a FlexFEC SSRC with a media SSRC in the SDP. BUG=webrtc:5654 R=magjed@webrtc.org CC=stefan@webrtc.org, perkj@webrtc.org Review-Url: https://codereview.webrtc.org/2503403004 Cr-Commit-Position: refs/heads/master@{#15177} --- webrtc/media/base/streamparams.cc | 30 ++++++++++++-- webrtc/media/base/streamparams.h | 21 ++++++++-- webrtc/media/base/streamparams_unittest.cc | 46 +++++++++++++++++++++- webrtc/media/base/testutils.cc | 13 ++++++ webrtc/media/base/testutils.h | 6 +++ 5 files changed, 107 insertions(+), 9 deletions(-) diff --git a/webrtc/media/base/streamparams.cc b/webrtc/media/base/streamparams.cc index da80f02fb5..2bbf04563b 100644 --- a/webrtc/media/base/streamparams.cc +++ b/webrtc/media/base/streamparams.cc @@ -23,6 +23,7 @@ void AddStream(std::vector* streams, const StreamParams& stream) { } const char kFecSsrcGroupSemantics[] = "FEC"; +const char kFecFrSsrcGroupSemantics[] = "FEC-FR"; const char kFidSsrcGroupSemantics[] = "FID"; const char kSimSsrcGroupSemantics[] = "SIM"; @@ -200,10 +201,33 @@ bool IsOneSsrcStream(const StreamParams& sp) { if (sp.ssrcs.size() == 1 && sp.ssrc_groups.empty()) { return true; } + const SsrcGroup* fid_group = sp.get_ssrc_group(kFidSsrcGroupSemantics); + const SsrcGroup* fecfr_group = sp.get_ssrc_group(kFecFrSsrcGroupSemantics); if (sp.ssrcs.size() == 2) { - const SsrcGroup* fid_group = sp.get_ssrc_group(kFidSsrcGroupSemantics); - if (fid_group != NULL) { - return (sp.ssrcs == fid_group->ssrcs); + if (fid_group != nullptr && sp.ssrcs == fid_group->ssrcs) { + return true; + } + if (fecfr_group != nullptr && sp.ssrcs == fecfr_group->ssrcs) { + return true; + } + } + if (sp.ssrcs.size() == 3) { + if (fid_group == nullptr || fecfr_group == nullptr) { + return false; + } + if (sp.ssrcs[0] != fid_group->ssrcs[0] || + sp.ssrcs[0] != fecfr_group->ssrcs[0]) { + return false; + } + // We do not check for FlexFEC over RTX, + // as this combination is not supported. + if (sp.ssrcs[1] == fid_group->ssrcs[1] && + sp.ssrcs[2] == fecfr_group->ssrcs[1]) { + return true; + } + if (sp.ssrcs[1] == fecfr_group->ssrcs[1] && + sp.ssrcs[2] == fid_group->ssrcs[1]) { + return true; } } return false; diff --git a/webrtc/media/base/streamparams.h b/webrtc/media/base/streamparams.h index 5b9d7bd2ef..a2d48c5178 100644 --- a/webrtc/media/base/streamparams.h +++ b/webrtc/media/base/streamparams.h @@ -37,6 +37,7 @@ namespace cricket { extern const char kFecSsrcGroupSemantics[]; +extern const char kFecFrSsrcGroupSemantics[]; extern const char kFidSsrcGroupSemantics[]; extern const char kSimSsrcGroupSemantics[]; @@ -112,16 +113,28 @@ struct StreamParams { // Convenience function to add an FID ssrc for a primary_ssrc // that's already been added. - inline bool AddFidSsrc(uint32_t primary_ssrc, uint32_t fid_ssrc) { + bool AddFidSsrc(uint32_t primary_ssrc, uint32_t fid_ssrc) { return AddSecondarySsrc(kFidSsrcGroupSemantics, primary_ssrc, fid_ssrc); } // Convenience function to lookup the FID ssrc for a primary_ssrc. // Returns false if primary_ssrc not found or FID not defined for it. - inline bool GetFidSsrc(uint32_t primary_ssrc, uint32_t* fid_ssrc) const { + bool GetFidSsrc(uint32_t primary_ssrc, uint32_t* fid_ssrc) const { return GetSecondarySsrc(kFidSsrcGroupSemantics, primary_ssrc, fid_ssrc); } + // Convenience function to add an FEC-FR ssrc for a primary_ssrc + // that's already been added. + bool AddFecFrSsrc(uint32_t primary_ssrc, uint32_t fecfr_ssrc) { + return AddSecondarySsrc(kFecFrSsrcGroupSemantics, primary_ssrc, fecfr_ssrc); + } + + // Convenience function to lookup the FEC-FR ssrc for a primary_ssrc. + // Returns false if primary_ssrc not found or FEC-FR not defined for it. + bool GetFecFrSsrc(uint32_t primary_ssrc, uint32_t* fecfr_ssrc) const { + return GetSecondarySsrc(kFecFrSsrcGroupSemantics, primary_ssrc, fecfr_ssrc); + } + // Convenience to get all the SIM SSRCs if there are SIM ssrcs, or // the first SSRC otherwise. void GetPrimarySsrcs(std::vector* ssrcs) const; @@ -289,8 +302,8 @@ inline bool RemoveStreamByIds(StreamParamsVec* streams, } // Checks if |sp| defines parameters for a single primary stream. There may -// be an RTX stream associated with the primary stream. Leaving as non-static so -// we can test this function. +// be an RTX stream or a FlexFEC stream (or both) associated with the primary +// stream. Leaving as non-static so we can test this function. bool IsOneSsrcStream(const StreamParams& sp); // Checks if |sp| defines parameters for one Simulcast stream. There may be RTX diff --git a/webrtc/media/base/streamparams_unittest.cc b/webrtc/media/base/streamparams_unittest.cc index 8784ffc3f8..dc48a721ac 100644 --- a/webrtc/media/base/streamparams_unittest.cc +++ b/webrtc/media/base/streamparams_unittest.cc @@ -128,7 +128,7 @@ TEST(StreamParams, FidFunctions) { EXPECT_FALSE(sp.GetFidSsrc(15, &fid_ssrc)); sp.add_ssrc(20); - sp.AddFidSsrc(20, 30); + EXPECT_TRUE(sp.AddFidSsrc(20, 30)); EXPECT_TRUE(sp.GetFidSsrc(20, &fid_ssrc)); EXPECT_EQ(30u, fid_ssrc); @@ -177,6 +177,34 @@ TEST(StreamParams, GetPrimaryAndFidSsrcs) { EXPECT_EQ(20u, fid_ssrcs[1]); } +TEST(StreamParams, FecFrFunctions) { + uint32_t fecfr_ssrc; + + cricket::StreamParams sp = cricket::StreamParams::CreateLegacy(1); + EXPECT_FALSE(sp.AddFecFrSsrc(10, 20)); + EXPECT_TRUE(sp.AddFecFrSsrc(1, 2)); + EXPECT_TRUE(sp.GetFecFrSsrc(1, &fecfr_ssrc)); + EXPECT_EQ(2u, fecfr_ssrc); + EXPECT_FALSE(sp.GetFecFrSsrc(15, &fecfr_ssrc)); + + sp.add_ssrc(20); + EXPECT_TRUE(sp.AddFecFrSsrc(20, 30)); + EXPECT_TRUE(sp.GetFecFrSsrc(20, &fecfr_ssrc)); + EXPECT_EQ(30u, fecfr_ssrc); + + // Manually create SsrcGroup to test bounds-checking + // in GetSecondarySsrc. We construct an invalid StreamParams + // for this. + std::vector fecfr_vector; + fecfr_vector.push_back(13); + cricket::SsrcGroup invalid_fecfr_group(cricket::kFecFrSsrcGroupSemantics, + fecfr_vector); + cricket::StreamParams sp_invalid; + sp_invalid.add_ssrc(13); + sp_invalid.ssrc_groups.push_back(invalid_fecfr_group); + EXPECT_FALSE(sp_invalid.GetFecFrSsrc(13, &fecfr_ssrc)); +} + TEST(StreamParams, ToString) { cricket::StreamParams sp = CreateStreamParamsWithSsrcGroup("XYZ", kSsrcs2, arraysize(kSsrcs2)); @@ -184,7 +212,6 @@ TEST(StreamParams, ToString) { sp.ToString().c_str()); } - TEST(StreamParams, TestIsOneSsrcStream_LegacyStream) { EXPECT_TRUE( cricket::IsOneSsrcStream(cricket::StreamParams::CreateLegacy(13))); @@ -197,6 +224,21 @@ TEST(StreamParams, TestIsOneSsrcStream_SingleRtxStream) { EXPECT_TRUE(cricket::IsOneSsrcStream(stream)); } +TEST(StreamParams, TestIsOneSsrcStream_SingleFlexfecStream) { + cricket::StreamParams stream; + stream.add_ssrc(13); + EXPECT_TRUE(stream.AddFecFrSsrc(13, 14)); + EXPECT_TRUE(cricket::IsOneSsrcStream(stream)); +} + +TEST(StreamParams, TestIsOneSsrcStream_SingleFlexfecAndRtxStream) { + cricket::StreamParams stream; + stream.add_ssrc(13); + EXPECT_TRUE(stream.AddFecFrSsrc(13, 14)); + EXPECT_TRUE(stream.AddFidSsrc(13, 15)); + EXPECT_TRUE(cricket::IsOneSsrcStream(stream)); +} + TEST(StreamParams, TestIsOneSsrcStream_SimulcastStream) { EXPECT_FALSE(cricket::IsOneSsrcStream( cricket::CreateSimStreamParams("cname", MAKE_VECTOR(kSsrcs2)))); diff --git a/webrtc/media/base/testutils.cc b/webrtc/media/base/testutils.cc index b1f520a929..93d1bc0b3d 100644 --- a/webrtc/media/base/testutils.cc +++ b/webrtc/media/base/testutils.cc @@ -268,4 +268,17 @@ cricket::StreamParams CreateSimWithRtxStreamParams( return sp; } +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; + return sp; +} + } // namespace cricket diff --git a/webrtc/media/base/testutils.h b/webrtc/media/base/testutils.h index 01a2a4f9c1..966a25be97 100644 --- a/webrtc/media/base/testutils.h +++ b/webrtc/media/base/testutils.h @@ -192,6 +192,12 @@ cricket::StreamParams CreateSimWithRtxStreamParams( const std::vector& ssrcs, const std::vector& rtx_ssrcs); +// Create StreamParams with single primary SSRC and corresponding FlexFEC SSRC. +cricket::StreamParams CreatePrimaryWithFecFrStreamParams( + const std::string& cname, + uint32_t primary_ssrc, + uint32_t flexfec_ssrc); + } // namespace cricket #endif // WEBRTC_MEDIA_BASE_TESTUTILS_H_