From cf416e43b6e1aca6173063ac8a36dbeb63e2b5c3 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Wed, 6 Feb 2019 11:23:36 +0000 Subject: [PATCH] Revert "Remove dead code from stream_params.h" This reverts commit 3f408d006a607b6306f08e4639b23f036a74238c. Reason for revert: (Speculative) breaks downstream project Original change's description: > Remove dead code from stream_params.h > > Bug: None > Change-Id: Ia360738200022d8225f6f6939ae58bd51e298e53 > Reviewed-on: https://webrtc-review.googlesource.com/c/121601 > Reviewed-by: Amit Hilbuch > Commit-Queue: Steve Anton > Cr-Commit-Position: refs/heads/master@{#26559} TBR=steveanton@webrtc.org,amithi@webrtc.org Change-Id: I75a4d691b6000e824745ffedbc3b4f8bd03c76c9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: None Reviewed-on: https://webrtc-review.googlesource.com/c/121644 Reviewed-by: Oleh Prypin Commit-Queue: Oleh Prypin Cr-Commit-Position: refs/heads/master@{#26566} --- media/base/stream_params.cc | 127 +++++++++++++++++++++++++++ media/base/stream_params.h | 54 ++++++++++++ media/base/stream_params_unittest.cc | 95 ++++++++++++++++++++ 3 files changed, 276 insertions(+) diff --git a/media/base/stream_params.cc b/media/base/stream_params.cc index 2e96798043..0545f75893 100644 --- a/media/base/stream_params.cc +++ b/media/base/stream_params.cc @@ -19,6 +19,11 @@ namespace cricket { namespace { +// NOTE: There is no check here for duplicate streams, so check before +// adding. +void AddStream(std::vector* streams, const StreamParams& stream) { + streams->push_back(stream); +} std::string SsrcsToString(const std::vector& ssrcs) { char buf[1024]; @@ -64,6 +69,54 @@ bool GetStream(const StreamParamsVec& streams, return found != nullptr; } +MediaStreams::MediaStreams() = default; +MediaStreams::~MediaStreams() = default; + +bool MediaStreams::GetAudioStream(const StreamSelector& selector, + StreamParams* stream) { + return GetStream(audio_, selector, stream); +} + +bool MediaStreams::GetVideoStream(const StreamSelector& selector, + StreamParams* stream) { + return GetStream(video_, selector, stream); +} + +bool MediaStreams::GetDataStream(const StreamSelector& selector, + StreamParams* stream) { + return GetStream(data_, selector, stream); +} + +void MediaStreams::CopyFrom(const MediaStreams& streams) { + audio_ = streams.audio_; + video_ = streams.video_; + data_ = streams.data_; +} + +void MediaStreams::AddAudioStream(const StreamParams& stream) { + AddStream(&audio_, stream); +} + +void MediaStreams::AddVideoStream(const StreamParams& stream) { + AddStream(&video_, stream); +} + +void MediaStreams::AddDataStream(const StreamParams& stream) { + AddStream(&data_, stream); +} + +bool MediaStreams::RemoveAudioStream(const StreamSelector& selector) { + return RemoveStream(&audio_, selector); +} + +bool MediaStreams::RemoveVideoStream(const StreamSelector& selector) { + return RemoveStream(&video_, selector); +} + +bool MediaStreams::RemoveDataStream(const StreamSelector& selector) { + return RemoveStream(&data_, selector); +} + SsrcGroup::SsrcGroup(const std::string& usage, const std::vector& ssrcs) : semantics(usage), ssrcs(ssrcs) {} @@ -236,4 +289,78 @@ std::string StreamParams::first_stream_id() const { return stream_ids_.empty() ? "" : stream_ids_[0]; } +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) { + 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; +} + +namespace { +void RemoveFirst(std::list* ssrcs, uint32_t value) { + auto it = absl::c_find(*ssrcs, value); + if (it != ssrcs->end()) { + ssrcs->erase(it); + } +} +} // namespace + +bool IsSimulcastStream(const StreamParams& sp) { + // Check for spec-compliant Simulcast using rids. + if (sp.rids().size() > 1) { + return true; + } + + const SsrcGroup* const sg = sp.get_ssrc_group(kSimSsrcGroupSemantics); + if (sg == NULL || sg->ssrcs.size() < 2) { + return false; + } + // Start with all StreamParams SSRCs. Remove simulcast SSRCs (from sg) and + // RTX SSRCs. If we still have SSRCs left, we don't know what they're for. + // Also we remove first-found SSRCs only. So duplicates should lead to errors. + std::list sp_ssrcs(sp.ssrcs.begin(), sp.ssrcs.end()); + for (size_t i = 0; i < sg->ssrcs.size(); ++i) { + RemoveFirst(&sp_ssrcs, sg->ssrcs[i]); + } + for (size_t i = 0; i < sp.ssrc_groups.size(); ++i) { + const SsrcGroup& group = sp.ssrc_groups[i]; + if (group.semantics.compare(kFidSsrcGroupSemantics) != 0 || + group.ssrcs.size() != 2) { + continue; + } + RemoveFirst(&sp_ssrcs, group.ssrcs[1]); + } + // If there's SSRCs left that we don't know how to handle, we bail out. + return sp_ssrcs.size() == 0; +} + } // namespace cricket diff --git a/media/base/stream_params.h b/media/base/stream_params.h index 42f191275d..c69f4237c1 100644 --- a/media/base/stream_params.h +++ b/media/base/stream_params.h @@ -249,6 +249,50 @@ struct StreamSelector { typedef std::vector StreamParamsVec; +// A collection of audio and video and data streams. Most of the +// methods are merely for convenience. Many of these methods are keyed +// by ssrc, which is the source identifier in the RTP spec +// (http://tools.ietf.org/html/rfc3550). +// TODO(pthatcher): Add basic unit test for these. +// See https://code.google.com/p/webrtc/issues/detail?id=4107 +struct MediaStreams { + public: + MediaStreams(); + ~MediaStreams(); + void CopyFrom(const MediaStreams& sources); + + bool empty() const { + return audio_.empty() && video_.empty() && data_.empty(); + } + + std::vector* mutable_audio() { return &audio_; } + std::vector* mutable_video() { return &video_; } + std::vector* mutable_data() { return &data_; } + const std::vector& audio() const { return audio_; } + const std::vector& video() const { return video_; } + const std::vector& data() const { return data_; } + + // Gets a stream, returning true if found. + bool GetAudioStream(const StreamSelector& selector, StreamParams* stream); + bool GetVideoStream(const StreamSelector& selector, StreamParams* stream); + bool GetDataStream(const StreamSelector& selector, StreamParams* stream); + // Adds a stream. + void AddAudioStream(const StreamParams& stream); + void AddVideoStream(const StreamParams& stream); + void AddDataStream(const StreamParams& stream); + // Removes a stream, returning true if found and removed. + bool RemoveAudioStream(const StreamSelector& selector); + bool RemoveVideoStream(const StreamSelector& selector); + bool RemoveDataStream(const StreamSelector& selector); + + private: + std::vector audio_; + std::vector video_; + std::vector data_; + + RTC_DISALLOW_COPY_AND_ASSIGN(MediaStreams); +}; + template const StreamParams* GetStream(const StreamParamsVec& streams, Condition condition) { @@ -325,6 +369,16 @@ inline bool RemoveStreamByIds(StreamParamsVec* streams, }); } +// Checks if |sp| defines parameters for a single primary stream. There may +// 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 +// streams associated with the simulcast streams. Leaving as non-static so we +// can test this function. +bool IsSimulcastStream(const StreamParams& sp); + } // namespace cricket #endif // MEDIA_BASE_STREAM_PARAMS_H_ diff --git a/media/base/stream_params_unittest.cc b/media/base/stream_params_unittest.cc index 7adf0f517d..53d355af6b 100644 --- a/media/base/stream_params_unittest.cc +++ b/media/base/stream_params_unittest.cc @@ -22,6 +22,8 @@ using ::testing::Ne; static const uint32_t kSsrcs1[] = {1}; static const uint32_t kSsrcs2[] = {1, 2}; +static const uint32_t kSsrcs3[] = {1, 2, 3}; +static const uint32_t kRtxSsrcs3[] = {4, 5, 6}; static cricket::StreamParams CreateStreamParamsWithSsrcGroup( const std::string& semantics, @@ -231,6 +233,99 @@ TEST(StreamParams, ToString) { sp.ToString().c_str()); } +TEST(StreamParams, TestIsOneSsrcStream_LegacyStream) { + EXPECT_TRUE( + cricket::IsOneSsrcStream(cricket::StreamParams::CreateLegacy(13))); +} + +TEST(StreamParams, TestIsOneSsrcStream_SingleRtxStream) { + cricket::StreamParams stream; + stream.add_ssrc(13); + EXPECT_TRUE(stream.AddFidSsrc(13, 14)); + 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)))); + EXPECT_FALSE(cricket::IsOneSsrcStream( + cricket::CreateSimStreamParams("cname", MAKE_VECTOR(kSsrcs3)))); +} + +TEST(StreamParams, TestIsOneSsrcStream_SimRtxStream) { + cricket::StreamParams stream = cricket::CreateSimWithRtxStreamParams( + "cname", MAKE_VECTOR(kSsrcs3), MAKE_VECTOR(kRtxSsrcs3)); + EXPECT_FALSE(cricket::IsOneSsrcStream(stream)); +} + +TEST(StreamParams, TestIsSimulcastStream_LegacyStream) { + EXPECT_FALSE( + cricket::IsSimulcastStream(cricket::StreamParams::CreateLegacy(13))); +} + +TEST(StreamParams, TestIsSimulcastStream_SingleRtxStream) { + cricket::StreamParams stream; + stream.add_ssrc(13); + EXPECT_TRUE(stream.AddFidSsrc(13, 14)); + EXPECT_FALSE(cricket::IsSimulcastStream(stream)); +} + +TEST(StreamParams, TestIsSimulcastStream_SimulcastStream) { + EXPECT_TRUE(cricket::IsSimulcastStream( + cricket::CreateSimStreamParams("cname", MAKE_VECTOR(kSsrcs2)))); + EXPECT_TRUE(cricket::IsSimulcastStream( + cricket::CreateSimStreamParams("cname", MAKE_VECTOR(kSsrcs3)))); +} + +TEST(StreamParams, TestIsSimulcastStream_SimRtxStream) { + cricket::StreamParams stream = cricket::CreateSimWithRtxStreamParams( + "cname", MAKE_VECTOR(kSsrcs3), MAKE_VECTOR(kRtxSsrcs3)); + EXPECT_TRUE(cricket::IsSimulcastStream(stream)); +} + +TEST(StreamParams, TestIsSimulcastStream_InvalidStreams) { + // stream1 has extra non-sim, non-fid ssrc. + cricket::StreamParams stream1 = cricket::CreateSimWithRtxStreamParams( + "cname", MAKE_VECTOR(kSsrcs3), MAKE_VECTOR(kRtxSsrcs3)); + stream1.add_ssrc(25); + EXPECT_FALSE(cricket::IsSimulcastStream(stream1)); + + // stream2 has invalid fid-group (no primary). + cricket::StreamParams stream2; + stream2.add_ssrc(13); + EXPECT_TRUE(stream2.AddFidSsrc(13, 14)); + stream2.ssrcs.erase( + std::remove(stream2.ssrcs.begin(), stream2.ssrcs.end(), 13u), + stream2.ssrcs.end()); + EXPECT_FALSE(cricket::IsSimulcastStream(stream2)); + + // stream3 has two SIM groups. + cricket::StreamParams stream3 = + cricket::CreateSimStreamParams("cname", MAKE_VECTOR(kSsrcs2)); + std::vector sim_ssrcs = MAKE_VECTOR(kRtxSsrcs3); + cricket::SsrcGroup sg(cricket::kSimSsrcGroupSemantics, sim_ssrcs); + for (size_t i = 0; i < sim_ssrcs.size(); i++) { + stream3.add_ssrc(sim_ssrcs[i]); + } + stream3.ssrc_groups.push_back(sg); + EXPECT_FALSE(cricket::IsSimulcastStream(stream3)); +} + TEST(StreamParams, TestGenerateSsrcs_SingleStreamWithRtxAndFlex) { rtc::UniqueRandomIdGenerator generator; cricket::StreamParams stream;