From 7b7e06fd23ac67d81f378b773bb631abb1d82116 Mon Sep 17 00:00:00 2001 From: eladalon Date: Thu, 3 Aug 2017 05:13:48 -0700 Subject: [PATCH] SSRC and RSID may only refer to one sink each in RtpDemuxer RTP demuxing should only match RTP packets with one sink. BUG=webrtc:7135 Review-Url: https://codereview.webrtc.org/2968693002 Cr-Commit-Position: refs/heads/master@{#19233} --- webrtc/call/rtp_demuxer.cc | 69 ++--- webrtc/call/rtp_demuxer.h | 24 +- webrtc/call/rtp_demuxer_unittest.cc | 258 ++++++++++++++---- webrtc/call/rtp_rtcp_demuxer_helper.h | 25 ++ webrtc/call/rtp_stream_receiver_controller.cc | 10 +- webrtc/call/rtp_stream_receiver_controller.h | 2 +- ...rtp_stream_receiver_controller_interface.h | 2 +- 7 files changed, 285 insertions(+), 105 deletions(-) diff --git a/webrtc/call/rtp_demuxer.cc b/webrtc/call/rtp_demuxer.cc index ce1234208f..39fc2351cf 100644 --- a/webrtc/call/rtp_demuxer.cc +++ b/webrtc/call/rtp_demuxer.cc @@ -27,41 +27,32 @@ RtpDemuxer::~RtpDemuxer() { RTC_DCHECK(rsid_sinks_.empty()); } -void RtpDemuxer::AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) { +bool RtpDemuxer::AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) { RTC_DCHECK(sink); - RecordSsrcToSinkAssociation(ssrc, sink); + // The association might already have been set by a different + // configuration source. + // We cannot RTC_DCHECK against an attempt to remap an SSRC, because + // such a configuration might have come from the network (1. resolution + // of an RSID or 2. RTCP messages with RSID resolutions). + return ssrc_sinks_.emplace(ssrc, sink).second; } void RtpDemuxer::AddSink(const std::string& rsid, RtpPacketSinkInterface* sink) { RTC_DCHECK(StreamId::IsLegalName(rsid)); RTC_DCHECK(sink); - RTC_DCHECK(!MultimapAssociationExists(rsid_sinks_, rsid, sink)); + RTC_DCHECK(rsid_sinks_.find(rsid) == rsid_sinks_.cend()); rsid_sinks_.emplace(rsid, sink); } bool RtpDemuxer::RemoveSink(const RtpPacketSinkInterface* sink) { RTC_DCHECK(sink); - return (RemoveFromMultimapByValue(&ssrc_sinks_, sink) + - RemoveFromMultimapByValue(&rsid_sinks_, sink)) > 0; -} - -void RtpDemuxer::RecordSsrcToSinkAssociation(uint32_t ssrc, - RtpPacketSinkInterface* sink) { - RTC_DCHECK(sink); - // The association might already have been set by a different - // configuration source. - if (!MultimapAssociationExists(ssrc_sinks_, ssrc, sink)) { - ssrc_sinks_.emplace(ssrc, sink); - } + return (RemoveFromMapByValue(&ssrc_sinks_, sink) + + RemoveFromMapByValue(&rsid_sinks_, sink)) > 0; } bool RtpDemuxer::OnRtpPacket(const RtpPacketReceived& packet) { - // TODO(eladalon): This will now check every single packet, but soon a CL will - // be added which will change the many-to-many association of packets to sinks - // to a many-to-one, meaning each packet will be associated with one sink - // at most. Then, only packets with an unknown SSRC will be checked for RSID. ResolveRsidToSsrcAssociations(packet); auto it_range = ssrc_sinks_.equal_range(packet.Ssrc()); @@ -91,20 +82,34 @@ void RtpDemuxer::DeregisterRsidResolutionObserver( void RtpDemuxer::ResolveRsidToSsrcAssociations( const RtpPacketReceived& packet) { std::string rsid; - if (packet.GetExtension(&rsid)) { - // All streams associated with this RSID need to be marked as associated - // with this SSRC (if they aren't already). - auto it_range = rsid_sinks_.equal_range(rsid); - for (auto it = it_range.first; it != it_range.second; ++it) { - RecordSsrcToSinkAssociation(packet.Ssrc(), it->second); - } - - NotifyObserversOfRsidResolution(rsid, packet.Ssrc()); - - // To prevent memory-overuse attacks, forget this RSID. Future packets - // with this RSID, but a different SSRC, will not spawn new associations. - rsid_sinks_.erase(it_range.first, it_range.second); + if (!packet.GetExtension(&rsid)) { + return; } + + auto it = rsid_sinks_.find(rsid); + if (it == rsid_sinks_.end()) { + // Might be unknown, or we might have already associated this RSID + // with a sink. + return; + } + + // If a sink is associated with an RSID, we should associate it with + // this SSRC. + if (!AddSink(packet.Ssrc(), it->second)) { + // In the faulty case of RSIDs mapped to SSRCs which are already associated + // with a sink, avoid propagating the problem to the resolution observers. + LOG(LS_WARNING) << "RSID (" << rsid << ") resolved to preconfigured SSRC (" + << packet.Ssrc() << ")."; + return; + } + + // We make the assumption that observers are only interested in notifications + // for RSIDs which are registered with this module. (RTCP sinks are normally + // created with RTP sinks.) + NotifyObserversOfRsidResolution(rsid, packet.Ssrc()); + + // This RSID cannot later be associated with another SSRC. + rsid_sinks_.erase(it); } void RtpDemuxer::NotifyObserversOfRsidResolution(const std::string& rsid, diff --git a/webrtc/call/rtp_demuxer.h b/webrtc/call/rtp_demuxer.h index e557c4dbcc..dccf0b69a1 100644 --- a/webrtc/call/rtp_demuxer.h +++ b/webrtc/call/rtp_demuxer.h @@ -31,18 +31,22 @@ class RtpDemuxer { RtpDemuxer(); ~RtpDemuxer(); - // Registers a sink. The same sink can be registered for multiple ssrcs, and - // the same ssrc can have multiple sinks. Null pointer is not allowed. - void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink); + // Registers a sink. Multiple SSRCs may be mapped to the same sink, but + // each SSRC may only be mapped to one sink. The return value reports + // whether the association has been recorded or rejected. Rejection may occur + // if the SSRC has already been associated with a sink. The previously added + // sink is *not* forgotten. + bool AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink); - // Registers a sink's association to an RSID. Null pointer is not allowed. + // Registers a sink's association to an RSID. Only one sink may be associated + // with a given RSID. Null pointer is not allowed. void AddSink(const std::string& rsid, RtpPacketSinkInterface* sink); // Removes a sink. Return value reports if anything was actually removed. // Null pointer is not allowed. bool RemoveSink(const RtpPacketSinkInterface* sink); - // Returns true if at least one matching sink was found. + // Handles RTP packets. Returns true if at least one matching sink was found. bool OnRtpPacket(const RtpPacketReceived& packet); // Allows other objects to be notified when RSID-SSRC associations are @@ -53,12 +57,6 @@ class RtpDemuxer { void DeregisterRsidResolutionObserver(const RsidResolutionObserver* observer); private: - // Records a sink<->SSRC association. This can happen by explicit - // configuration by AddSink(ssrc...), or by inferred configuration from an - // RSID-based configuration which is resolved to an SSRC upon - // packet reception. - void RecordSsrcToSinkAssociation(uint32_t ssrc, RtpPacketSinkInterface* sink); - // Find the associations of RSID to SSRCs. void ResolveRsidToSsrcAssociations(const RtpPacketReceived& packet); @@ -67,13 +65,13 @@ class RtpDemuxer { // This records the association SSRCs to sinks. Other associations, such // as by RSID, also end up here once the RSID, etc., is resolved to an SSRC. - std::multimap ssrc_sinks_; + std::map ssrc_sinks_; // A sink may be associated with an RSID - RTP Stream ID. This tag has a // one-to-one association with an SSRC, but that SSRC is not yet known. // When it becomes known, the association of the sink to the RSID is deleted // from this container, and moved into |ssrc_sinks_|. - std::multimap rsid_sinks_; + std::map rsid_sinks_; // Observers which will be notified when an RSID association to an SSRC is // resolved by this object. diff --git a/webrtc/call/rtp_demuxer_unittest.cc b/webrtc/call/rtp_demuxer_unittest.cc index a2b4578a71..f9196caef2 100644 --- a/webrtc/call/rtp_demuxer_unittest.cc +++ b/webrtc/call/rtp_demuxer_unittest.cc @@ -76,6 +76,17 @@ std::unique_ptr CreateRtpPacketReceivedWithRsid( return packet; } +TEST(RtpDemuxerTest, CanAddSinkBySsrc) { + RtpDemuxer demuxer; + MockRtpPacketSink sink; + constexpr uint32_t ssrc = 1; + + EXPECT_TRUE(demuxer.AddSink(ssrc, &sink)); + + // Test tear-down + demuxer.RemoveSink(&sink); +} + TEST(RtpDemuxerTest, OnRtpPacketCalledOnCorrectSinkBySsrc) { RtpDemuxer demuxer; @@ -144,29 +155,6 @@ TEST(RtpDemuxerTest, PacketsDeliveredInRightOrder) { demuxer.RemoveSink(&sink); } -TEST(RtpDemuxerTest, MultipleSinksMappedToSameSsrc) { - RtpDemuxer demuxer; - - MockRtpPacketSink sinks[3]; - constexpr uint32_t ssrc = 404; - for (auto& sink : sinks) { - demuxer.AddSink(ssrc, &sink); - } - - // Reception of an RTP packet associated with the shared SSRC triggers the - // callback on all of the sinks associated with it. - auto packet = CreateRtpPacketReceived(ssrc); - for (auto& sink : sinks) { - EXPECT_CALL(sink, OnRtpPacket(SamePacketAs(*packet))); - } - EXPECT_TRUE(demuxer.OnRtpPacket(*packet)); - - // Test tear-down - for (const auto& sink : sinks) { - demuxer.RemoveSink(&sink); - } -} - TEST(RtpDemuxerTest, SinkMappedToMultipleSsrcs) { RtpDemuxer demuxer; @@ -224,14 +212,62 @@ TEST(RtpDemuxerTest, NoCallbackOnSsrcSinkRemovedAfterFirstPacket) { EXPECT_FALSE(demuxer.OnRtpPacket(*packet)); } -TEST(RtpDemuxerTest, RepeatedSsrcAssociationsDoNotTriggerRepeatedCallbacks) { +TEST(RtpDemuxerTest, AddSinkFailsIfCalledForTwoSinks) { + RtpDemuxer demuxer; + MockRtpPacketSink sink_a; + MockRtpPacketSink sink_b; + constexpr uint32_t ssrc = 1; + ASSERT_TRUE(demuxer.AddSink(ssrc, &sink_a)); + + EXPECT_FALSE(demuxer.AddSink(ssrc, &sink_b)); + + // Test tear-down + demuxer.RemoveSink(&sink_a); +} + +// An SSRC may only be mapped to a single sink. However, since configuration +// of this associations might come from the network, we need to fail gracefully. +TEST(RtpDemuxerTest, OnlyOneSinkPerSsrcGetsOnRtpPacketTriggered) { + RtpDemuxer demuxer; + + MockRtpPacketSink sinks[3]; + constexpr uint32_t ssrc = 404; + ASSERT_TRUE(demuxer.AddSink(ssrc, &sinks[0])); + ASSERT_FALSE(demuxer.AddSink(ssrc, &sinks[1])); + ASSERT_FALSE(demuxer.AddSink(ssrc, &sinks[2])); + + // The first sink associated with the SSRC remains active; other sinks + // were not really added, and so do not get OnRtpPacket() called. + auto packet = CreateRtpPacketReceived(ssrc); + EXPECT_CALL(sinks[0], OnRtpPacket(SamePacketAs(*packet))).Times(1); + EXPECT_CALL(sinks[1], OnRtpPacket(_)).Times(0); + EXPECT_CALL(sinks[2], OnRtpPacket(_)).Times(0); + ASSERT_TRUE(demuxer.OnRtpPacket(*packet)); + + // Test tear-down + demuxer.RemoveSink(&sinks[0]); +} + +TEST(RtpDemuxerTest, AddSinkFailsIfCalledTwiceEvenIfSameSink) { + RtpDemuxer demuxer; + MockRtpPacketSink sink; + constexpr uint32_t ssrc = 1; + ASSERT_TRUE(demuxer.AddSink(ssrc, &sink)); + + EXPECT_FALSE(demuxer.AddSink(ssrc, &sink)); + + // Test tear-down + demuxer.RemoveSink(&sink); +} + +TEST(RtpDemuxerTest, NoRepeatedCallbackOnRepeatedAddSinkForSameSink) { RtpDemuxer demuxer; constexpr uint32_t ssrc = 111; MockRtpPacketSink sink; - demuxer.AddSink(ssrc, &sink); - demuxer.AddSink(ssrc, &sink); + ASSERT_TRUE(demuxer.AddSink(ssrc, &sink)); + ASSERT_FALSE(demuxer.AddSink(ssrc, &sink)); auto packet = CreateRtpPacketReceived(ssrc); EXPECT_CALL(sink, OnRtpPacket(SamePacketAs(*packet))).Times(1); @@ -427,31 +463,6 @@ TEST(RtpDemuxerTest, MultipleRsidsOnSameSink) { demuxer.RemoveSink(&sink); } -TEST(RtpDemuxerTest, RsidUsedByMultipleSinks) { - RtpDemuxer demuxer; - - MockRtpPacketSink sinks[3]; - const std::string shared_rsid = "a"; - - for (MockRtpPacketSink& sink : sinks) { - demuxer.AddSink(shared_rsid, &sink); - } - - constexpr uint32_t shared_ssrc = 888; - auto packet = CreateRtpPacketReceivedWithRsid(shared_rsid, shared_ssrc); - - for (auto& sink : sinks) { - EXPECT_CALL(sink, OnRtpPacket(SamePacketAs(*packet))).Times(1); - } - - EXPECT_TRUE(demuxer.OnRtpPacket(*packet)); - - // Test tear-down - for (MockRtpPacketSink& sink : sinks) { - demuxer.RemoveSink(&sink); - } -} - TEST(RtpDemuxerTest, SinkWithBothRsidAndSsrcAssociations) { RtpDemuxer demuxer; @@ -496,12 +507,16 @@ TEST(RtpDemuxerTest, AssociatingByRsidAndBySsrcCannotTriggerDoubleCall) { demuxer.RemoveSink(&sink); } -TEST(RtpDemuxerTest, RsidObserversInformedOfResolutions) { +TEST(RtpDemuxerTest, RsidObserversInformedOfResolutionsOfTrackedRsids) { RtpDemuxer demuxer; constexpr uint32_t ssrc = 111; const std::string rsid = "a"; + // Only RSIDs which the demuxer knows may be resolved. + NiceMock sink; + demuxer.AddSink(rsid, &sink); + MockRsidResolutionObserver rsid_resolution_observers[3]; for (auto& observer : rsid_resolution_observers) { demuxer.RegisterRsidResolutionObserver(&observer); @@ -515,6 +530,112 @@ TEST(RtpDemuxerTest, RsidObserversInformedOfResolutions) { for (auto& observer : rsid_resolution_observers) { demuxer.DeregisterRsidResolutionObserver(&observer); } + demuxer.RemoveSink(&sink); +} + +TEST(RtpDemuxerTest, RsidObserversNotInformedOfResolutionsOfUntrackedRsids) { + RtpDemuxer demuxer; + + constexpr uint32_t ssrc = 111; + const std::string rsid = "a"; + + MockRsidResolutionObserver rsid_resolution_observers[3]; + for (auto& observer : rsid_resolution_observers) { + demuxer.RegisterRsidResolutionObserver(&observer); + EXPECT_CALL(observer, OnRsidResolved(rsid, ssrc)).Times(0); + } + + // The expected calls to OnRsidResolved() will be triggered by this. + demuxer.OnRtpPacket(*CreateRtpPacketReceivedWithRsid(rsid, ssrc)); + + // Test tear-down + for (auto& observer : rsid_resolution_observers) { + demuxer.DeregisterRsidResolutionObserver(&observer); + } +} + +// If one sink is associated with SSRC x, and another sink with RSID y, we +// should never observe RSID x being resolved to SSRC x, or else we'd end +// up with one SSRC mapped to two sinks. However, if such faulty input +// ever reaches us, we should handle it gracefully - not crash, and keep the +// packets routed only to the SSRC sink. +TEST(RtpDemuxerTest, PacketFittingBothRsidSinkAndSsrcSinkGivenOnlyToSsrcSink) { + RtpDemuxer demuxer; + + constexpr uint32_t ssrc = 111; + MockRtpPacketSink ssrc_sink; + demuxer.AddSink(ssrc, &ssrc_sink); + + const std::string rsid = "a"; + MockRtpPacketSink rsid_sink; + demuxer.AddSink(rsid, &rsid_sink); + + auto packet = CreateRtpPacketReceivedWithRsid(rsid, ssrc); + EXPECT_CALL(ssrc_sink, OnRtpPacket(SamePacketAs(*packet))).Times(1); + EXPECT_CALL(rsid_sink, OnRtpPacket(SamePacketAs(*packet))).Times(0); + demuxer.OnRtpPacket(*packet); + + // Test tear-down + demuxer.RemoveSink(&ssrc_sink); + demuxer.RemoveSink(&rsid_sink); +} + +TEST(RtpDemuxerTest, + PacketFittingBothRsidSinkAndSsrcSinkDoesNotTriggerResolutionCallbacks) { + RtpDemuxer demuxer; + + constexpr uint32_t ssrc = 111; + NiceMock ssrc_sink; + demuxer.AddSink(ssrc, &ssrc_sink); + + const std::string rsid = "a"; + NiceMock rsid_sink; + demuxer.AddSink(rsid, &rsid_sink); + + MockRsidResolutionObserver observer; + demuxer.RegisterRsidResolutionObserver(&observer); + + auto packet = CreateRtpPacketReceivedWithRsid(rsid, ssrc); + EXPECT_CALL(observer, OnRsidResolved(_, _)).Times(0); + demuxer.OnRtpPacket(*packet); + + // Test tear-down + demuxer.RemoveSink(&ssrc_sink); + demuxer.RemoveSink(&rsid_sink); +} + +// We're not expecting RSIDs to be resolved to SSRCs which were previously +// mapped to sinks, and make no guarantees except for graceful handling. +TEST(RtpDemuxerTest, GracefullyHandleRsidBeingMappedToPrevouslyAssociatedSsrc) { + RtpDemuxer demuxer; + + constexpr uint32_t ssrc = 111; + NiceMock ssrc_sink; + demuxer.AddSink(ssrc, &ssrc_sink); + + const std::string rsid = "a"; + MockRtpPacketSink rsid_sink; + demuxer.AddSink(rsid, &rsid_sink); + + MockRsidResolutionObserver observer; + demuxer.RegisterRsidResolutionObserver(&observer); + + // The SSRC was mapped to an SSRC sink, but was even active (packets flowed + // over it). + auto packet = CreateRtpPacketReceivedWithRsid(rsid, ssrc); + demuxer.OnRtpPacket(*packet); + + // If the SSRC sink is ever removed, the RSID sink *might* receive indications + // of packets, and observers *might* be informed. Only graceful handling + // is guaranteed. + demuxer.RemoveSink(&ssrc_sink); + EXPECT_CALL(rsid_sink, OnRtpPacket(SamePacketAs(*packet))).Times(AtLeast(0)); + EXPECT_CALL(observer, OnRsidResolved(rsid, ssrc)).Times(AtLeast(0)); + demuxer.OnRtpPacket(*packet); + + // Test tear-down + demuxer.DeregisterRsidResolutionObserver(&observer); + demuxer.RemoveSink(&rsid_sink); } TEST(RtpDemuxerTest, DeregisteredRsidObserversNotInformedOfResolutions) { @@ -554,12 +675,14 @@ TEST(RtpDemuxerTest, DeregisteredRsidObserversNotInformedOfResolutions) { TEST(RtpDemuxerTest, RsidMustBeNonEmpty) { RtpDemuxer demuxer; MockRtpPacketSink sink; + EXPECT_DEATH(demuxer.AddSink("", &sink), ""); } TEST(RtpDemuxerTest, RsidMustBeAlphaNumeric) { RtpDemuxer demuxer; MockRtpPacketSink sink; + EXPECT_DEATH(demuxer.AddSink("a_3", &sink), ""); } @@ -567,23 +690,45 @@ TEST(RtpDemuxerTest, RsidMustNotExceedMaximumLength) { RtpDemuxer demuxer; MockRtpPacketSink sink; std::string rsid(StreamId::kMaxSize + 1, 'a'); + EXPECT_DEATH(demuxer.AddSink(rsid, &sink), ""); } TEST(RtpDemuxerTest, RepeatedRsidAssociationsDisallowed) { + RtpDemuxer demuxer; + MockRtpPacketSink sink_a; + MockRtpPacketSink sink_b; + + const std::string rsid = "a"; + demuxer.AddSink(rsid, &sink_a); + + EXPECT_DEATH(demuxer.AddSink(rsid, &sink_b), ""); + + // Test tear-down + demuxer.RemoveSink(&sink_a); +} + +TEST(RtpDemuxerTest, RepeatedRsidAssociationsDisallowedEvenIfSameSink) { RtpDemuxer demuxer; MockRtpPacketSink sink; - demuxer.AddSink("a", &sink); - EXPECT_DEATH(demuxer.AddSink("a", &sink), ""); + + const std::string rsid = "a"; + demuxer.AddSink(rsid, &sink); + + EXPECT_DEATH(demuxer.AddSink(rsid, &sink), ""); + + // Test tear-down demuxer.RemoveSink(&sink); } -TEST(RtpDemuxerTest, - DoubleRegisterationOfNeverRegisteredRsidResolutionObserverDisallowed) { +TEST(RtpDemuxerTest, DoubleRegisterationOfRsidResolutionObserverDisallowed) { RtpDemuxer demuxer; MockRsidResolutionObserver observer; demuxer.RegisterRsidResolutionObserver(&observer); + EXPECT_DEATH(demuxer.RegisterRsidResolutionObserver(&observer), ""); + + // Test tear-down demuxer.DeregisterRsidResolutionObserver(&observer); } @@ -591,6 +736,7 @@ TEST(RtpDemuxerTest, DregisterationOfNeverRegisteredRsidResolutionObserverDisallowed) { RtpDemuxer demuxer; MockRsidResolutionObserver observer; + EXPECT_DEATH(demuxer.DeregisterRsidResolutionObserver(&observer), ""); } diff --git a/webrtc/call/rtp_rtcp_demuxer_helper.h b/webrtc/call/rtp_rtcp_demuxer_helper.h index 4791e55ca3..85204365fb 100644 --- a/webrtc/call/rtp_rtcp_demuxer_helper.h +++ b/webrtc/call/rtp_rtcp_demuxer_helper.h @@ -21,6 +21,7 @@ namespace webrtc { +// TODO(eladalon): Remove this in the next CL. template bool MultimapAssociationExists(const Container& multimap, const typename Container::key_type& key, @@ -31,6 +32,7 @@ bool MultimapAssociationExists(const Container& multimap, [val](Reference elem) { return elem.second == val; }); } +// TODO(eladalon): Remove this in the next CL. template size_t RemoveFromMultimapByValue(Container* multimap, const Value& value) { size_t count = 0; @@ -45,11 +47,26 @@ size_t RemoveFromMultimapByValue(Container* multimap, const Value& value) { return count; } +template +size_t RemoveFromMapByValue(Map* map, const Value& value) { + size_t count = 0; + for (auto it = map->begin(); it != map->end();) { + if (it->second == value) { + it = map->erase(it); + ++count; + } else { + ++it; + } + } + return count; +} + template bool ContainerHasKey(const Container& c, const Key& k) { return std::find(c.cbegin(), c.cend(), k) != c.cend(); } +// TODO(eladalon): Remove this in the next CL. template bool MultimapHasValue(const Container& c, const typename Container::mapped_type& v) { @@ -59,6 +76,14 @@ bool MultimapHasValue(const Container& c, return std::any_of(c.cbegin(), c.cend(), predicate); } +template +bool MapHasValue(const Map& map, const typename Map::mapped_type& value) { + auto predicate = [value](const typename Map::value_type& it) { + return it.second == value; + }; + return std::any_of(map.cbegin(), map.cend(), predicate); +} + rtc::Optional ParseRtcpPacketSenderSsrc( rtc::ArrayView packet); diff --git a/webrtc/call/rtp_stream_receiver_controller.cc b/webrtc/call/rtp_stream_receiver_controller.cc index 1238665352..94fa83b60e 100644 --- a/webrtc/call/rtp_stream_receiver_controller.cc +++ b/webrtc/call/rtp_stream_receiver_controller.cc @@ -9,6 +9,8 @@ */ #include "webrtc/call/rtp_stream_receiver_controller.h" + +#include "webrtc/rtc_base/logging.h" #include "webrtc/rtc_base/ptr_util.h" namespace webrtc { @@ -18,7 +20,11 @@ RtpStreamReceiverController::Receiver::Receiver( uint32_t ssrc, RtpPacketSinkInterface* sink) : controller_(controller), sink_(sink) { - controller_->AddSink(ssrc, sink_); + const bool sink_added = controller_->AddSink(ssrc, sink_); + if (!sink_added) { + LOG(LS_ERROR) << "RtpStreamReceiverController::Receiver::Receiver: Sink " + << "could not be added for SSRC=" << ssrc << "."; + } } RtpStreamReceiverController::Receiver::~Receiver() { @@ -43,7 +49,7 @@ bool RtpStreamReceiverController::OnRtpPacket(const RtpPacketReceived& packet) { return demuxer_.OnRtpPacket(packet); } -void RtpStreamReceiverController::AddSink(uint32_t ssrc, +bool RtpStreamReceiverController::AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) { rtc::CritScope cs(&lock_); return demuxer_.AddSink(ssrc, sink); diff --git a/webrtc/call/rtp_stream_receiver_controller.h b/webrtc/call/rtp_stream_receiver_controller.h index 0fbf669513..4583b9d479 100644 --- a/webrtc/call/rtp_stream_receiver_controller.h +++ b/webrtc/call/rtp_stream_receiver_controller.h @@ -38,7 +38,7 @@ class RtpStreamReceiverController RtpPacketSinkInterface* sink) override; // Thread-safe wrappers for the corresponding RtpDemuxer methods. - void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) override; + bool AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) override; size_t RemoveSink(const RtpPacketSinkInterface* sink) override; // TODO(nisse): Not yet responsible for parsing. diff --git a/webrtc/call/rtp_stream_receiver_controller_interface.h b/webrtc/call/rtp_stream_receiver_controller_interface.h index 51d25a525e..94b4f10852 100644 --- a/webrtc/call/rtp_stream_receiver_controller_interface.h +++ b/webrtc/call/rtp_stream_receiver_controller_interface.h @@ -38,7 +38,7 @@ class RtpStreamReceiverControllerInterface { uint32_t ssrc, RtpPacketSinkInterface* sink) = 0; // For registering additional sinks, needed for FlexFEC. - virtual void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) = 0; + virtual bool AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) = 0; virtual size_t RemoveSink(const RtpPacketSinkInterface* sink) = 0; };