Implemented the GetSources() in native code.

webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc

 /*
  *  Copyright (c) 2012 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/modules/rtp_rtcp/source/rtp_receiver_impl.h"
 
 #include <assert.h>
 #include <math.h>
 #include <stdlib.h>
 #include <string.h>
 
+#include <set>
+#include <vector>
+
 #include "webrtc/base/logging.h"
 #include "webrtc/common_types.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_receiver_strategy.h"
 
 namespace webrtc {
 
+// Only return the sources in the last 10 seconds.
+static const int64_t kGetSourcesTimeoutMs = 10000;

[danilchap, 2017/04/05 16:24:21]

constants outside class doesn't have to be marked static.
+nit: put them right after "using RtpUtility::Payload"

[Zhi Huang, 2017/04/06 03:09:49]

Done.

[the sun, 2017/04/06 06:55:37]

They should still be made private to the translation unit from the linker's
point of view, otherwise crazy things can happen if a constant/function/class
with the same name is defined elsewhere. Anonymous namespaces are preferred for
this.

+
+// The maximum size of the sources lists.

[the sun, 2017/04/05 14:58:30]

nit: "The maximum combined size"...

[Zhi Huang, 2017/04/06 03:09:50]

Done.

+static const int kMaxSourceListsSize = 100;
+
 using RtpUtility::Payload;
 
 RtpReceiver* RtpReceiver::CreateVideoReceiver(
     Clock* clock,
     RtpData* incoming_payload_callback,
     RtpFeedback* incoming_messages_callback,
     RTPPayloadRegistry* rtp_payload_registry) {
   if (!incoming_payload_callback)
     incoming_payload_callback = NullObjectRtpData();
   if (!incoming_messages_callback)
@@ skipping 10 matching lines @@
     RTPPayloadRegistry* rtp_payload_registry) {
   if (!incoming_payload_callback)
     incoming_payload_callback = NullObjectRtpData();
   if (!incoming_messages_callback)
     incoming_messages_callback = NullObjectRtpFeedback();
   return new RtpReceiverImpl(
       clock, incoming_messages_callback, rtp_payload_registry,
       RTPReceiverStrategy::CreateAudioStrategy(incoming_payload_callback));
 }
 
-RtpReceiverImpl::RtpReceiverImpl(
-    Clock* clock,
-    RtpFeedback* incoming_messages_callback,
-    RTPPayloadRegistry* rtp_payload_registry,
-    RTPReceiverStrategy* rtp_media_receiver)
+RtpReceiverImpl::RtpReceiverImpl(Clock* clock,
+                                 RtpFeedback* incoming_messages_callback,
+                                 RTPPayloadRegistry* rtp_payload_registry,
+                                 RTPReceiverStrategy* rtp_media_receiver)
     : clock_(clock),
       rtp_payload_registry_(rtp_payload_registry),
       rtp_media_receiver_(rtp_media_receiver),
       cb_rtp_feedback_(incoming_messages_callback),
       last_receive_time_(0),
       last_received_payload_length_(0),
       ssrc_(0),
       num_csrcs_(0),
       current_remote_csrc_(),
       last_received_timestamp_(0),
@@ skipping 82 matching lines @@
     }
     LOG(LS_WARNING) << "Receiving invalid payload type.";
     return false;
   }
 
   WebRtcRTPHeader webrtc_rtp_header;
   memset(&webrtc_rtp_header, 0, sizeof(webrtc_rtp_header));
   webrtc_rtp_header.header = rtp_header;
   CheckCSRC(webrtc_rtp_header);
 
+  UpdateSources();
+
   size_t payload_data_length = payload_length - rtp_header.paddingLength;
 
   bool is_first_packet_in_frame = false;
   {
     rtc::CritScope lock(&critical_section_rtp_receiver_);
     if (HaveReceivedFrame()) {
       is_first_packet_in_frame =
           last_received_sequence_number_ + 1 == rtp_header.sequenceNumber &&
           last_received_timestamp_ != rtp_header.timestamp;
     } else {
@@ skipping 23 matching lines @@
       last_received_sequence_number_ = rtp_header.sequenceNumber;
     }
   }
   return true;
 }
 
 TelephoneEventHandler* RtpReceiverImpl::GetTelephoneEventHandler() {
   return rtp_media_receiver_->GetTelephoneEventHandler();
 }
 
+const std::vector<RtpSource> RtpReceiverImpl::GetSources() {
+  int64_t now = clock_->TimeInMilliseconds();
+  std::vector<RtpSource> sources;
+
+  {
+    rtc::CritScope lock(&critical_section_rtp_receiver_);
+

[the sun, 2017/04/05 14:58:30]

Since we assume that the lists are sorted in the below loops, we should add a
DCHECK that they are, e.g.:

RTC_DCHECK(std::is_sorted(ssrc_sources_.begin(), ssrc_sources_.end(),
    [](const RtpSource& lhs, const RtpSource& rhs) { return lhs.timestamp() <
rhs.timestamp(); });
RTC_DCHECK(std::is_sorted(csrc_sources_.begin(), csrc_sources_.end(),
    [](const RtpSource& lhs, const RtpSource& rhs) { return lhs.timestamp() <
rhs.timestamp(); });

http://www.cplusplus.com/reference/algorithm/is_sorted/

[Zhi Huang, 2017/04/06 03:09:49]

Done.

+    std::set<uint32_t> selected_ssrcs;
+    for (auto rit = ssrc_sources_.rbegin(); rit != ssrc_sources_.rend();
+         ++rit) {
+      if (now - rit->timestamp() > kGetSourcesTimeoutMs) {

[the sun, 2017/04/05 14:58:30]

nit: () around the subtraction, here and below

[Zhi Huang, 2017/04/06 03:09:50]

Done.

+        break;
+      }
+      if (selected_ssrcs.find(rit->source_id()) == selected_ssrcs.end()) {

[danilchap, 2017/04/05 16:24:21]

to avoid double lookup you can use return value of the insert:
if (selected_ssrcs.insert(rit->source_id()).second) {
  sources.push_back(*rit);
}

[Zhi Huang, 2017/04/06 03:09:50]

Done.

+        selected_ssrcs.insert(rit->source_id());
+        sources.push_back(*rit);
+      }
+    }
+
+    for (auto rit = csrc_sources_.rbegin(); rit != csrc_sources_.rend();
+         ++rit) {
+      if (now - rit->timestamp() > kGetSourcesTimeoutMs) {
+        break;
+      }
+      sources.push_back(*rit);
+    }
+  }  // End critsect.
+
+  return sources;
+}
+
 bool RtpReceiverImpl::Timestamp(uint32_t* timestamp) const {
   rtc::CritScope lock(&critical_section_rtp_receiver_);
   if (!HaveReceivedFrame())
     return false;
   *timestamp = last_received_timestamp_;
   return true;
 }
 
 bool RtpReceiverImpl::LastReceivedTimeMs(int64_t* receive_time_ms) const {
   rtc::CritScope lock(&critical_section_rtp_receiver_);
@@ skipping 238 matching lines @@
     // Using CSRC 0 to signal this event, not interop safe, other
     // implementations might have CSRC 0 as a valid value.
     if (num_csrcs_diff > 0) {
       cb_rtp_feedback_->OnIncomingCSRCChanged(0, true);
     } else if (num_csrcs_diff < 0) {
       cb_rtp_feedback_->OnIncomingCSRCChanged(0, false);
     }
   }
 }
 
+void RtpReceiverImpl::UpdateSources() {
+  rtc::CritScope lock(&critical_section_rtp_receiver_);
+  int64_t now = clock_->TimeInMilliseconds();
+
+  for (size_t i = 0; i < num_csrcs_; ++i) {
+    auto map_it = iterator_by_csrc_.find(current_remote_csrc_[i]);
+    if (map_it == iterator_by_csrc_.end()) {
+      // If it is a new CSRC, append a new object to the end of the list.
+      csrc_sources_.emplace_back(now, current_remote_csrc_[i],
+                                 RtpSourceType::RTP_CSRC_SOURCE);
+    } else {
+      // If it is an existing CSRC, move the object to the end of the list.
+      map_it->second->update_timestamp(now);
+      csrc_sources_.splice(csrc_sources_.end(), csrc_sources_, map_it->second);
+    }
+    // Update the unordered_map.
+    iterator_by_csrc_[current_remote_csrc_[i]] = std::prev(csrc_sources_.end());
+
+    // Remove the out of date objects if the lists are too large.

[hbos, 2017/04/05 11:15:41]

nit: I think this bit should be moved to the end of UpdateSources(), done once
and after |ssrc_sources_| is updated. Otherwise we could theoretically do it
|num_csrcs_| times, and UpdateSources() iterates all of |csrc_sources_|.

[the sun, 2017/04/05 14:58:29]

Good catch!

[Zhi Huang, 2017/04/06 03:09:49]

Done.

+    if ((csrc_sources_.size() + ssrc_sources_.size()) > kMaxSourceListsSize) {
+      UpdateSourceLists(now);
+    }
+  }
+
+  // If this is the first packet or the SSRC is changed, insert a new
+  // contributing source that uses the SSRC.
+  if (ssrc_sources_.size() == 0 ||
+      std::prev(ssrc_sources_.end())->source_id() != ssrc_) {
+    ssrc_sources_.emplace_back(now, ssrc_, RtpSourceType::RTP_SSRC_SOURCE);
+  } else {
+    std::prev(ssrc_sources_.end())->update_timestamp(now);
+  }
+}
+
+// Update the lists and remove the out of date objects.

[the sun, 2017/04/05 14:58:29]

With a different function name you wouldn't need the explaining comment, e.g.:
LimitSourceListSizes()

[Zhi Huang, 2017/04/06 03:09:50]

I'll remove the length limitation and just call this RemoveOutdatedSources().

[the sun, 2017/04/06 06:55:38]

Much better than my suggestion!

+void RtpReceiverImpl::UpdateSourceLists(int64_t now) {

[hbos, 2017/04/05 11:15:41]

We only throw away outdated objects. Max object count is never enforced. I think
this makes kMaxSourceListSize misleading. I think we should always update to
remove outdates objects. Because the lists are sorted, if the first element is
not outdated we can stop looking, so it is efficient not to have a
kMaxSourceListSize.

If we want to have a max source size for security reasons then we should really
enforce the list size, and throw away even up-to-date objects that exceed the
limit.

[the sun, 2017/04/05 14:58:29]

+1

[Zhi Huang, 2017/04/06 03:09:50]

I added this because I thought the idea is to make the per packet operation as
simple as possible. But I don't like kMaxSourceListSize either and I can't
justify why it is 100. I'll remove it.

+  for (auto it = csrc_sources_.begin(); it != csrc_sources_.end();) {
+    if (now - it->timestamp() <= kGetSourcesTimeoutMs) {
+      break;
+    }
+    iterator_by_csrc_.erase(it->source_id());
+    it = csrc_sources_.erase(it);

[Taylor Brandstetter, 2017/04/05 04:27:55]

extreme nit: I think this could just do "++it" here, and then outside the loop,
remove everything up to "it" with:

csrc_sources_.erase(csrc_soruces_.begin(), it);

Which could have a slight performance benefit?

[the sun, 2017/04/05 14:58:29]

Possibly, but likely not since it's a std::list. Would be consistent with the
below loop though so I think it would still make sense.

[Zhi Huang, 2017/04/06 03:09:50]

Done.

+  }
+
+  for (auto it = ssrc_sources_.begin(); it != ssrc_sources_.end(); ++it) {
+    if (now - it->timestamp() <= kGetSourcesTimeoutMs) {
+      ssrc_sources_.erase(ssrc_sources_.begin(), it);
+      break;
+    }

[hbos, 2017/04/05 11:15:41]

Because of the break this will at most remove one ssrc. If multiple packets
arrive the same time, ssrc_sources_ could contain outdated objects. It should
loop for as long as !empty && first element is outdated.

Since we're doing removal of the first element, how about using list instead of
vector?

[the sun, 2017/04/05 14:58:30]

The idea is to remove all old elements in one go. Missing test?

[Zhi Huang, 2017/04/06 03:09:49]

I don't think it will only remove one ssrc.

The iterator keeps moving until it finds the first one that is not out of date
(goes into the if condition), remove all the objects before it and then break.
There is at least one source that is not outdated, because this method is only
called when a new packet is received.

It seems that the difference is "erase then break" and "break then erase".

[Zhi Huang, 2017/04/06 03:09:50]

I've added a test for this. 
I agree with Taylor (his comments in the unit tests) that there is no clear way
to do black box test. So I exposed the crsc_sources_ and the ssrc_sources_ in
the RtpReceiverImpl and downcasted the RtpReceiver in the test. Is this
acceptable?

[the sun, 2017/04/06 06:55:37]

True, both works.

[the sun, 2017/04/06 06:55:38]

Yes, it's a good idea though to mark such functions in some way so their purpose
is clear to the casual reader.

+  }

[the sun, 2017/04/05 14:58:29]

The erase should go here, after searching for the first item to remain in the
list.

[Zhi Huang, 2017/04/06 03:09:50]

Alright. This looks more clear.

+}
+
 }  // namespace webrtc