Enhance thread checks in BaseChannel classes.
Improve consistency between using DCHECK checkers and compile time.
For virtual methods, we were sometimes using both and in other cases
we could be using compile time checks but were using runtime.
Added annotation for last_send_params_, last_recv_params_ in
audio/video channel classes.
Also removing redundant logging for when registration with the
transport fails. This is already being logged in the demuxer.
Bug: webrtc:12230
Change-Id: I48e156c9996dec26a990151301dabc06673541d0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244095
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35630}
diff --git a/pc/channel.cc b/pc/channel.cc
index eb7219c..925d459 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -157,14 +157,13 @@
return sb.Release();
}
-bool BaseChannel::ConnectToRtpTransport() {
+bool BaseChannel::ConnectToRtpTransport_n() {
RTC_DCHECK(rtp_transport_);
RTC_DCHECK(media_channel());
// We don't need to call OnDemuxerCriteriaUpdatePending/Complete because
// there's no previous criteria to worry about.
if (!rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria_, this)) {
- RTC_LOG(LS_ERROR) << "Failed to set up demuxing for " << ToString();
return false;
}
rtp_transport_->SignalReadyToSend.connect(
@@ -178,7 +177,7 @@
return true;
}
-void BaseChannel::DisconnectFromRtpTransport() {
+void BaseChannel::DisconnectFromRtpTransport_n() {
RTC_DCHECK(rtp_transport_);
RTC_DCHECK(media_channel());
rtp_transport_->UnregisterRtpDemuxerSink(this);
@@ -209,7 +208,7 @@
media_channel_->SetInterface(/*iface=*/nullptr);
if (rtp_transport_) {
- DisconnectFromRtpTransport();
+ DisconnectFromRtpTransport_n();
}
});
}
@@ -222,14 +221,12 @@
}
if (rtp_transport_) {
- DisconnectFromRtpTransport();
+ DisconnectFromRtpTransport_n();
}
rtp_transport_ = rtp_transport;
if (rtp_transport_) {
- if (!ConnectToRtpTransport()) {
- RTC_LOG(LS_ERROR) << "Failed to connect to the new RtpTransport for "
- << ToString() << ".";
+ if (!ConnectToRtpTransport_n()) {
return false;
}
OnTransportReadyToSend(rtp_transport_->IsReadyToSend());
@@ -297,12 +294,6 @@
return SetPayloadTypeDemuxingEnabled_w(enabled);
}
-bool BaseChannel::IsReadyToReceiveMedia_w() const {
- // Receive data if we are enabled and have local content,
- return enabled_ &&
- webrtc::RtpTransceiverDirectionHasRecv(local_content_direction_);
-}
-
bool BaseChannel::IsReadyToSendMedia_w() const {
// Send outgoing data if we are enabled, have local and remote content,
// and we have had some form of connectivity.
@@ -795,24 +786,23 @@
void VoiceChannel::UpdateMediaSendRecvState_w() {
// Render incoming data if we're the active call, and we have the local
// content. We receive data on the default channel and multiplexed streams.
- RTC_DCHECK_RUN_ON(worker_thread());
- bool recv = IsReadyToReceiveMedia_w();
- media_channel()->SetPlayout(recv);
+ bool ready_to_receive = enabled() && webrtc::RtpTransceiverDirectionHasRecv(
+ local_content_direction());
+ media_channel()->SetPlayout(ready_to_receive);
// Send outgoing data if we're the active call, we have the remote content,
// and we have had some form of connectivity.
bool send = IsReadyToSendMedia_w();
media_channel()->SetSend(send);
- RTC_LOG(LS_INFO) << "Changing voice state, recv=" << recv << " send=" << send
- << " for " << ToString();
+ RTC_LOG(LS_INFO) << "Changing voice state, recv=" << ready_to_receive
+ << " send=" << send << " for " << ToString();
}
bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
SdpType type,
std::string* error_desc) {
TRACE_EVENT0("webrtc", "VoiceChannel::SetLocalContent_w");
- RTC_DCHECK_RUN_ON(worker_thread());
RTC_LOG(LS_INFO) << "Setting local voice description for " << ToString();
RtpHeaderExtensions rtp_header_extensions =
@@ -872,7 +862,6 @@
SdpType type,
std::string* error_desc) {
TRACE_EVENT0("webrtc", "VoiceChannel::SetRemoteContent_w");
- RTC_DCHECK_RUN_ON(worker_thread());
RTC_LOG(LS_INFO) << "Setting remote voice description for " << ToString();
const AudioContentDescription* audio = content->as_audio();
@@ -953,7 +942,6 @@
void VideoChannel::UpdateMediaSendRecvState_w() {
// Send outgoing data if we're the active call, we have the remote content,
// and we have had some form of connectivity.
- RTC_DCHECK_RUN_ON(worker_thread());
bool send = IsReadyToSendMedia_w();
if (!media_channel()->SetSend(send)) {
RTC_LOG(LS_ERROR) << "Failed to SetSend on video channel: " + ToString();
@@ -974,7 +962,6 @@
SdpType type,
std::string* error_desc) {
TRACE_EVENT0("webrtc", "VideoChannel::SetLocalContent_w");
- RTC_DCHECK_RUN_ON(worker_thread());
RTC_LOG(LS_INFO) << "Setting local video description for " << ToString();
RtpHeaderExtensions rtp_header_extensions =
@@ -1065,7 +1052,6 @@
SdpType type,
std::string* error_desc) {
TRACE_EVENT0("webrtc", "VideoChannel::SetRemoteContent_w");
- RTC_DCHECK_RUN_ON(worker_thread());
RTC_LOG(LS_INFO) << "Setting remote video description for " << ToString();
const VideoContentDescription* video = content->as_video();
diff --git a/pc/channel.h b/pc/channel.h
index 9d14db4..76f6b54 100644
--- a/pc/channel.h
+++ b/pc/channel.h
@@ -193,27 +193,39 @@
}
protected:
- void set_local_content_direction(webrtc::RtpTransceiverDirection direction) {
- RTC_DCHECK_RUN_ON(worker_thread());
+ void set_local_content_direction(webrtc::RtpTransceiverDirection direction)
+ RTC_RUN_ON(worker_thread()) {
local_content_direction_ = direction;
}
- void set_remote_content_direction(webrtc::RtpTransceiverDirection direction) {
- RTC_DCHECK_RUN_ON(worker_thread());
+
+ webrtc::RtpTransceiverDirection local_content_direction() const
+ RTC_RUN_ON(worker_thread()) {
+ return local_content_direction_;
+ }
+
+ void set_remote_content_direction(webrtc::RtpTransceiverDirection direction)
+ RTC_RUN_ON(worker_thread()) {
remote_content_direction_ = direction;
}
- // These methods verify that:
+
+ webrtc::RtpTransceiverDirection remote_content_direction() const
+ RTC_RUN_ON(worker_thread()) {
+ return remote_content_direction_;
+ }
+
+ bool enabled() const RTC_RUN_ON(worker_thread()) { return enabled_; }
+ rtc::Thread* signaling_thread() const { return signaling_thread_; }
+
+ // Call to verify that:
// * The required content description directions have been set.
// * The channel is enabled.
- // * And for sending:
- // - The SRTP filter is active if it's needed.
- // - The transport has been writable before, meaning it should be at least
- // possible to succeed in sending a packet.
+ // * The SRTP filter is active if it's needed.
+ // * The transport has been writable before, meaning it should be at least
+ // possible to succeed in sending a packet.
//
// When any of these properties change, UpdateMediaSendRecvState_w should be
// called.
- bool IsReadyToReceiveMedia_w() const RTC_RUN_ON(worker_thread());
bool IsReadyToSendMedia_w() const RTC_RUN_ON(worker_thread());
- rtc::Thread* signaling_thread() const { return signaling_thread_; }
// NetworkInterface implementation, called by MediaEngine
bool SendPacket(rtc::CopyOnWriteBuffer* packet,
@@ -291,8 +303,8 @@
std::string ToString() const;
private:
- bool ConnectToRtpTransport() RTC_RUN_ON(network_thread());
- void DisconnectFromRtpTransport() RTC_RUN_ON(network_thread());
+ bool ConnectToRtpTransport_n() RTC_RUN_ON(network_thread());
+ void DisconnectFromRtpTransport_n() RTC_RUN_ON(network_thread());
void SignalSentPacket_n(const rtc::SentPacket& sent_packet);
rtc::Thread* const worker_thread_;
@@ -387,20 +399,22 @@
private:
// overrides from BaseChannel
- void UpdateMediaSendRecvState_w() override;
+ void UpdateMediaSendRecvState_w() RTC_RUN_ON(worker_thread()) override;
bool SetLocalContent_w(const MediaContentDescription* content,
webrtc::SdpType type,
- std::string* error_desc) override;
+ std::string* error_desc)
+ RTC_RUN_ON(worker_thread()) override;
bool SetRemoteContent_w(const MediaContentDescription* content,
webrtc::SdpType type,
- std::string* error_desc) override;
+ std::string* error_desc)
+ RTC_RUN_ON(worker_thread()) override;
// Last AudioSendParameters sent down to the media_channel() via
// SetSendParameters.
- AudioSendParameters last_send_params_;
+ AudioSendParameters last_send_params_ RTC_GUARDED_BY(worker_thread());
// Last AudioRecvParameters sent down to the media_channel() via
// SetRecvParameters.
- AudioRecvParameters last_recv_params_;
+ AudioRecvParameters last_recv_params_ RTC_GUARDED_BY(worker_thread());
};
// VideoChannel is a specialization for video.
@@ -421,28 +435,30 @@
return static_cast<VideoMediaChannel*>(BaseChannel::media_channel());
}
- void FillBitrateInfo(BandwidthEstimationInfo* bwe_info);
-
cricket::MediaType media_type() const override {
return cricket::MEDIA_TYPE_VIDEO;
}
+ void FillBitrateInfo(BandwidthEstimationInfo* bwe_info);
+
private:
// overrides from BaseChannel
- void UpdateMediaSendRecvState_w() override;
+ void UpdateMediaSendRecvState_w() RTC_RUN_ON(worker_thread()) override;
bool SetLocalContent_w(const MediaContentDescription* content,
webrtc::SdpType type,
- std::string* error_desc) override;
+ std::string* error_desc)
+ RTC_RUN_ON(worker_thread()) override;
bool SetRemoteContent_w(const MediaContentDescription* content,
webrtc::SdpType type,
- std::string* error_desc) override;
+ std::string* error_desc)
+ RTC_RUN_ON(worker_thread()) override;
// Last VideoSendParameters sent down to the media_channel() via
// SetSendParameters.
- VideoSendParameters last_send_params_;
+ VideoSendParameters last_send_params_ RTC_GUARDED_BY(worker_thread());
// Last VideoRecvParameters sent down to the media_channel() via
// SetRecvParameters.
- VideoRecvParameters last_recv_params_;
+ VideoRecvParameters last_recv_params_ RTC_GUARDED_BY(worker_thread());
};
} // namespace cricket
