From 6f542d5e92e7b15abdbb6cdcfff7b5d145141153 Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 24 Jan 2022 13:36:40 +0100 Subject: [PATCH] Move call to TransportFeedbackDemuxer::AddPacket to transport queue. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before, this call was being made from the SendPacket path of the pacer. The transport will post a task to the transport queue regardless so this change moves the lock inside of the demuxer away from the pacer and over to the transport queue. Moving forward, the calls to register/unregister with the feedback demuxer, will occur on the transport queue as well and we can change the transport OnTransportFeedback() implementation to forward the calls to the demuxer on the transport queue as well. That will bring all calls into the same execution context and we won't need a lock. Bug: webrtc:13517, webrtc:11993 Change-Id: If714ca6d2b164a1a2b6bcb8c99787372064a31a0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/248164 Reviewed-by: Erik Språng Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#35775} --- call/rtp_transport_controller_send.cc | 3 +-- .../congestion_controller/rtp/transport_feedback_demuxer.cc | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index c9388e47aa..fc4f483087 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -559,11 +559,10 @@ void RtpTransportControllerSend::OnReceivedRtcpReceiverReport( void RtpTransportControllerSend::OnAddPacket( const RtpPacketSendInfo& packet_info) { - feedback_demuxer_.AddPacket(packet_info); - Timestamp creation_time = Timestamp::Millis(clock_->TimeInMilliseconds()); task_queue_.PostTask([this, packet_info, creation_time]() { RTC_DCHECK_RUN_ON(&task_queue_); + feedback_demuxer_.AddPacket(packet_info); transport_feedback_adapter_.AddPacket( packet_info, send_side_bwe_with_overhead_ ? transport_overhead_bytes_per_packet_ : 0, diff --git a/modules/congestion_controller/rtp/transport_feedback_demuxer.cc b/modules/congestion_controller/rtp/transport_feedback_demuxer.cc index 62b85b10b8..29accb5be0 100644 --- a/modules/congestion_controller/rtp/transport_feedback_demuxer.cc +++ b/modules/congestion_controller/rtp/transport_feedback_demuxer.cc @@ -44,6 +44,10 @@ void TransportFeedbackDemuxer::DeRegisterStreamFeedbackObserver( } void TransportFeedbackDemuxer::AddPacket(const RtpPacketSendInfo& packet_info) { + // Currently called on the send transport queue. + // TODO(tommi): When registration/unregistration as well as + // OnTransportFeedback callbacks occur on the transport queue, we can remove + // this lock. MutexLock lock(&lock_); StreamFeedbackObserver::StreamPacketInfo info;