SSRC and RSID may only refer to one sink each in RtpDemuxer

webrtc/call/rtp_demuxer.cc

 /*
  *  Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/call/rtp_demuxer.h"
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/call/rsid_resolution_observer.h"
 #include "webrtc/call/rtp_packet_sink_interface.h"
 #include "webrtc/call/rtp_rtcp_demuxer_helper.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_header_extensions.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h"
 
 namespace webrtc {
 
 RtpDemuxer::RtpDemuxer() = default;
 
 RtpDemuxer::~RtpDemuxer() {
   RTC_DCHECK(ssrc_sinks_.empty());
   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).
+  if (ssrc_sinks_.find(ssrc) == ssrc_sinks_.cend()) {
+    ssrc_sinks_.emplace(ssrc, sink);
+    return true;
+  } else {
+    return false;
+  }
 }
 
 void RtpDemuxer::AddSink(const std::string& rsid,
                          RtpPacketSinkInterface* sink) {
   RTC_DCHECK(StreamId::IsLegalName(rsid));
   RTC_DCHECK(sink);
-  RTC_DCHECK(!MultimapAssociationExists(rsid_sinks_, rsid, sink));
+  // TODO(eladalon): Discuss with nisse and danilchap whether we want to

[danilchap, 2017/06/30 16:45:31]

I do not know a use case where adding sink by rsid expect to fail.
i.e. keep DCHECK.
returning bool will push DCHECK to user of the RtpDemuxer. that is an option
too, but for now better change less.

[eladalon, 2017/07/03 08:23:35]

Acknowledged.

+  // keep this a DCHECK, or protect against it softly, like with SSRC.
+  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());
@@ skipping 16 matching lines @@
   RTC_DCHECK(observer);
   auto it = std::find(rsid_resolution_observers_.begin(),
                       rsid_resolution_observers_.end(), observer);
   RTC_DCHECK(it != rsid_resolution_observers_.end());
   rsid_resolution_observers_.erase(it);
 }
 
 void RtpDemuxer::ResolveRsidToSsrcAssociations(
     const RtpPacketReceived& packet) {
   std::string rsid;
-  if (packet.GetExtension<RtpStreamId>(&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);
-    }
+  if (!packet.GetExtension<RtpStreamId>(&rsid)) {
+    return;
+  }
 
-    NotifyObserversOfRsidResolution(rsid, packet.Ssrc());
+  auto it = rsid_sinks_.find(rsid);
+  if (it == rsid_sinks_.end()) {
+    return;
+  }
 
-    // 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 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.
+    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,
                                                  uint32_t ssrc) {
   for (auto* observer : rsid_resolution_observers_) {
     observer->OnRsidResolved(rsid, ssrc);
   }
 }
 
 }  // namespace webrtc