diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 025ae651e7..9a1aad90c1 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -289,6 +289,10 @@ void ModuleRtpRtcpImpl2::SetCsrcs(const std::vector& csrcs) { // TODO(pbos): Handle media and RTX streams separately (separate RTCP // feedbacks). RTCPSender::FeedbackState ModuleRtpRtcpImpl2::GetFeedbackState() { + // TODO(bugs.webrtc.org/11581): Called by potentially multiple threads. + // "Send*" methods and on the ProcessThread. Make sure it's only called on the + // construction thread. + RTCPSender::FeedbackState state; // This is called also when receiver_only is true. Hence below // checks that rtp_sender_ exists. @@ -653,6 +657,7 @@ void ModuleRtpRtcpImpl2::BitrateSent(uint32_t* total_rate, uint32_t* video_rate, uint32_t* fec_rate, uint32_t* nack_rate) const { + RTC_DCHECK_RUN_ON(worker_queue_); RtpSendRates send_rates = rtp_sender_->packet_sender.GetSendRates(); *total_rate = send_rates.Sum().bps(); if (video_rate) @@ -663,6 +668,7 @@ void ModuleRtpRtcpImpl2::BitrateSent(uint32_t* total_rate, } RtpSendRates ModuleRtpRtcpImpl2::GetSendRates() const { + RTC_DCHECK_RUN_ON(worker_queue_); return rtp_sender_->packet_sender.GetSendRates(); } diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index 50a997af63..7196dcd659 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -17,7 +17,6 @@ #include "absl/strings/match.h" #include "api/transport/field_trial_based_config.h" #include "logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h" -#include "modules/remote_bitrate_estimator/test/bwe_test_logging.h" #include "rtc_base/logging.h" #include "rtc_base/task_utils/to_queued_task.h" @@ -70,7 +69,9 @@ RtpSenderEgress::RtpSenderEgress(const RtpRtcpInterface::Configuration& config, packet_history_(packet_history), transport_(config.outgoing_transport), event_log_(config.event_log), +#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE is_audio_(config.audio), +#endif need_rtp_packet_infos_(config.need_rtp_packet_infos), transport_feedback_observer_(config.transport_feedback_callback), send_side_delay_observer_(config.send_side_delay_observer), @@ -110,30 +111,18 @@ void RtpSenderEgress::SendPacket(RtpPacketToSend* packet, RTC_DCHECK_RUN_ON(&pacer_checker_); RTC_DCHECK(packet); - const uint32_t packet_ssrc = packet->Ssrc(); RTC_DCHECK(packet->packet_type().has_value()); RTC_DCHECK(HasCorrectSsrc(*packet)); - int64_t now_ms = clock_->TimeInMilliseconds(); - if (is_audio_) { + const uint32_t packet_ssrc = packet->Ssrc(); + const int64_t now_ms = clock_->TimeInMilliseconds(); + #if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE - BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "AudioTotBitrate_kbps", now_ms, - GetSendRates().Sum().kbps(), packet_ssrc); - BWE_TEST_LOGGING_PLOT_WITH_SSRC( - 1, "AudioNackBitrate_kbps", now_ms, - GetSendRates()[RtpPacketMediaType::kRetransmission].kbps(), - packet_ssrc); + worker_queue_->PostTask( + ToQueuedTask(task_safety_, [this, now_ms, packet_ssrc]() { + BweTestLoggingPlot(now_ms, packet_ssrc); + })); #endif - } else { -#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE - BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoTotBitrate_kbps", now_ms, - GetSendRates().Sum().kbps(), packet_ssrc); - BWE_TEST_LOGGING_PLOT_WITH_SSRC( - 1, "VideoNackBitrate_kbps", now_ms, - GetSendRates()[RtpPacketMediaType::kRetransmission].kbps(), - packet_ssrc); -#endif - } if (need_rtp_packet_infos_ && packet->packet_type() == RtpPacketToSend::Type::kVideo) { @@ -226,11 +215,12 @@ void RtpSenderEgress::SendPacket(RtpPacketToSend* packet, RtpPacketMediaType packet_type = *packet->packet_type(); RtpPacketCounter counter(*packet); size_t size = packet->size(); - worker_queue_->PostTask(ToQueuedTask( - task_safety_, [this, now_ms, ssrc = packet->Ssrc(), packet_type, - counter = std::move(counter), size]() { + worker_queue_->PostTask( + ToQueuedTask(task_safety_, [this, now_ms, packet_ssrc, packet_type, + counter = std::move(counter), size]() { RTC_DCHECK_RUN_ON(worker_queue_); - UpdateRtpStats(now_ms, ssrc, packet_type, std::move(counter), size); + UpdateRtpStats(now_ms, packet_ssrc, packet_type, std::move(counter), + size); })); } } @@ -516,4 +506,25 @@ void RtpSenderEgress::PeriodicUpdate() { send_rates[RtpPacketMediaType::kRetransmission].bps(), ssrc_); } +#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE +void RtpSenderEgress::BweTestLoggingPlot(int64_t now_ms, uint32_t packet_ssrc) { + RTC_DCHECK_RUN_ON(worker_queue_); + + const auto rates = GetSendRates(); + if (is_audio_) { + BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "AudioTotBitrate_kbps", now_ms, + rates.Sum().kbps(), packet_ssrc); + BWE_TEST_LOGGING_PLOT_WITH_SSRC( + 1, "AudioNackBitrate_kbps", now_ms, + rates[RtpPacketMediaType::kRetransmission].kbps(), packet_ssrc); + } else { + BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoTotBitrate_kbps", now_ms, + rates.Sum().kbps(), packet_ssrc); + BWE_TEST_LOGGING_PLOT_WITH_SSRC( + 1, "VideoNackBitrate_kbps", now_ms, + rates[RtpPacketMediaType::kRetransmission].kbps(), packet_ssrc); + } +} +#endif // BWE_TEST_LOGGING_COMPILE_TIME_ENABLE + } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.h b/modules/rtp_rtcp/source/rtp_sender_egress.h index 1ef71d72a7..5b50ddfb93 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.h +++ b/modules/rtp_rtcp/source/rtp_sender_egress.h @@ -20,6 +20,7 @@ #include "api/rtc_event_log/rtc_event_log.h" #include "api/task_queue/task_queue_base.h" #include "api/units/data_rate.h" +#include "modules/remote_bitrate_estimator/test/bwe_test_logging.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtp_packet_history.h" #include "modules/rtp_rtcp/source/rtp_packet_to_send.h" @@ -110,6 +111,9 @@ class RtpSenderEgress { RtpPacketMediaType packet_type, RtpPacketCounter counter, size_t packet_size); +#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE + void BweTestLoggingPlot(int64_t now_ms, uint32_t packet_ssrc); +#endif // Called on a timer, once a second, on the worker_queue_. void PeriodicUpdate(); @@ -125,7 +129,9 @@ class RtpSenderEgress { RtpPacketHistory* const packet_history_; Transport* const transport_; RtcEventLog* const event_log_; +#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE const bool is_audio_; +#endif const bool need_rtp_packet_infos_; TransportFeedbackObserver* const transport_feedback_observer_;