Replace lock with checker in TransportFeedbackDemuxer
Bug: webrtc:13517, webrtc:11993
Change-Id: If44c7a7428454c0bf5a6fee6d12d816e3dc8e27c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/248163
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35772}
diff --git a/modules/congestion_controller/rtp/BUILD.gn b/modules/congestion_controller/rtp/BUILD.gn
index 1a70447..39d4d68 100644
--- a/modules/congestion_controller/rtp/BUILD.gn
+++ b/modules/congestion_controller/rtp/BUILD.gn
@@ -61,6 +61,7 @@
"../../../rtc_base:rtc_base_approved",
"../../../rtc_base/network:sent_packet",
"../../../rtc_base/synchronization:mutex",
+ "../../../rtc_base/system:no_unique_address",
"../../../system_wrappers",
"../../../system_wrappers:field_trial",
"../../rtp_rtcp:rtp_rtcp_format",
diff --git a/modules/congestion_controller/rtp/transport_feedback_demuxer.cc b/modules/congestion_controller/rtp/transport_feedback_demuxer.cc
index 6ab3ad8..62b85b1 100644
--- a/modules/congestion_controller/rtp/transport_feedback_demuxer.cc
+++ b/modules/congestion_controller/rtp/transport_feedback_demuxer.cc
@@ -15,10 +15,17 @@
namespace {
static const size_t kMaxPacketsInHistory = 5000;
}
+
+TransportFeedbackDemuxer::TransportFeedbackDemuxer() {
+ // In case the construction thread is different from where the registration
+ // and callbacks occur, detach from the construction thread.
+ observer_checker_.Detach();
+}
+
void TransportFeedbackDemuxer::RegisterStreamFeedbackObserver(
std::vector<uint32_t> ssrcs,
StreamFeedbackObserver* observer) {
- MutexLock lock(&observers_lock_);
+ RTC_DCHECK_RUN_ON(&observer_checker_);
RTC_DCHECK(observer);
RTC_DCHECK(absl::c_find_if(observers_, [=](const auto& pair) {
return pair.second == observer;
@@ -28,7 +35,7 @@
void TransportFeedbackDemuxer::DeRegisterStreamFeedbackObserver(
StreamFeedbackObserver* observer) {
- MutexLock lock(&observers_lock_);
+ RTC_DCHECK_RUN_ON(&observer_checker_);
RTC_DCHECK(observer);
const auto it = absl::c_find_if(
observers_, [=](const auto& pair) { return pair.second == observer; });
@@ -65,14 +72,14 @@
if (it != history_.end()) {
auto packet_info = it->second;
packet_info.received = packet.received();
- stream_feedbacks.push_back(packet_info);
+ stream_feedbacks.push_back(std::move(packet_info));
if (packet.received())
history_.erase(it);
}
}
}
- MutexLock lock(&observers_lock_);
+ RTC_DCHECK_RUN_ON(&observer_checker_);
for (auto& observer : observers_) {
std::vector<StreamFeedbackObserver::StreamPacketInfo> selected_feedback;
for (const auto& packet_info : stream_feedbacks) {
diff --git a/modules/congestion_controller/rtp/transport_feedback_demuxer.h b/modules/congestion_controller/rtp/transport_feedback_demuxer.h
index 634a37ea..895288f 100644
--- a/modules/congestion_controller/rtp/transport_feedback_demuxer.h
+++ b/modules/congestion_controller/rtp/transport_feedback_demuxer.h
@@ -14,14 +14,27 @@
#include <utility>
#include <vector>
+#include "api/sequence_checker.h"
#include "modules/include/module_common_types_public.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "rtc_base/synchronization/mutex.h"
+#include "rtc_base/system/no_unique_address.h"
namespace webrtc {
-class TransportFeedbackDemuxer : public StreamFeedbackProvider {
+// Implementation of StreamFeedbackProvider that provides a way for
+// implementations of StreamFeedbackObserver to register for feedback callbacks
+// for a given set of SSRCs.
+// Registration methods need to be called from the same execution context
+// (thread or task queue) and callbacks to
+// StreamFeedbackObserver::OnPacketFeedbackVector will be made in that same
+// context.
+// TODO(tommi): This appears to be the only implementation of this interface.
+// Do we need the interface?
+class TransportFeedbackDemuxer final : public StreamFeedbackProvider {
public:
+ TransportFeedbackDemuxer();
+
// Implements StreamFeedbackProvider interface
void RegisterStreamFeedbackObserver(
std::vector<uint32_t> ssrcs,
@@ -40,9 +53,9 @@
// Maps a set of ssrcs to corresponding observer. Vectors are used rather than
// set/map to ensure that the processing order is consistent independently of
// the randomized ssrcs.
- Mutex observers_lock_;
+ RTC_NO_UNIQUE_ADDRESS SequenceChecker observer_checker_;
std::vector<std::pair<std::vector<uint32_t>, StreamFeedbackObserver*>>
- observers_ RTC_GUARDED_BY(&observers_lock_);
+ observers_ RTC_GUARDED_BY(&observer_checker_);
};
} // namespace webrtc
