Make RtpDemuxerCriteria's mid_ and rsid_ const.

Remove unnecessary optimization from BaseChannel,
previous_demuxer_criteria_, that I'm not seeing as providing value.

Previously it was used to avoid a thread hop if a reconfiguration
wasn't needed, but the way that was done, wasn't thread safe. So after
addressing that issue, the variable more represents increased complexity
in the code than runtime efficiency.

Bug: webrtc:11993, webrtc:12230
Change-Id: Ic8e3e1d2f57e669a168cc7b5cf5d407133976e3c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244093
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35628}
This commit is contained in:
Tomas Gunnarsson
2022-01-04 17:21:54 +00:00
committed by WebRTC LUCI CQ
parent 61f70780a3
commit 9abd74dc1e
3 changed files with 7 additions and 26 deletions

View File

@ -65,11 +65,8 @@ class RtpDemuxerCriteria {
private:
// Intentionally private member variables to encourage specifying them via the
// constructor and consider them to be const as much as possible.
// Post construction, reading the values needs to be done via accessors and if
// changing the value is required, that can still be done via the implicit
// assignment operator (which also reassigns all other member variables).
std::string mid_;
std::string rsid_;
const std::string mid_;
const std::string rsid_;
flat_set<uint32_t> ssrcs_;
flat_set<uint8_t> payload_types_;
};

View File

@ -163,11 +163,7 @@ bool BaseChannel::ConnectToRtpTransport() {
// We don't need to call OnDemuxerCriteriaUpdatePending/Complete because
// there's no previous criteria to worry about.
bool result = rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria_, this);
if (result) {
previous_demuxer_criteria_ = demuxer_criteria_;
} else {
previous_demuxer_criteria_ = {};
if (!rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria_, this)) {
RTC_LOG(LS_ERROR) << "Failed to set up demuxing for " << ToString();
return false;
}
@ -493,18 +489,10 @@ bool BaseChannel::RegisterRtpDemuxerSink_w() {
bool ret = network_thread_->Invoke<bool>(
RTC_FROM_HERE, [this, demuxer_criteria = demuxer_criteria_] {
RTC_DCHECK_RUN_ON(network_thread());
RTC_DCHECK(rtp_transport_);
if (demuxer_criteria_ == previous_demuxer_criteria_)
return true;
bool result =
rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria, this);
if (result) {
previous_demuxer_criteria_ = demuxer_criteria;
} else {
previous_demuxer_criteria_ = {};
}
return result;
// Note that RegisterRtpDemuxerSink first unregisters the sink if
// already registered. So this will change the state of the class
// whether the call succeeds or not.
return rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria, this);
});
media_channel_->OnDemuxerCriteriaUpdateComplete();

View File

@ -355,10 +355,6 @@ class BaseChannel : public ChannelInterface,
// TODO(bugs.webrtc.org/12239): Modified on worker thread, accessed
// on network thread in RegisterRtpDemuxerSink_n (called from Init_w)
webrtc::RtpDemuxerCriteria demuxer_criteria_;
// Accessed on the worker thread, modified on the network thread from
// RegisterRtpDemuxerSink_w's Invoke.
webrtc::RtpDemuxerCriteria previous_demuxer_criteria_
RTC_GUARDED_BY(network_thread());
// This generator is used to generate SSRCs for local streams.
// This is needed in cases where SSRCs are not negotiated or set explicitly
// like in Simulcast.