zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert of SSRC and RSID may only refer to one sink each in RtpDemuxer (patchset #15 id:280001 of https://codereview.webrtc.org/2968693002/ )

Browse Source 
Reason for revert:
Some internal tests keep failing after this change. Try to fix it by reverting it. Will reland it if this isn't the root cause.

Original issue's description:
> 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}
> Committed: 7b7e06fd23

TBR=nisse@webrtc.org,danilchap@webrtc.org,perkj@webrtc.org,stefan@webrtc.org,holmer@google.com,deadbeef@webrtc.org,pthatcher@webrtc.org,steveanton@webrtc.org,eladalon@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7135

Review-Url: https://codereview.webrtc.org/2993633002
Cr-Commit-Position: refs/heads/master@{#19239}
This commit is contained in:
zhihuang
2017-08-03 10:09:44 -07:00
committed by Commit Bot
parent 7eaa62bc48
commit 59b603fbed
7 changed files with 105 additions and 285 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
webrtc/call/rtp_demuxer.cc
View File

@ -27,32 +27,41 @@ RtpDemuxer::~RtpDemuxer() {
RTC_DCHECK(rsid_sinks_.empty()); RTC_DCHECK(rsid_sinks_.empty());
} }
bool RtpDemuxer::AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) { void RtpDemuxer::AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) {
RTC_DCHECK(sink); RTC_DCHECK(sink);
// The association might already have been set by a different RecordSsrcToSinkAssociation(ssrc, sink);
// 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, void RtpDemuxer::AddSink(const std::string& rsid,
RtpPacketSinkInterface* sink) { RtpPacketSinkInterface* sink) {
RTC_DCHECK(StreamId::IsLegalName(rsid)); RTC_DCHECK(StreamId::IsLegalName(rsid));
RTC_DCHECK(sink); RTC_DCHECK(sink);
RTC_DCHECK(rsid_sinks_.find(rsid) == rsid_sinks_.cend()); RTC_DCHECK(!MultimapAssociationExists(rsid_sinks_, rsid, sink));
rsid_sinks_.emplace(rsid, sink); rsid_sinks_.emplace(rsid, sink);
} }
bool RtpDemuxer::RemoveSink(const RtpPacketSinkInterface* sink) { bool RtpDemuxer::RemoveSink(const RtpPacketSinkInterface* sink) {
RTC_DCHECK(sink); RTC_DCHECK(sink);
return (RemoveFromMapByValue(&ssrc_sinks_, sink) + return (RemoveFromMultimapByValue(&ssrc_sinks_, sink) +
RemoveFromMapByValue(&rsid_sinks_, sink)) > 0; 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);
}
} }
bool RtpDemuxer::OnRtpPacket(const RtpPacketReceived& packet) { 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); ResolveRsidToSsrcAssociations(packet);
auto it_range = ssrc_sinks_.equal_range(packet.Ssrc()); auto it_range = ssrc_sinks_.equal_range(packet.Ssrc());
@ -82,34 +91,20 @@ void RtpDemuxer::DeregisterRsidResolutionObserver(
void RtpDemuxer::ResolveRsidToSsrcAssociations( void RtpDemuxer::ResolveRsidToSsrcAssociations(
const RtpPacketReceived& packet) { const RtpPacketReceived& packet) {
std::string rsid; std::string rsid;
if (!packet.GetExtension<RtpStreamId>(&rsid)) { if (packet.GetExtension<RtpStreamId>(&rsid)) {
return; // 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);
} }
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()); NotifyObserversOfRsidResolution(rsid, packet.Ssrc());
// This RSID cannot later be associated with another SSRC. // To prevent memory-overuse attacks, forget this RSID. Future packets
rsid_sinks_.erase(it); // with this RSID, but a different SSRC, will not spawn new associations.
rsid_sinks_.erase(it_range.first, it_range.second);
}
} }
void RtpDemuxer::NotifyObserversOfRsidResolution(const std::string& rsid, void RtpDemuxer::NotifyObserversOfRsidResolution(const std::string& rsid,

24
webrtc/call/rtp_demuxer.h
View File

@ -31,22 +31,18 @@ class RtpDemuxer {
RtpDemuxer(); RtpDemuxer();
~RtpDemuxer(); ~RtpDemuxer();
// Registers a sink. Multiple SSRCs may be mapped to the same sink, but // Registers a sink. The same sink can be registered for multiple ssrcs, and
// each SSRC may only be mapped to one sink. The return value reports // the same ssrc can have multiple sinks. Null pointer is not allowed.
// whether the association has been recorded or rejected. Rejection may occur void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink);
// 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. Only one sink may be associated // Registers a sink's association to an RSID. Null pointer is not allowed.
// with a given RSID. Null pointer is not allowed.
void AddSink(const std::string& rsid, RtpPacketSinkInterface* sink); void AddSink(const std::string& rsid, RtpPacketSinkInterface* sink);
// Removes a sink. Return value reports if anything was actually removed. // Removes a sink. Return value reports if anything was actually removed.
// Null pointer is not allowed. // Null pointer is not allowed.
bool RemoveSink(const RtpPacketSinkInterface* sink); bool RemoveSink(const RtpPacketSinkInterface* sink);
// Handles RTP packets. Returns true if at least one matching sink was found. // Returns true if at least one matching sink was found.
bool OnRtpPacket(const RtpPacketReceived& packet); bool OnRtpPacket(const RtpPacketReceived& packet);
// Allows other objects to be notified when RSID-SSRC associations are // Allows other objects to be notified when RSID-SSRC associations are
@ -57,6 +53,12 @@ class RtpDemuxer {
void DeregisterRsidResolutionObserver(const RsidResolutionObserver* observer); void DeregisterRsidResolutionObserver(const RsidResolutionObserver* observer);
private: 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. // Find the associations of RSID to SSRCs.
void ResolveRsidToSsrcAssociations(const RtpPacketReceived& packet); void ResolveRsidToSsrcAssociations(const RtpPacketReceived& packet);
@ -65,13 +67,13 @@ class RtpDemuxer {
// This records the association SSRCs to sinks. Other associations, such // 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. // as by RSID, also end up here once the RSID, etc., is resolved to an SSRC.
std::map<uint32_t, RtpPacketSinkInterface*> ssrc_sinks_; std::multimap<uint32_t, RtpPacketSinkInterface*> ssrc_sinks_;
// A sink may be associated with an RSID - RTP Stream ID. This tag has a // 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. // 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 // When it becomes known, the association of the sink to the RSID is deleted
// from this container, and moved into |ssrc_sinks_|. // from this container, and moved into |ssrc_sinks_|.
std::map<std::string, RtpPacketSinkInterface*> rsid_sinks_; std::multimap<std::string, RtpPacketSinkInterface*> rsid_sinks_;
// Observers which will be notified when an RSID association to an SSRC is // Observers which will be notified when an RSID association to an SSRC is
// resolved by this object. // resolved by this object.

258
webrtc/call/rtp_demuxer_unittest.cc
View File

@ -76,17 +76,6 @@ std::unique_ptr<RtpPacketReceived> CreateRtpPacketReceivedWithRsid(
return packet; 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) { TEST(RtpDemuxerTest, OnRtpPacketCalledOnCorrectSinkBySsrc) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
@ -155,6 +144,29 @@ TEST(RtpDemuxerTest, PacketsDeliveredInRightOrder) {
demuxer.RemoveSink(&sink); 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) { TEST(RtpDemuxerTest, SinkMappedToMultipleSsrcs) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
@ -212,62 +224,14 @@ TEST(RtpDemuxerTest, NoCallbackOnSsrcSinkRemovedAfterFirstPacket) {
EXPECT_FALSE(demuxer.OnRtpPacket(*packet)); EXPECT_FALSE(demuxer.OnRtpPacket(*packet));
} }
TEST(RtpDemuxerTest, AddSinkFailsIfCalledForTwoSinks) { TEST(RtpDemuxerTest, RepeatedSsrcAssociationsDoNotTriggerRepeatedCallbacks) {
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; RtpDemuxer demuxer;
constexpr uint32_t ssrc = 111; constexpr uint32_t ssrc = 111;
MockRtpPacketSink sink; MockRtpPacketSink sink;
ASSERT_TRUE(demuxer.AddSink(ssrc, &sink)); demuxer.AddSink(ssrc, &sink);
ASSERT_FALSE(demuxer.AddSink(ssrc, &sink)); demuxer.AddSink(ssrc, &sink);
auto packet = CreateRtpPacketReceived(ssrc); auto packet = CreateRtpPacketReceived(ssrc);
EXPECT_CALL(sink, OnRtpPacket(SamePacketAs(*packet))).Times(1); EXPECT_CALL(sink, OnRtpPacket(SamePacketAs(*packet))).Times(1);
@ -463,6 +427,31 @@ TEST(RtpDemuxerTest, MultipleRsidsOnSameSink) {
demuxer.RemoveSink(&sink); 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) { TEST(RtpDemuxerTest, SinkWithBothRsidAndSsrcAssociations) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
@ -507,16 +496,12 @@ TEST(RtpDemuxerTest, AssociatingByRsidAndBySsrcCannotTriggerDoubleCall) {
demuxer.RemoveSink(&sink); demuxer.RemoveSink(&sink);
} }
TEST(RtpDemuxerTest, RsidObserversInformedOfResolutionsOfTrackedRsids) { TEST(RtpDemuxerTest, RsidObserversInformedOfResolutions) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
constexpr uint32_t ssrc = 111; constexpr uint32_t ssrc = 111;
const std::string rsid = "a"; const std::string rsid = "a";
// Only RSIDs which the demuxer knows may be resolved.
NiceMock<MockRtpPacketSink> sink;
demuxer.AddSink(rsid, &sink);
MockRsidResolutionObserver rsid_resolution_observers[3]; MockRsidResolutionObserver rsid_resolution_observers[3];
for (auto& observer : rsid_resolution_observers) { for (auto& observer : rsid_resolution_observers) {
demuxer.RegisterRsidResolutionObserver(&observer); demuxer.RegisterRsidResolutionObserver(&observer);
@ -530,112 +515,6 @@ TEST(RtpDemuxerTest, RsidObserversInformedOfResolutionsOfTrackedRsids) {
for (auto& observer : rsid_resolution_observers) { for (auto& observer : rsid_resolution_observers) {
demuxer.DeregisterRsidResolutionObserver(&observer); 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<MockRtpPacketSink> ssrc_sink;
demuxer.AddSink(ssrc, &ssrc_sink);
const std::string rsid = "a";
NiceMock<MockRtpPacketSink> 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<MockRtpPacketSink> 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) { TEST(RtpDemuxerTest, DeregisteredRsidObserversNotInformedOfResolutions) {
@ -675,14 +554,12 @@ TEST(RtpDemuxerTest, DeregisteredRsidObserversNotInformedOfResolutions) {
TEST(RtpDemuxerTest, RsidMustBeNonEmpty) { TEST(RtpDemuxerTest, RsidMustBeNonEmpty) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
MockRtpPacketSink sink; MockRtpPacketSink sink;
EXPECT_DEATH(demuxer.AddSink("", &sink), ""); EXPECT_DEATH(demuxer.AddSink("", &sink), "");
} }
TEST(RtpDemuxerTest, RsidMustBeAlphaNumeric) { TEST(RtpDemuxerTest, RsidMustBeAlphaNumeric) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
MockRtpPacketSink sink; MockRtpPacketSink sink;
EXPECT_DEATH(demuxer.AddSink("a_3", &sink), ""); EXPECT_DEATH(demuxer.AddSink("a_3", &sink), "");
} }
@ -690,45 +567,23 @@ TEST(RtpDemuxerTest, RsidMustNotExceedMaximumLength) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
MockRtpPacketSink sink; MockRtpPacketSink sink;
std::string rsid(StreamId::kMaxSize + 1, 'a'); std::string rsid(StreamId::kMaxSize + 1, 'a');
EXPECT_DEATH(demuxer.AddSink(rsid, &sink), ""); EXPECT_DEATH(demuxer.AddSink(rsid, &sink), "");
} }
TEST(RtpDemuxerTest, RepeatedRsidAssociationsDisallowed) { 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; RtpDemuxer demuxer;
MockRtpPacketSink sink; MockRtpPacketSink sink;
demuxer.AddSink("a", &sink);
const std::string rsid = "a"; EXPECT_DEATH(demuxer.AddSink("a", &sink), "");
demuxer.AddSink(rsid, &sink);
EXPECT_DEATH(demuxer.AddSink(rsid, &sink), "");
// Test tear-down
demuxer.RemoveSink(&sink); demuxer.RemoveSink(&sink);
} }
TEST(RtpDemuxerTest, DoubleRegisterationOfRsidResolutionObserverDisallowed) { TEST(RtpDemuxerTest,
DoubleRegisterationOfNeverRegisteredRsidResolutionObserverDisallowed) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
MockRsidResolutionObserver observer; MockRsidResolutionObserver observer;
demuxer.RegisterRsidResolutionObserver(&observer); demuxer.RegisterRsidResolutionObserver(&observer);
EXPECT_DEATH(demuxer.RegisterRsidResolutionObserver(&observer), ""); EXPECT_DEATH(demuxer.RegisterRsidResolutionObserver(&observer), "");
// Test tear-down
demuxer.DeregisterRsidResolutionObserver(&observer); demuxer.DeregisterRsidResolutionObserver(&observer);
} }
@ -736,7 +591,6 @@ TEST(RtpDemuxerTest,
DregisterationOfNeverRegisteredRsidResolutionObserverDisallowed) { DregisterationOfNeverRegisteredRsidResolutionObserverDisallowed) {
RtpDemuxer demuxer; RtpDemuxer demuxer;
MockRsidResolutionObserver observer; MockRsidResolutionObserver observer;
EXPECT_DEATH(demuxer.DeregisterRsidResolutionObserver(&observer), ""); EXPECT_DEATH(demuxer.DeregisterRsidResolutionObserver(&observer), "");
} }

25
webrtc/call/rtp_rtcp_demuxer_helper.h
View File

@ -21,7 +21,6 @@
namespace webrtc { namespace webrtc {
// TODO(eladalon): Remove this in the next CL.
template <typename Container> template <typename Container>
bool MultimapAssociationExists(const Container& multimap, bool MultimapAssociationExists(const Container& multimap,
const typename Container::key_type& key, const typename Container::key_type& key,
@ -32,7 +31,6 @@ bool MultimapAssociationExists(const Container& multimap,
[val](Reference elem) { return elem.second == val; }); [val](Reference elem) { return elem.second == val; });
} }
// TODO(eladalon): Remove this in the next CL.
template <typename Container, typename Value> template <typename Container, typename Value>
size_t RemoveFromMultimapByValue(Container* multimap, const Value& value) { size_t RemoveFromMultimapByValue(Container* multimap, const Value& value) {
size_t count = 0; size_t count = 0;
@ -47,26 +45,11 @@ size_t RemoveFromMultimapByValue(Container* multimap, const Value& value) {
return count; return count;
} }
template <typename Map, typename Value>
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 <typename Container, typename Key> template <typename Container, typename Key>
bool ContainerHasKey(const Container& c, const Key& k) { bool ContainerHasKey(const Container& c, const Key& k) {
return std::find(c.cbegin(), c.cend(), k) != c.cend(); return std::find(c.cbegin(), c.cend(), k) != c.cend();
} }
// TODO(eladalon): Remove this in the next CL.
template <typename Container> template <typename Container>
bool MultimapHasValue(const Container& c, bool MultimapHasValue(const Container& c,
const typename Container::mapped_type& v) { const typename Container::mapped_type& v) {
@ -76,14 +59,6 @@ bool MultimapHasValue(const Container& c,
return std::any_of(c.cbegin(), c.cend(), predicate); return std::any_of(c.cbegin(), c.cend(), predicate);
} }
template <typename Map>
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<uint32_t> ParseRtcpPacketSenderSsrc( rtc::Optional<uint32_t> ParseRtcpPacketSenderSsrc(
rtc::ArrayView<const uint8_t> packet); rtc::ArrayView<const uint8_t> packet);

10
webrtc/call/rtp_stream_receiver_controller.cc
View File

@ -9,8 +9,6 @@
*/ */
#include "webrtc/call/rtp_stream_receiver_controller.h" #include "webrtc/call/rtp_stream_receiver_controller.h"
#include "webrtc/rtc_base/logging.h"
#include "webrtc/rtc_base/ptr_util.h" #include "webrtc/rtc_base/ptr_util.h"
namespace webrtc { namespace webrtc {
@ -20,11 +18,7 @@ RtpStreamReceiverController::Receiver::Receiver(
uint32_t ssrc, uint32_t ssrc,
RtpPacketSinkInterface* sink) RtpPacketSinkInterface* sink)
: controller_(controller), sink_(sink) { : controller_(controller), sink_(sink) {
const bool sink_added = controller_->AddSink(ssrc, sink_); controller_->AddSink(ssrc, sink_);
if (!sink_added) {
LOG(LS_ERROR) << "RtpStreamReceiverController::Receiver::Receiver: Sink "
<< "could not be added for SSRC=" << ssrc << ".";
}
} }
RtpStreamReceiverController::Receiver::~Receiver() { RtpStreamReceiverController::Receiver::~Receiver() {
@ -49,7 +43,7 @@ bool RtpStreamReceiverController::OnRtpPacket(const RtpPacketReceived& packet) {
return demuxer_.OnRtpPacket(packet); return demuxer_.OnRtpPacket(packet);
} }
bool RtpStreamReceiverController::AddSink(uint32_t ssrc, void RtpStreamReceiverController::AddSink(uint32_t ssrc,
RtpPacketSinkInterface* sink) { RtpPacketSinkInterface* sink) {
rtc::CritScope cs(&lock_); rtc::CritScope cs(&lock_);
return demuxer_.AddSink(ssrc, sink); return demuxer_.AddSink(ssrc, sink);

2
webrtc/call/rtp_stream_receiver_controller.h
View File

@ -38,7 +38,7 @@ class RtpStreamReceiverController
RtpPacketSinkInterface* sink) override; RtpPacketSinkInterface* sink) override;
// Thread-safe wrappers for the corresponding RtpDemuxer methods. // Thread-safe wrappers for the corresponding RtpDemuxer methods.
bool AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) override; void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) override;
size_t RemoveSink(const RtpPacketSinkInterface* sink) override; size_t RemoveSink(const RtpPacketSinkInterface* sink) override;
// TODO(nisse): Not yet responsible for parsing. // TODO(nisse): Not yet responsible for parsing.

2
webrtc/call/rtp_stream_receiver_controller_interface.h
View File

@ -38,7 +38,7 @@ class RtpStreamReceiverControllerInterface {
uint32_t ssrc, uint32_t ssrc,
RtpPacketSinkInterface* sink) = 0; RtpPacketSinkInterface* sink) = 0;
// For registering additional sinks, needed for FlexFEC. // For registering additional sinks, needed for FlexFEC.
virtual bool AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) = 0; virtual void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) = 0;
virtual size_t RemoveSink(const RtpPacketSinkInterface* sink) = 0; virtual size_t RemoveSink(const RtpPacketSinkInterface* sink) = 0;
}; };