zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove lock from MediaChannel

Browse Source 
Pending messages on network thread for MediaChannel, will be dropped
when the MediaChannel object is deleted (without blocking).

Remove MessageHandler inheritance from Channel since Post-ing to the
network thread has been removed from there.

Copy/pasted code for SendRtp/SendRtcp in WebRtcVideoChannel and
WebRtcVoiceMediaChannel consolidated in MediaChannel.

Bug: webrtc:11993
Change-Id: I05320eb7f86b98adba50ca5eb8b76b78f4111263
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217720
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33955}
This commit is contained in:
Tommi
2021-05-07 18:02:53 +02:00
committed by WebRTC LUCI CQ
parent 5183f00d3a
commit cf2aeffdc2
9 changed files with 131 additions and 212 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
pc/channel.cc
View File

@ -44,11 +44,6 @@ using ::webrtc::PendingTaskSafetyFlag;
using ::webrtc::SdpType;
using ::webrtc::ToQueuedTask;
struct SendPacketMessageData : public rtc::MessageData {
rtc::CopyOnWriteBuffer packet;
rtc::PacketOptions options;
};
// Finds a stream based on target's Primary SSRC or RIDs.
// This struct is used in BaseChannel::UpdateLocalStreams_w.
struct StreamFinder {
@ -84,13 +79,6 @@ struct StreamFinder {
} // namespace
enum {
MSG_SEND_RTP_PACKET = 1,
MSG_SEND_RTCP_PACKET,
MSG_READYTOSENDDATA,
MSG_DATARECEIVED,
};
static void SafeSetError(const std::string& message, std::string* error_desc) {
if (error_desc) {
*error_desc = message;
@ -224,13 +212,10 @@ void BaseChannel::Deinit() {
network_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
RTC_DCHECK_RUN_ON(network_thread());
media_channel_->SetInterface(/*iface=*/nullptr);
FlushRtcpMessages_n();
if (rtp_transport_) {
DisconnectFromRtpTransport();
}
// Clear pending read packets/messages.
network_thread_->Clear(this);
});
}
@ -340,15 +325,7 @@ bool BaseChannel::SendRtcp(rtc::CopyOnWriteBuffer* packet,
int BaseChannel::SetOption(SocketType type,
rtc::Socket::Option opt,
int value) {
return network_thread_->Invoke<int>(RTC_FROM_HERE, [this, type, opt, value] {
RTC_DCHECK_RUN_ON(network_thread());
return SetOption_n(type, opt, value);
});
}
int BaseChannel::SetOption_n(SocketType type,
rtc::Socket::Option opt,
int value) {
RTC_DCHECK_RUN_ON(network_thread());
RTC_DCHECK(rtp_transport_);
switch (type) {
case ST_RTP:
@ -403,6 +380,7 @@ void BaseChannel::OnTransportReadyToSend(bool ready) {
bool BaseChannel::SendPacket(bool rtcp,
rtc::CopyOnWriteBuffer* packet,
const rtc::PacketOptions& options) {
RTC_DCHECK_RUN_ON(network_thread());
// Until all the code is migrated to use RtpPacketType instead of bool.
RtpPacketType packet_type = rtcp ? RtpPacketType::kRtcp : RtpPacketType::kRtp;
// SendPacket gets called from MediaEngine, on a pacer or an encoder thread.
@ -412,16 +390,6 @@ bool BaseChannel::SendPacket(bool rtcp,
// SRTP and the inner workings of the transport channels.
// The only downside is that we can't return a proper failure code if
// needed. Since UDP is unreliable anyway, this should be a non-issue.
if (!network_thread_->IsCurrent()) {
// Avoid a copy by transferring the ownership of the packet data.
int message_id = rtcp ? MSG_SEND_RTCP_PACKET : MSG_SEND_RTP_PACKET;
SendPacketMessageData* data = new SendPacketMessageData;
data->packet = std::move(*packet);
data->options = options;
network_thread_->Post(RTC_FROM_HERE, this, message_id, data);
return true;
}
RTC_DCHECK_RUN_ON(network_thread());
TRACE_EVENT0("webrtc", "BaseChannel::SendPacket");
@ -794,22 +762,6 @@ RtpHeaderExtensions BaseChannel::GetFilteredRtpHeaderExtensions(
return webrtc::RtpExtension::FilterDuplicateNonEncrypted(extensions);
}
void BaseChannel::OnMessage(rtc::Message* pmsg) {
TRACE_EVENT0("webrtc", "BaseChannel::OnMessage");
switch (pmsg->message_id) {
case MSG_SEND_RTP_PACKET:
case MSG_SEND_RTCP_PACKET: {
RTC_DCHECK_RUN_ON(network_thread());
SendPacketMessageData* data =
static_cast<SendPacketMessageData*>(pmsg->pdata);
bool rtcp = pmsg->message_id == MSG_SEND_RTCP_PACKET;
SendPacket(rtcp, &data->packet, data->options);
delete data;
break;
}
}
}
void BaseChannel::MaybeAddHandledPayloadType(int payload_type) {
if (payload_type_demuxing_enabled_) {
demuxer_criteria_.payload_types.insert(static_cast<uint8_t>(payload_type));
@ -824,17 +776,6 @@ void BaseChannel::ClearHandledPayloadTypes() {
payload_types_.clear();
}
void BaseChannel::FlushRtcpMessages_n() {
// Flush all remaining RTCP messages. This should only be called in
// destructor.
rtc::MessageList rtcp_messages;
network_thread_->Clear(this, MSG_SEND_RTCP_PACKET, &rtcp_messages);
for (const auto& message : rtcp_messages) {
network_thread_->Send(RTC_FROM_HERE, this, MSG_SEND_RTCP_PACKET,
message.pdata);
}
}
void BaseChannel::SignalSentPacket_n(const rtc::SentPacket& sent_packet) {
RTC_DCHECK_RUN_ON(network_thread());
media_channel()->OnPacketSent(sent_packet);

10
pc/channel.h
View File

@ -54,7 +54,6 @@
#include "rtc_base/checks.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/location.h"
#include "rtc_base/message_handler.h"
#include "rtc_base/network.h"
#include "rtc_base/network/sent_packet.h"
#include "rtc_base/network_route.h"
@ -93,8 +92,6 @@ struct CryptoParams;
// NetworkInterface.
class BaseChannel : public ChannelInterface,
// TODO(tommi): Remove MessageHandler inheritance.
public rtc::MessageHandler,
// TODO(tommi): Remove has_slots inheritance.
public sigslot::has_slots<>,
// TODO(tommi): Consider implementing these interfaces
@ -186,8 +183,6 @@ class BaseChannel : public ChannelInterface,
// Only public for unit tests. Otherwise, consider protected.
int SetOption(SocketType type, rtc::Socket::Option o, int val) override;
int SetOption_n(SocketType type, rtc::Socket::Option o, int val)
RTC_RUN_ON(network_thread());
// RtpPacketSinkInterface overrides.
void OnRtpPacket(const webrtc::RtpPacketReceived& packet) override;
@ -223,8 +218,6 @@ class BaseChannel : public ChannelInterface,
bool IsReadyToSendMedia_w() const RTC_RUN_ON(worker_thread());
rtc::Thread* signaling_thread() const { return signaling_thread_; }
void FlushRtcpMessages_n() RTC_RUN_ON(network_thread());
// NetworkInterface implementation, called by MediaEngine
bool SendPacket(rtc::CopyOnWriteBuffer* packet,
const rtc::PacketOptions& options) override;
@ -285,9 +278,6 @@ class BaseChannel : public ChannelInterface,
RtpHeaderExtensions GetFilteredRtpHeaderExtensions(
const RtpHeaderExtensions& extensions);
// From MessageHandler
void OnMessage(rtc::Message* pmsg) override;
// Add |payload_type| to |demuxer_criteria_| if payload type demuxing is
// enabled.
void MaybeAddHandledPayloadType(int payload_type) RTC_RUN_ON(worker_thread());

58
pc/channel_unittest.cc
View File

@ -398,25 +398,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> {
return result;
}
bool Terminate() {
channel1_.reset();
channel2_.reset();
fake_rtp_dtls_transport1_.reset();
fake_rtcp_dtls_transport1_.reset();
fake_rtp_dtls_transport2_.reset();
fake_rtcp_dtls_transport2_.reset();
fake_rtp_packet_transport1_.reset();
fake_rtcp_packet_transport1_.reset();
fake_rtp_packet_transport2_.reset();
fake_rtcp_packet_transport2_.reset();
if (network_thread_keeper_) {
RTC_DCHECK_EQ(network_thread_, network_thread_keeper_.get());
network_thread_ = nullptr;
network_thread_keeper_.reset();
}
return true;
}
void SendRtp(typename T::MediaChannel* media_channel, rtc::Buffer data) {
network_thread_->PostTask(webrtc::ToQueuedTask(
network_thread_safety_, [media_channel, data = std::move(data)]() {
@ -917,29 +898,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> {
EXPECT_EQ(1U, media_channel2()->codecs().size());
}
// Test that we don't crash if packets are sent during call teardown
// when RTCP mux is enabled. This is a regression test against a specific
// race condition that would only occur when a RTCP packet was sent during
// teardown of a channel on which RTCP mux was enabled.
void TestCallTeardownRtcpMux() {
class LastWordMediaChannel : public T::MediaChannel {
public:
explicit LastWordMediaChannel(rtc::Thread* network_thread)
: T::MediaChannel(NULL, typename T::Options(), network_thread) {}
~LastWordMediaChannel() {
T::MediaChannel::SendRtp(kPcmuFrame, sizeof(kPcmuFrame),
rtc::PacketOptions());
T::MediaChannel::SendRtcp(kRtcpReport, sizeof(kRtcpReport));
}
};
CreateChannels(std::make_unique<LastWordMediaChannel>(network_thread_),
std::make_unique<LastWordMediaChannel>(network_thread_),
RTCP_MUX, RTCP_MUX);
EXPECT_TRUE(SendInitiate());
EXPECT_TRUE(SendAccept());
EXPECT_TRUE(Terminate());
}
// Send voice RTP data to the other side and ensure it gets there.
void SendRtpToRtp() {
CreateChannels(RTCP_MUX, RTCP_MUX);
@ -1668,10 +1626,6 @@ TEST_F(VoiceChannelSingleThreadTest, TestCallSetup) {
Base::TestCallSetup();
}
TEST_F(VoiceChannelSingleThreadTest, TestCallTeardownRtcpMux) {
Base::TestCallTeardownRtcpMux();
}
TEST_F(VoiceChannelSingleThreadTest, SendRtpToRtp) {
Base::SendRtpToRtp();
}
@ -1809,10 +1763,6 @@ TEST_F(VoiceChannelDoubleThreadTest, TestCallSetup) {
Base::TestCallSetup();
}
TEST_F(VoiceChannelDoubleThreadTest, TestCallTeardownRtcpMux) {
Base::TestCallTeardownRtcpMux();
}
TEST_F(VoiceChannelDoubleThreadTest, SendRtpToRtp) {
Base::SendRtpToRtp();
}
@ -1948,10 +1898,6 @@ TEST_F(VideoChannelSingleThreadTest, TestCallSetup) {
Base::TestCallSetup();
}
TEST_F(VideoChannelSingleThreadTest, TestCallTeardownRtcpMux) {
Base::TestCallTeardownRtcpMux();
}
TEST_F(VideoChannelSingleThreadTest, SendRtpToRtp) {
Base::SendRtpToRtp();
}
@ -2237,10 +2183,6 @@ TEST_F(VideoChannelDoubleThreadTest, TestCallSetup) {
Base::TestCallSetup();
}
TEST_F(VideoChannelDoubleThreadTest, TestCallTeardownRtcpMux) {
Base::TestCallTeardownRtcpMux();
}
TEST_F(VideoChannelDoubleThreadTest, SendRtpToRtp) {
Base::SendRtpToRtp();
}