Remove lock from RtpStreamReceiverController.
The demuxer variable is now being used from the same thread consistently
so it's safe to replace the lock with a sequence checker.
Down the line, we may move construction+use of the
RtpStreamReceiverController class in Call, over to the network thread.
This should be possible without further modifications to
RtpStreamReceiverController.
Bug: webrtc:11993, webrtc:11567
Change-Id: Iee8c31ddf9b26b39393f40b5b1d25343b0233ae3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/202245
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33016}
diff --git a/call/rtp_demuxer.cc b/call/rtp_demuxer.cc
index 9fc4ba1..c09aefd 100644
--- a/call/rtp_demuxer.cc
+++ b/call/rtp_demuxer.cc
@@ -91,7 +91,7 @@
return sb.Release();
}
-RtpDemuxer::RtpDemuxer() = default;
+RtpDemuxer::RtpDemuxer(bool use_mid /* = true*/) : use_mid_(use_mid) {}
RtpDemuxer::~RtpDemuxer() {
RTC_DCHECK(sink_by_mid_.empty());
diff --git a/call/rtp_demuxer.h b/call/rtp_demuxer.h
index 3aa7e9d..b89f154 100644
--- a/call/rtp_demuxer.h
+++ b/call/rtp_demuxer.h
@@ -94,7 +94,7 @@
// relevant for demuxing.
static std::string DescribePacket(const RtpPacketReceived& packet);
- RtpDemuxer();
+ explicit RtpDemuxer(bool use_mid = true);
~RtpDemuxer();
RtpDemuxer(const RtpDemuxer&) = delete;
@@ -132,10 +132,6 @@
// if the packet was forwarded and false if the packet was dropped.
bool OnRtpPacket(const RtpPacketReceived& packet);
- // Configure whether to look at the MID header extension when demuxing
- // incoming RTP packets. By default this is enabled.
- void set_use_mid(bool use_mid) { use_mid_ = use_mid; }
-
private:
// Returns true if adding a sink with the given criteria would cause conflicts
// with the existing criteria and should be rejected.
@@ -191,7 +187,7 @@
// Adds a binding from the SSRC to the given sink.
void AddSsrcSinkBinding(uint32_t ssrc, RtpPacketSinkInterface* sink);
- bool use_mid_ = true;
+ const bool use_mid_;
};
} // namespace webrtc
diff --git a/call/rtp_stream_receiver_controller.cc b/call/rtp_stream_receiver_controller.cc
index f440b42..7150b34 100644
--- a/call/rtp_stream_receiver_controller.cc
+++ b/call/rtp_stream_receiver_controller.cc
@@ -37,11 +37,7 @@
controller_->RemoveSink(sink_);
}
-RtpStreamReceiverController::RtpStreamReceiverController() {
- // At this level the demuxer is only configured to demux by SSRC, so don't
- // worry about MIDs (MIDs are handled by upper layers).
- demuxer_.set_use_mid(false);
-}
+RtpStreamReceiverController::RtpStreamReceiverController() {}
RtpStreamReceiverController::~RtpStreamReceiverController() = default;
@@ -52,19 +48,19 @@
}
bool RtpStreamReceiverController::OnRtpPacket(const RtpPacketReceived& packet) {
- rtc::CritScope cs(&lock_);
+ RTC_DCHECK_RUN_ON(&demuxer_sequence_);
return demuxer_.OnRtpPacket(packet);
}
bool RtpStreamReceiverController::AddSink(uint32_t ssrc,
RtpPacketSinkInterface* sink) {
- rtc::CritScope cs(&lock_);
+ RTC_DCHECK_RUN_ON(&demuxer_sequence_);
return demuxer_.AddSink(ssrc, sink);
}
size_t RtpStreamReceiverController::RemoveSink(
const RtpPacketSinkInterface* sink) {
- rtc::CritScope cs(&lock_);
+ RTC_DCHECK_RUN_ON(&demuxer_sequence_);
return demuxer_.RemoveSink(sink);
}
diff --git a/call/rtp_stream_receiver_controller.h b/call/rtp_stream_receiver_controller.h
index 62447aa..2611561 100644
--- a/call/rtp_stream_receiver_controller.h
+++ b/call/rtp_stream_receiver_controller.h
@@ -14,7 +14,7 @@
#include "call/rtp_demuxer.h"
#include "call/rtp_stream_receiver_controller_interface.h"
-#include "rtc_base/deprecated/recursive_critical_section.h"
+#include "rtc_base/synchronization/sequence_checker.h"
namespace webrtc {
@@ -58,13 +58,18 @@
RtpPacketSinkInterface* const sink_;
};
- // TODO(nisse): Move to a TaskQueue for synchronization. When used
- // by Call, we expect construction and all methods but OnRtpPacket
- // to be called on the same thread, and OnRtpPacket to be called
- // by a single, but possibly distinct, thread. But applications not
- // using Call may have use threads differently.
- rtc::RecursiveCriticalSection lock_;
- RtpDemuxer demuxer_ RTC_GUARDED_BY(&lock_);
+ // TODO(bugs.webrtc.org/11993): We expect construction and all methods to be
+ // called on the same thread/tq. Currently this is the worker thread
+ // (including OnRtpPacket) but a more natural fit would be the network thread.
+ // Using a sequence checker to ensure that usage is correct but at the same
+ // time not require a specific thread/tq, an instance of this class + the
+ // associated functionality should be easily moved from one execution context
+ // to another (i.e. when network packets don't hop to the worker thread inside
+ // of Call).
+ SequenceChecker demuxer_sequence_;
+ // At this level the demuxer is only configured to demux by SSRC, so don't
+ // worry about MIDs (MIDs are handled by upper layers).
+ RtpDemuxer demuxer_ RTC_GUARDED_BY(&demuxer_sequence_){false /*use_mid*/};
};
} // namespace webrtc
