admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enhance thread checks in BaseChannel classes.

Browse Source 
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}
This commit is contained in:
Tomas Gunnarsson 2022-01-05 10:26:36 +00:00 committed by WebRTC LUCI CQ
parent a89432f94b
commit b496c32901
2 changed files with 51 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
pc/channel.cc
View File

@ -157,14 +157,13 @@ std::string BaseChannel::ToString() const {
return sb.Release(); return sb.Release();
} }
bool BaseChannel::ConnectToRtpTransport() { bool BaseChannel::ConnectToRtpTransport_n() {
RTC_DCHECK(rtp_transport_); RTC_DCHECK(rtp_transport_);
RTC_DCHECK(media_channel()); RTC_DCHECK(media_channel());
// We don't need to call OnDemuxerCriteriaUpdatePending/Complete because // We don't need to call OnDemuxerCriteriaUpdatePending/Complete because
// there's no previous criteria to worry about. // there's no previous criteria to worry about.
if (!rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria_, this)) { if (!rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria_, this)) {
RTC_LOG(LS_ERROR) << "Failed to set up demuxing for " << ToString();
return false; return false;
} }
rtp_transport_->SignalReadyToSend.connect( rtp_transport_->SignalReadyToSend.connect(
@ -178,7 +177,7 @@ bool BaseChannel::ConnectToRtpTransport() {
return true; return true;
} }
void BaseChannel::DisconnectFromRtpTransport() { void BaseChannel::DisconnectFromRtpTransport_n() {
RTC_DCHECK(rtp_transport_); RTC_DCHECK(rtp_transport_);
RTC_DCHECK(media_channel()); RTC_DCHECK(media_channel());
rtp_transport_->UnregisterRtpDemuxerSink(this); rtp_transport_->UnregisterRtpDemuxerSink(this);
@ -209,7 +208,7 @@ void BaseChannel::Deinit() {
media_channel_->SetInterface(/*iface=*/nullptr); media_channel_->SetInterface(/*iface=*/nullptr);
if (rtp_transport_) { if (rtp_transport_) {
DisconnectFromRtpTransport(); DisconnectFromRtpTransport_n();
} }
}); });
} }
@ -222,14 +221,12 @@ bool BaseChannel::SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) {
} }
if (rtp_transport_) { if (rtp_transport_) {
DisconnectFromRtpTransport(); DisconnectFromRtpTransport_n();
} }
rtp_transport_ = rtp_transport; rtp_transport_ = rtp_transport;
if (rtp_transport_) { if (rtp_transport_) {
if (!ConnectToRtpTransport()) { if (!ConnectToRtpTransport_n()) {
RTC_LOG(LS_ERROR) << "Failed to connect to the new RtpTransport for "
<< ToString() << ".";
return false; return false;
} }
OnTransportReadyToSend(rtp_transport_->IsReadyToSend()); OnTransportReadyToSend(rtp_transport_->IsReadyToSend());
@ -297,12 +294,6 @@ bool BaseChannel::SetPayloadTypeDemuxingEnabled(bool enabled) {
return SetPayloadTypeDemuxingEnabled_w(enabled); 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 { bool BaseChannel::IsReadyToSendMedia_w() const {
// Send outgoing data if we are enabled, have local and remote content, // Send outgoing data if we are enabled, have local and remote content,
// and we have had some form of connectivity. // and we have had some form of connectivity.
@ -795,24 +786,23 @@ VoiceChannel::~VoiceChannel() {
void VoiceChannel::UpdateMediaSendRecvState_w() { void VoiceChannel::UpdateMediaSendRecvState_w() {
// Render incoming data if we're the active call, and we have the local // 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. // content. We receive data on the default channel and multiplexed streams.
RTC_DCHECK_RUN_ON(worker_thread()); bool ready_to_receive = enabled() && webrtc::RtpTransceiverDirectionHasRecv(
bool recv = IsReadyToReceiveMedia_w(); local_content_direction());
media_channel()->SetPlayout(recv); media_channel()->SetPlayout(ready_to_receive);
// Send outgoing data if we're the active call, we have the remote content, // Send outgoing data if we're the active call, we have the remote content,
// and we have had some form of connectivity. // and we have had some form of connectivity.
bool send = IsReadyToSendMedia_w(); bool send = IsReadyToSendMedia_w();
media_channel()->SetSend(send); media_channel()->SetSend(send);
RTC_LOG(LS_INFO) << "Changing voice state, recv=" << recv << " send=" << send RTC_LOG(LS_INFO) << "Changing voice state, recv=" << ready_to_receive
<< " for " << ToString(); << " send=" << send << " for " << ToString();
} }
bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
SdpType type, SdpType type,
std::string* error_desc) { std::string* error_desc) {
TRACE_EVENT0("webrtc", "VoiceChannel::SetLocalContent_w"); TRACE_EVENT0("webrtc", "VoiceChannel::SetLocalContent_w");
RTC_DCHECK_RUN_ON(worker_thread());
RTC_LOG(LS_INFO) << "Setting local voice description for " << ToString(); RTC_LOG(LS_INFO) << "Setting local voice description for " << ToString();
RtpHeaderExtensions rtp_header_extensions = RtpHeaderExtensions rtp_header_extensions =
@ -872,7 +862,6 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content,
SdpType type, SdpType type,
std::string* error_desc) { std::string* error_desc) {
TRACE_EVENT0("webrtc", "VoiceChannel::SetRemoteContent_w"); TRACE_EVENT0("webrtc", "VoiceChannel::SetRemoteContent_w");
RTC_DCHECK_RUN_ON(worker_thread());
RTC_LOG(LS_INFO) << "Setting remote voice description for " << ToString(); RTC_LOG(LS_INFO) << "Setting remote voice description for " << ToString();
const AudioContentDescription* audio = content->as_audio(); const AudioContentDescription* audio = content->as_audio();
@ -953,7 +942,6 @@ VideoChannel::~VideoChannel() {
void VideoChannel::UpdateMediaSendRecvState_w() { void VideoChannel::UpdateMediaSendRecvState_w() {
// Send outgoing data if we're the active call, we have the remote content, // Send outgoing data if we're the active call, we have the remote content,
// and we have had some form of connectivity. // and we have had some form of connectivity.
RTC_DCHECK_RUN_ON(worker_thread());
bool send = IsReadyToSendMedia_w(); bool send = IsReadyToSendMedia_w();
if (!media_channel()->SetSend(send)) { if (!media_channel()->SetSend(send)) {
RTC_LOG(LS_ERROR) << "Failed to SetSend on video channel: " + ToString(); RTC_LOG(LS_ERROR) << "Failed to SetSend on video channel: " + ToString();
@ -974,7 +962,6 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
SdpType type, SdpType type,
std::string* error_desc) { std::string* error_desc) {
TRACE_EVENT0("webrtc", "VideoChannel::SetLocalContent_w"); TRACE_EVENT0("webrtc", "VideoChannel::SetLocalContent_w");
RTC_DCHECK_RUN_ON(worker_thread());
RTC_LOG(LS_INFO) << "Setting local video description for " << ToString(); RTC_LOG(LS_INFO) << "Setting local video description for " << ToString();
RtpHeaderExtensions rtp_header_extensions = RtpHeaderExtensions rtp_header_extensions =
@ -1065,7 +1052,6 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
SdpType type, SdpType type,
std::string* error_desc) { std::string* error_desc) {
TRACE_EVENT0("webrtc", "VideoChannel::SetRemoteContent_w"); TRACE_EVENT0("webrtc", "VideoChannel::SetRemoteContent_w");
RTC_DCHECK_RUN_ON(worker_thread());
RTC_LOG(LS_INFO) << "Setting remote video description for " << ToString(); RTC_LOG(LS_INFO) << "Setting remote video description for " << ToString();
const VideoContentDescription* video = content->as_video(); const VideoContentDescription* video = content->as_video();

64
pc/channel.h
View File

@ -193,27 +193,39 @@ class BaseChannel : public ChannelInterface,
} }
protected: protected:
void set_local_content_direction(webrtc::RtpTransceiverDirection direction) { void set_local_content_direction(webrtc::RtpTransceiverDirection direction)
RTC_DCHECK_RUN_ON(worker_thread()); RTC_RUN_ON(worker_thread()) {
local_content_direction_ = direction; 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; 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 required content description directions have been set.
// * The channel is enabled. // * The channel is enabled.
// * And for sending: // * The SRTP filter is active if it's needed.
// - The SRTP filter is active if it's needed. // * The transport has been writable before, meaning it should be at least
// - The transport has been writable before, meaning it should be at least
// possible to succeed in sending a packet. // possible to succeed in sending a packet.
// //
// When any of these properties change, UpdateMediaSendRecvState_w should be // When any of these properties change, UpdateMediaSendRecvState_w should be
// called. // called.
bool IsReadyToReceiveMedia_w() const RTC_RUN_ON(worker_thread());
bool IsReadyToSendMedia_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 // NetworkInterface implementation, called by MediaEngine
bool SendPacket(rtc::CopyOnWriteBuffer* packet, bool SendPacket(rtc::CopyOnWriteBuffer* packet,
@ -291,8 +303,8 @@ class BaseChannel : public ChannelInterface,
std::string ToString() const; std::string ToString() const;
private: private:
bool ConnectToRtpTransport() RTC_RUN_ON(network_thread()); bool ConnectToRtpTransport_n() RTC_RUN_ON(network_thread());
void DisconnectFromRtpTransport() RTC_RUN_ON(network_thread()); void DisconnectFromRtpTransport_n() RTC_RUN_ON(network_thread());
void SignalSentPacket_n(const rtc::SentPacket& sent_packet); void SignalSentPacket_n(const rtc::SentPacket& sent_packet);
rtc::Thread* const worker_thread_; rtc::Thread* const worker_thread_;
@ -387,20 +399,22 @@ class VoiceChannel : public BaseChannel {
private: private:
// overrides from BaseChannel // overrides from BaseChannel
void UpdateMediaSendRecvState_w() override; void UpdateMediaSendRecvState_w() RTC_RUN_ON(worker_thread()) override;
bool SetLocalContent_w(const MediaContentDescription* content, bool SetLocalContent_w(const MediaContentDescription* content,
webrtc::SdpType type, webrtc::SdpType type,
std::string* error_desc) override; std::string* error_desc)
RTC_RUN_ON(worker_thread()) override;
bool SetRemoteContent_w(const MediaContentDescription* content, bool SetRemoteContent_w(const MediaContentDescription* content,
webrtc::SdpType type, 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 // Last AudioSendParameters sent down to the media_channel() via
// SetSendParameters. // SetSendParameters.
AudioSendParameters last_send_params_; AudioSendParameters last_send_params_ RTC_GUARDED_BY(worker_thread());
// Last AudioRecvParameters sent down to the media_channel() via // Last AudioRecvParameters sent down to the media_channel() via
// SetRecvParameters. // SetRecvParameters.
AudioRecvParameters last_recv_params_; AudioRecvParameters last_recv_params_ RTC_GUARDED_BY(worker_thread());
}; };
// VideoChannel is a specialization for video. // VideoChannel is a specialization for video.
@ -421,28 +435,30 @@ class VideoChannel : public BaseChannel {
return static_cast<VideoMediaChannel*>(BaseChannel::media_channel()); return static_cast<VideoMediaChannel*>(BaseChannel::media_channel());
} }
void FillBitrateInfo(BandwidthEstimationInfo* bwe_info);
cricket::MediaType media_type() const override { cricket::MediaType media_type() const override {
return cricket::MEDIA_TYPE_VIDEO; return cricket::MEDIA_TYPE_VIDEO;
} }
void FillBitrateInfo(BandwidthEstimationInfo* bwe_info);
private: private:
// overrides from BaseChannel // overrides from BaseChannel
void UpdateMediaSendRecvState_w() override; void UpdateMediaSendRecvState_w() RTC_RUN_ON(worker_thread()) override;
bool SetLocalContent_w(const MediaContentDescription* content, bool SetLocalContent_w(const MediaContentDescription* content,
webrtc::SdpType type, webrtc::SdpType type,
std::string* error_desc) override; std::string* error_desc)
RTC_RUN_ON(worker_thread()) override;
bool SetRemoteContent_w(const MediaContentDescription* content, bool SetRemoteContent_w(const MediaContentDescription* content,
webrtc::SdpType type, 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 // Last VideoSendParameters sent down to the media_channel() via
// SetSendParameters. // SetSendParameters.
VideoSendParameters last_send_params_; VideoSendParameters last_send_params_ RTC_GUARDED_BY(worker_thread());
// Last VideoRecvParameters sent down to the media_channel() via // Last VideoRecvParameters sent down to the media_channel() via
// SetRecvParameters. // SetRecvParameters.
VideoRecvParameters last_recv_params_; VideoRecvParameters last_recv_params_ RTC_GUARDED_BY(worker_thread());
}; };
} // namespace cricket } // namespace cricket