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

Handle OnPacketSent on the network thread via MediaChannel.

Browse Source 
* Adds a OnPacketSent callback to MediaChannel, which matches with
  MediaChannel::NetworkInterface::SendPacket.
* Moves the OnPacketSent handling to the media channel implementations
  (video/voice) and removes the PeerConnection/SdpOfferAnswerHandler
  layer from the call path.
* Call::OnSentPacket is called directly from the channels on the network
  thread. This eliminates a PostTask to the worker thread for every
  audio/video network packet.
* Remove sigslot dependency from MediaChannel (and derived).

Bug: webrtc:11993
Change-Id: I1f79a7aa60f05d47e1882f9be1c9323ea8fac5f6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215403
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33777}
This commit is contained in:
Tomas Gunnarsson 2021-04-19 12:53:09 +02:00 committed by Commit Bot
parent edb7ea2e69
commit eb9c3f237b
15 changed files with 155 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
call/call.cc
View File

@ -1250,6 +1250,12 @@ void Call::UpdateAggregateNetworkState() {
}
void Call::OnSentPacket(const rtc::SentPacket& sent_packet) {
// In production and with most tests, this method will be called on the
// network thread. However some test classes such as DirectTransport don't
// incorporate a network thread. This means that tests for RtpSenderEgress
// and ModuleRtpRtcpImpl2 that use DirectTransport, will call this method
// on a ProcessThread. This is alright as is since we forward the call to
// implementations that either just do a PostTask or use locking.
video_send_delay_stats_->OnSentPacket(sent_packet.packet_id,
clock_->TimeInMilliseconds());
transport_send_ptr_->OnSentPacket(sent_packet);

6
call/rtp_transport_controller_send_interface.h
View File

@ -135,7 +135,13 @@ class RtpTransportControllerSendInterface {
virtual int64_t GetPacerQueuingDelayMs() const = 0;
virtual absl::optional<Timestamp> GetFirstPacketTime() const = 0;
virtual void EnablePeriodicAlrProbing(bool enable) = 0;
// Called when a packet has been sent.
// The call should arrive on the network thread, but may not in all cases
// (some tests don't adhere to this). Implementations today should not block
// the calling thread or make assumptions about the thread context.
virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0;
virtual void OnReceivedPacket(const ReceivedPacket& received_packet) = 0;
virtual void SetSdpBitrateParameters(

12
media/base/fake_media_engine.h
View File

@ -267,14 +267,14 @@ class RtpHelper : public Base {
void set_recv_rtcp_parameters(const RtcpParameters& params) {
recv_rtcp_parameters_ = params;
}
virtual void OnPacketReceived(rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) {
void OnPacketReceived(rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) override {
rtp_packets_.push_back(std::string(packet.cdata<char>(), packet.size()));
}
virtual void OnReadyToSend(bool ready) { ready_to_send_ = ready; }
virtual void OnNetworkRouteChanged(const std::string& transport_name,
const rtc::NetworkRoute& network_route) {
void OnPacketSent(const rtc::SentPacket& sent_packet) override {}
void OnReadyToSend(bool ready) override { ready_to_send_ = ready; }
void OnNetworkRouteChanged(const std::string& transport_name,
const rtc::NetworkRoute& network_route) override {
last_network_route_ = network_route;
++num_network_route_changes_;
transport_overhead_per_packet_ = network_route.packet_overhead;

82
media/base/media_channel.cc
View File

@ -47,13 +47,95 @@ void MediaChannel::SetFrameDecryptor(
void MediaChannel::SetVideoCodecSwitchingEnabled(bool enabled) {}
bool MediaChannel::SendPacket(rtc::CopyOnWriteBuffer* packet,
const rtc::PacketOptions& options) {
return DoSendPacket(packet, false, options);
}
bool MediaChannel::SendRtcp(rtc::CopyOnWriteBuffer* packet,
const rtc::PacketOptions& options) {
return DoSendPacket(packet, true, options);
}
int MediaChannel::SetOption(NetworkInterface::SocketType type,
rtc::Socket::Option opt,
int option)
RTC_LOCKS_EXCLUDED(network_interface_mutex_) {
webrtc::MutexLock lock(&network_interface_mutex_);
return SetOptionLocked(type, opt, option);
}
// Corresponds to the SDP attribute extmap-allow-mixed, see RFC8285.
// Set to true if it's allowed to mix one- and two-byte RTP header extensions
// in the same stream. The setter and getter must only be called from
// worker_thread.
void MediaChannel::SetExtmapAllowMixed(bool extmap_allow_mixed) {
extmap_allow_mixed_ = extmap_allow_mixed;
}
bool MediaChannel::ExtmapAllowMixed() const {
return extmap_allow_mixed_;
}
void MediaChannel::SetEncoderToPacketizerFrameTransformer(
uint32_t ssrc,
rtc::scoped_refptr<webrtc::FrameTransformerInterface> frame_transformer) {}
void MediaChannel::SetDepacketizerToDecoderFrameTransformer(
uint32_t ssrc,
rtc::scoped_refptr<webrtc::FrameTransformerInterface> frame_transformer) {}
int MediaChannel::SetOptionLocked(NetworkInterface::SocketType type,
rtc::Socket::Option opt,
int option) {
if (!network_interface_)
return -1;
return network_interface_->SetOption(type, opt, option);
}
bool MediaChannel::DscpEnabled() const {
return enable_dscp_;
}
// This is the DSCP value used for both RTP and RTCP channels if DSCP is
// enabled. It can be changed at any time via |SetPreferredDscp|.
rtc::DiffServCodePoint MediaChannel::PreferredDscp() const {
webrtc::MutexLock lock(&network_interface_mutex_);
return preferred_dscp_;
}
int MediaChannel::SetPreferredDscp(rtc::DiffServCodePoint preferred_dscp) {
webrtc::MutexLock lock(&network_interface_mutex_);
if (preferred_dscp == preferred_dscp_) {
return 0;
}
preferred_dscp_ = preferred_dscp;
return UpdateDscp();
}
int MediaChannel::UpdateDscp() {
rtc::DiffServCodePoint value =
enable_dscp_ ? preferred_dscp_ : rtc::DSCP_DEFAULT;
int ret =
SetOptionLocked(NetworkInterface::ST_RTP, rtc::Socket::OPT_DSCP, value);
if (ret == 0) {
ret = SetOptionLocked(NetworkInterface::ST_RTCP, rtc::Socket::OPT_DSCP,
value);
}
return ret;
}
bool MediaChannel::DoSendPacket(rtc::CopyOnWriteBuffer* packet,
bool rtcp,
const rtc::PacketOptions& options) {
webrtc::MutexLock lock(&network_interface_mutex_);
if (!network_interface_)
return false;
return (!rtcp) ? network_interface_->SendPacket(packet, options)
: network_interface_->SendRtcp(packet, options);
}
MediaSenderInfo::MediaSenderInfo() = default;
MediaSenderInfo::~MediaSenderInfo() = default;

70
media/base/media_channel.h
View File

@ -51,7 +51,6 @@
#include "rtc_base/string_encode.h"
#include "rtc_base/strings/string_builder.h"
#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/third_party/sigslot/sigslot.h"
namespace rtc {
class Timing;
@ -154,7 +153,7 @@ struct VideoOptions {
}
};
class MediaChannel : public sigslot::has_slots<> {
class MediaChannel {
public:
class NetworkInterface {
public:
@ -171,7 +170,7 @@ class MediaChannel : public sigslot::has_slots<> {
explicit MediaChannel(const MediaConfig& config);
MediaChannel();
~MediaChannel() override;
virtual ~MediaChannel();
virtual cricket::MediaType media_type() const = 0;
@ -181,6 +180,9 @@ class MediaChannel : public sigslot::has_slots<> {
// Called on the network when an RTP packet is received.
virtual void OnPacketReceived(rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) = 0;
// Called on the network thread after a transport has finished sending a
// packet.
virtual void OnPacketSent(const rtc::SentPacket& sent_packet) = 0;
// Called when the socket's ability to send has changed.
virtual void OnReadyToSend(bool ready) = 0;
// Called when the network route used for sending packets changed.
@ -239,30 +241,21 @@ class MediaChannel : public sigslot::has_slots<> {
// Base method to send packet using NetworkInterface.
bool SendPacket(rtc::CopyOnWriteBuffer* packet,
const rtc::PacketOptions& options) {
return DoSendPacket(packet, false, options);
}
const rtc::PacketOptions& options);
bool SendRtcp(rtc::CopyOnWriteBuffer* packet,
const rtc::PacketOptions& options) {
return DoSendPacket(packet, true, options);
}
const rtc::PacketOptions& options);
int SetOption(NetworkInterface::SocketType type,
rtc::Socket::Option opt,
int option) RTC_LOCKS_EXCLUDED(network_interface_mutex_) {
webrtc::MutexLock lock(&network_interface_mutex_);
return SetOptionLocked(type, opt, option);
}
int option) RTC_LOCKS_EXCLUDED(network_interface_mutex_);
// Corresponds to the SDP attribute extmap-allow-mixed, see RFC8285.
// Set to true if it's allowed to mix one- and two-byte RTP header extensions
// in the same stream. The setter and getter must only be called from
// worker_thread.
void SetExtmapAllowMixed(bool extmap_allow_mixed) {
extmap_allow_mixed_ = extmap_allow_mixed;
}
bool ExtmapAllowMixed() const { return extmap_allow_mixed_; }
void SetExtmapAllowMixed(bool extmap_allow_mixed);
bool ExtmapAllowMixed() const;
virtual webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const = 0;
virtual webrtc::RTCError SetRtpSendParameters(
@ -280,58 +273,27 @@ class MediaChannel : public sigslot::has_slots<> {
int SetOptionLocked(NetworkInterface::SocketType type,
rtc::Socket::Option opt,
int option)
RTC_EXCLUSIVE_LOCKS_REQUIRED(network_interface_mutex_) {
if (!network_interface_)
return -1;
return network_interface_->SetOption(type, opt, option);
}
RTC_EXCLUSIVE_LOCKS_REQUIRED(network_interface_mutex_);
bool DscpEnabled() const { return enable_dscp_; }
bool DscpEnabled() const;
// This is the DSCP value used for both RTP and RTCP channels if DSCP is
// enabled. It can be changed at any time via |SetPreferredDscp|.
rtc::DiffServCodePoint PreferredDscp() const
RTC_LOCKS_EXCLUDED(network_interface_mutex_) {
webrtc::MutexLock lock(&network_interface_mutex_);
return preferred_dscp_;
}
RTC_LOCKS_EXCLUDED(network_interface_mutex_);
int SetPreferredDscp(rtc::DiffServCodePoint preferred_dscp)
RTC_LOCKS_EXCLUDED(network_interface_mutex_) {
webrtc::MutexLock lock(&network_interface_mutex_);
if (preferred_dscp == preferred_dscp_) {
return 0;
}
preferred_dscp_ = preferred_dscp;
return UpdateDscp();
}
RTC_LOCKS_EXCLUDED(network_interface_mutex_);
private:
// Apply the preferred DSCP setting to the underlying network interface RTP
// and RTCP channels. If DSCP is disabled, then apply the default DSCP value.
int UpdateDscp() RTC_EXCLUSIVE_LOCKS_REQUIRED(network_interface_mutex_) {
rtc::DiffServCodePoint value =
enable_dscp_ ? preferred_dscp_ : rtc::DSCP_DEFAULT;
int ret =
SetOptionLocked(NetworkInterface::ST_RTP, rtc::Socket::OPT_DSCP, value);
if (ret == 0) {
ret = SetOptionLocked(NetworkInterface::ST_RTCP, rtc::Socket::OPT_DSCP,
value);
}
return ret;
}
int UpdateDscp() RTC_EXCLUSIVE_LOCKS_REQUIRED(network_interface_mutex_);
bool DoSendPacket(rtc::CopyOnWriteBuffer* packet,
bool rtcp,
const rtc::PacketOptions& options)
RTC_LOCKS_EXCLUDED(network_interface_mutex_) {
webrtc::MutexLock lock(&network_interface_mutex_);
if (!network_interface_)
return false;
return (!rtcp) ? network_interface_->SendPacket(packet, options)
: network_interface_->SendRtcp(packet, options);
}
RTC_LOCKS_EXCLUDED(network_interface_mutex_);
const bool enable_dscp_;
// |network_interface_| can be accessed from the worker_thread and

12
media/engine/webrtc_video_engine.cc
View File

@ -1789,6 +1789,18 @@ void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet,
}));
}
void WebRtcVideoChannel::OnPacketSent(const rtc::SentPacket& sent_packet) {
RTC_DCHECK_RUN_ON(&network_thread_checker_);
// TODO(tommi): We shouldn't need to go through call_ to deliver this
// notification. We should already have direct access to
// video_send_delay_stats_ and transport_send_ptr_ via `stream_`.
// So we should be able to remove OnSentPacket from Call and handle this per
// channel instead. At the moment Call::OnSentPacket calls OnSentPacket for
// the video stats, for all sent packets, including audio, which causes
// unnecessary lookups.
call_->OnSentPacket(sent_packet);
}
void WebRtcVideoChannel::BackfillBufferedPackets(
rtc::ArrayView<const uint32_t> ssrcs) {
RTC_DCHECK_RUN_ON(&thread_checker_);

1
media/engine/webrtc_video_engine.h
View File

@ -170,6 +170,7 @@ class WebRtcVideoChannel : public VideoMediaChannel,
void OnPacketReceived(rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) override;
void OnPacketSent(const rtc::SentPacket& sent_packet) override;
void OnReadyToSend(bool ready) override;
void OnNetworkRouteChanged(const std::string& transport_name,
const rtc::NetworkRoute& network_route) override;

11
media/engine/webrtc_voice_engine.cc
View File

@ -2297,6 +2297,17 @@ void WebRtcVoiceMediaChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet,
}));
}
void WebRtcVoiceMediaChannel::OnPacketSent(const rtc::SentPacket& sent_packet) {
RTC_DCHECK_RUN_ON(&network_thread_checker_);
// TODO(tommi): We shouldn't need to go through call_ to deliver this
// notification. We should already have direct access to
// video_send_delay_stats_ and transport_send_ptr_ via `stream_`.
// So we should be able to remove OnSentPacket from Call and handle this per
// channel instead. At the moment Call::OnSentPacket calls OnSentPacket for
// the video stats, which we should be able to skip.
call_->OnSentPacket(sent_packet);
}
void WebRtcVoiceMediaChannel::OnNetworkRouteChanged(
const std::string& transport_name,
const rtc::NetworkRoute& network_route) {

1
media/engine/webrtc_voice_engine.h
View File

@ -217,6 +217,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel,
void OnPacketReceived(rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) override;
void OnPacketSent(const rtc::SentPacket& sent_packet) override;
void OnNetworkRouteChanged(const std::string& transport_name,
const rtc::NetworkRoute& network_route) override;
void OnReadyToSend(bool ready) override;

16
pc/channel.cc
View File

@ -174,6 +174,8 @@ std::string BaseChannel::ToString() const {
bool BaseChannel::ConnectToRtpTransport() {
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.
bool result = rtp_transport_->RegisterRtpDemuxerSink(demuxer_criteria_, this);
@ -197,6 +199,7 @@ bool BaseChannel::ConnectToRtpTransport() {
void BaseChannel::DisconnectFromRtpTransport() {
RTC_DCHECK(rtp_transport_);
RTC_DCHECK(media_channel());
rtp_transport_->UnregisterRtpDemuxerSink(this);
rtp_transport_->SignalReadyToSend.disconnect(this);
rtp_transport_->SignalNetworkRouteChanged.disconnect(this);
@ -389,13 +392,6 @@ sigslot::signal1<ChannelInterface*>& BaseChannel::SignalFirstPacketReceived() {
return SignalFirstPacketReceived_;
}
sigslot::signal1<const rtc::SentPacket&>& BaseChannel::SignalSentPacket() {
// TODO(bugs.webrtc.org/11994): Uncomment this check once callers have been
// fixed to access this variable from the correct thread.
// RTC_DCHECK_RUN_ON(worker_thread_);
return SignalSentPacket_;
}
void BaseChannel::OnTransportReadyToSend(bool ready) {
RTC_DCHECK_RUN_ON(network_thread());
media_channel_->OnReadyToSend(ready);
@ -844,10 +840,8 @@ void BaseChannel::FlushRtcpMessages_n() {
}
void BaseChannel::SignalSentPacket_n(const rtc::SentPacket& sent_packet) {
worker_thread_->PostTask(ToQueuedTask(alive_, [this, sent_packet] {
RTC_DCHECK_RUN_ON(worker_thread());
SignalSentPacket()(sent_packet);
}));
RTC_DCHECK_RUN_ON(network_thread());
media_channel()->OnPacketSent(sent_packet);
}
void BaseChannel::SetNegotiatedHeaderExtensions_w(

8
pc/channel.h
View File

@ -179,9 +179,6 @@ class BaseChannel : public ChannelInterface,
// Used for latency measurements.
sigslot::signal1<ChannelInterface*>& SignalFirstPacketReceived() override;
// Forward SignalSentPacket to worker thread.
sigslot::signal1<const rtc::SentPacket&>& SignalSentPacket();
// From RtpTransport - public for testing only
void OnTransportReadyToSend(bool ready);
@ -319,8 +316,7 @@ class BaseChannel : public ChannelInterface,
private:
bool ConnectToRtpTransport() RTC_RUN_ON(network_thread());
void DisconnectFromRtpTransport() RTC_RUN_ON(network_thread());
void SignalSentPacket_n(const rtc::SentPacket& sent_packet)
RTC_RUN_ON(network_thread());
void SignalSentPacket_n(const rtc::SentPacket& sent_packet);
rtc::Thread* const worker_thread_;
rtc::Thread* const network_thread_;
@ -328,8 +324,6 @@ class BaseChannel : public ChannelInterface,
rtc::scoped_refptr<webrtc::PendingTaskSafetyFlag> alive_;
sigslot::signal1<ChannelInterface*> SignalFirstPacketReceived_
RTC_GUARDED_BY(signaling_thread_);
sigslot::signal1<const rtc::SentPacket&> SignalSentPacket_
RTC_GUARDED_BY(worker_thread_);
const std::string content_name_;

6
pc/peer_connection.cc
View File

@ -2796,12 +2796,6 @@ void PeerConnection::ReportNegotiatedCiphers(
}
}
void PeerConnection::OnSentPacket_w(const rtc::SentPacket& sent_packet) {
RTC_DCHECK_RUN_ON(worker_thread());
RTC_DCHECK(call_);
call_->OnSentPacket(sent_packet);
}
bool PeerConnection::OnTransportChanged(
const std::string& mid,
RtpTransportInternal* rtp_transport,

2
pc/peer_connection.h
View File

@ -426,8 +426,6 @@ class PeerConnection : public PeerConnectionInternal,
// this session.
bool SrtpRequired() const;
void OnSentPacket_w(const rtc::SentPacket& sent_packet);
bool SetupDataChannelTransport_n(const std::string& mid)
RTC_RUN_ON(network_thread());
void TeardownDataChannelTransport_n() RTC_RUN_ON(network_thread());

17
pc/sdp_offer_answer.cc
View File

@ -4520,17 +4520,10 @@ cricket::VoiceChannel* SdpOfferAnswerHandler::CreateVoiceChannel(
// TODO(bugs.webrtc.org/11992): CreateVoiceChannel internally switches to the
// worker thread. We shouldn't be using the |call_ptr_| hack here but simply
// be on the worker thread and use |call_| (update upstream code).
cricket::VoiceChannel* voice_channel = channel_manager()->CreateVoiceChannel(
return channel_manager()->CreateVoiceChannel(
pc_->call_ptr(), pc_->configuration()->media_config, rtp_transport,
signaling_thread(), mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(),
&ssrc_generator_, audio_options());
if (!voice_channel) {
return nullptr;
}
voice_channel->SignalSentPacket().connect(pc_,
&PeerConnection::OnSentPacket_w);
return voice_channel;
}
// TODO(steveanton): Perhaps this should be managed by the RtpTransceiver.
@ -4546,17 +4539,11 @@ cricket::VideoChannel* SdpOfferAnswerHandler::CreateVideoChannel(
// TODO(bugs.webrtc.org/11992): CreateVideoChannel internally switches to the
// worker thread. We shouldn't be using the |call_ptr_| hack here but simply
// be on the worker thread and use |call_| (update upstream code).
cricket::VideoChannel* video_channel = channel_manager()->CreateVideoChannel(
return channel_manager()->CreateVideoChannel(
pc_->call_ptr(), pc_->configuration()->media_config, rtp_transport,
signaling_thread(), mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(),
&ssrc_generator_, video_options(),
video_bitrate_allocator_factory_.get());
if (!video_channel) {
return nullptr;
}
video_channel->SignalSentPacket().connect(pc_,
&PeerConnection::OnSentPacket_w);
return video_channel;
}
bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) {

6
video/send_delay_stats.h
View File

@ -27,6 +27,12 @@
namespace webrtc {
// Used to collect delay stats for video streams. The class gets callbacks
// from more than one threads and internally uses a mutex for data access
// synchronization.
// TODO(bugs.webrtc.org/11993): OnSendPacket and OnSentPacket will eventually
// be called consistently on the same thread. Once we're there, we should be
// able to avoid locking (at least for the fast path).
class SendDelayStats : public SendPacketObserver {
public:
explicit SendDelayStats(Clock* clock);