diff --git a/api/transport/media/media_transport_interface.h b/api/transport/media/media_transport_interface.h index 04a8e50031..dbe68d344b 100644 --- a/api/transport/media/media_transport_interface.h +++ b/api/transport/media/media_transport_interface.h @@ -29,7 +29,6 @@ #include "api/transport/media/video_transport.h" #include "api/transport/network_control.h" #include "api/units/data_rate.h" -#include "common_types.h" // NOLINT(build/include) #include "rtc_base/copy_on_write_buffer.h" #include "rtc_base/network_route.h" @@ -238,13 +237,6 @@ class MediaTransportInterface : public DataChannelTransportInterface { // Corresponding observers for audio and video overhead. Before destruction, // the observers must be unregistered by setting nullptr. - // TODO(nisse): Should move to per-stream objects, since packetization - // overhead can vary per stream, e.g., depending on negotiated extensions. In - // addition, we should move towards reporting total overhead including all - // layers. Currently, overhead of the lower layers is reported elsewhere, - // e.g., on route change between IPv4 and IPv6. - virtual void SetAudioOverheadObserver(OverheadObserver* observer) {} - // Registers an observer for network change events. If the network route is // already established when the callback is added, |callback| will be called // immediately with the current network route. Before media transport is diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 36010d80c3..8730c45258 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -118,7 +118,6 @@ AudioSendStream::AudioSendStream( voe::CreateChannelSend(clock, task_queue_factory, module_process_thread, - /*overhead_observer=*/this, config.send_transport, rtcp_rtt_stats, event_log, @@ -343,6 +342,12 @@ void AudioSendStream::ConfigureStream( RTC_LOG(LS_ERROR) << "Failed to set up send codec state."; } + // Set currently known overhead (used in ANA, opus only). + { + rtc::CritScope cs(&overhead_per_packet_lock_); + UpdateOverheadForEncoder(); + } + channel_send_->CallEncoder([this](AudioEncoder* encoder) { if (!encoder) { return; @@ -505,10 +510,17 @@ void AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { // thread. Then this check can be enabled. // RTC_DCHECK(!worker_thread_checker_.IsCurrent()); channel_send_->ReceivedRTCPPacket(packet, length); + worker_queue_->PostTask([&]() { + // Poll if overhead has changed, which it can do if ack triggers us to stop + // sending mid/rid. + rtc::CritScope cs(&overhead_per_packet_lock_); + UpdateOverheadForEncoder(); + }); } uint32_t AudioSendStream::OnBitrateUpdated(BitrateAllocationUpdate update) { RTC_DCHECK_RUN_ON(worker_queue_); + // Pick a target bitrate between the constraints. Overrules the allocator if // it 1) allocated a bitrate of zero to disable the stream or 2) allocated a // higher than max to allow for e.g. extra FEC. @@ -531,13 +543,6 @@ void AudioSendStream::SetTransportOverhead( UpdateOverheadForEncoder(); } -void AudioSendStream::OnOverheadChanged( - size_t overhead_bytes_per_packet_bytes) { - rtc::CritScope cs(&overhead_per_packet_lock_); - audio_overhead_per_packet_bytes_ = overhead_bytes_per_packet_bytes; - UpdateOverheadForEncoder(); -} - void AudioSendStream::UpdateOverheadForEncoder() { const size_t overhead_per_packet_bytes = GetPerPacketOverheadBytes(); if (overhead_per_packet_bytes == 0) { @@ -546,7 +551,7 @@ void AudioSendStream::UpdateOverheadForEncoder() { channel_send_->CallEncoder([&](AudioEncoder* encoder) { encoder->OnReceivedOverhead(overhead_per_packet_bytes); }); - worker_queue_->PostTask([this, overhead_per_packet_bytes] { + auto update_task = [this, overhead_per_packet_bytes] { RTC_DCHECK_RUN_ON(worker_queue_); if (total_packet_overhead_bytes_ != overhead_per_packet_bytes) { total_packet_overhead_bytes_ = overhead_per_packet_bytes; @@ -554,7 +559,12 @@ void AudioSendStream::UpdateOverheadForEncoder() { ConfigureBitrateObserver(); } } - }); + }; + if (worker_queue_->IsCurrent()) { + update_task(); + } else { + worker_queue_->PostTask(update_task); + } } size_t AudioSendStream::TestOnlyGetPerPacketOverheadBytes() const { @@ -564,7 +574,7 @@ size_t AudioSendStream::TestOnlyGetPerPacketOverheadBytes() const { size_t AudioSendStream::GetPerPacketOverheadBytes() const { return transport_overhead_per_packet_bytes_ + - audio_overhead_per_packet_bytes_; + rtp_rtcp_module_->ExpectedPerPacketOverhead(); } RtpState AudioSendStream::GetRtpState() const { @@ -651,8 +661,9 @@ bool AudioSendStream::SetupSendCodec(const Config& new_config) { // If overhead changes later, it will be updated in UpdateOverheadForEncoder. { rtc::CritScope cs(&overhead_per_packet_lock_); - if (GetPerPacketOverheadBytes() > 0) { - encoder->OnReceivedOverhead(GetPerPacketOverheadBytes()); + size_t overhead = GetPerPacketOverheadBytes(); + if (overhead > 0) { + encoder->OnReceivedOverhead(overhead); } } @@ -704,12 +715,6 @@ bool AudioSendStream::ReconfigureSendCodec(const Config& new_config) { ReconfigureANA(new_config); ReconfigureCNG(new_config); - // Set currently known overhead (used in ANA, opus only). - { - rtc::CritScope cs(&overhead_per_packet_lock_); - UpdateOverheadForEncoder(); - } - return true; } @@ -836,9 +841,9 @@ void AudioSendStream::ConfigureBitrateObserver() { priority_bitrate += max_overhead; } else { RTC_DCHECK(frame_length_range_); - const DataSize kOverheadPerPacket = + const DataSize overhead_per_packet = DataSize::Bytes(total_packet_overhead_bytes_); - DataRate min_overhead = kOverheadPerPacket / frame_length_range_->second; + DataRate min_overhead = overhead_per_packet / frame_length_range_->second; priority_bitrate += min_overhead; } } diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index 909d4e9fcc..92e9a7fb16 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -52,8 +52,7 @@ namespace internal { class AudioState; class AudioSendStream final : public webrtc::AudioSendStream, - public webrtc::BitrateAllocatorObserver, - public webrtc::OverheadObserver { + public webrtc::BitrateAllocatorObserver { public: AudioSendStream(Clock* clock, const webrtc::AudioSendStream::Config& config, @@ -99,10 +98,6 @@ class AudioSendStream final : public webrtc::AudioSendStream, void SetTransportOverhead(int transport_overhead_per_packet_bytes); - // OverheadObserver override reports audio packetization overhead from - // RTP/RTCP module or Media Transport. - void OnOverheadChanged(size_t overhead_bytes_per_packet_bytes) override; - RtpState GetRtpState() const; const voe::ChannelSendInterface* GetChannel() const; @@ -205,10 +200,6 @@ class AudioSendStream final : public webrtc::AudioSendStream, size_t transport_overhead_per_packet_bytes_ RTC_GUARDED_BY(overhead_per_packet_lock_) = 0; - // Current audio packetization overhead (RTP or Media Transport). - size_t audio_overhead_per_packet_bytes_ - RTC_GUARDED_BY(overhead_per_packet_lock_) = 0; - bool registered_with_allocator_ RTC_GUARDED_BY(worker_queue_) = false; size_t total_packet_overhead_bytes_ RTC_GUARDED_BY(worker_queue_) = 0; absl::optional> frame_length_range_ diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index ea753f7d0b..334fdf50f7 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -660,9 +660,9 @@ TEST(AudioSendStreamTest, SSBweWithOverhead) { "WebRTC-Audio-LegacyOverhead/Disabled/"); for (bool use_null_audio_processing : {false, true}) { ConfigHelper helper(true, true, use_null_audio_processing); + EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead) + .WillRepeatedly(Return(kOverheadPerPacket.bytes())); auto send_stream = helper.CreateAudioSendStream(); - EXPECT_CALL(*helper.channel_send(), CallEncoder(_)).Times(1); - send_stream->OnOverheadChanged(kOverheadPerPacket.bytes()); const DataRate bitrate = DataRate::BitsPerSec(helper.config().max_bitrate_bps) + kMaxOverheadRate; @@ -684,9 +684,9 @@ TEST(AudioSendStreamTest, SSBweWithOverheadMinRespected) { "WebRTC-Audio-Allocation/min:6kbps,max:64kbps/"); for (bool use_null_audio_processing : {false, true}) { ConfigHelper helper(true, true, use_null_audio_processing); + EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead) + .WillRepeatedly(Return(kOverheadPerPacket.bytes())); auto send_stream = helper.CreateAudioSendStream(); - EXPECT_CALL(*helper.channel_send(), CallEncoder(_)).Times(1); - send_stream->OnOverheadChanged(kOverheadPerPacket.bytes()); const DataRate bitrate = DataRate::KilobitsPerSec(6) + kMinOverheadRate; EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation(Field( @@ -706,9 +706,9 @@ TEST(AudioSendStreamTest, SSBweWithOverheadMaxRespected) { "WebRTC-Audio-Allocation/min:6kbps,max:64kbps/"); for (bool use_null_audio_processing : {false, true}) { ConfigHelper helper(true, true, use_null_audio_processing); + EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead) + .WillRepeatedly(Return(kOverheadPerPacket.bytes())); auto send_stream = helper.CreateAudioSendStream(); - EXPECT_CALL(*helper.channel_send(), CallEncoder(_)).Times(1); - send_stream->OnOverheadChanged(kOverheadPerPacket.bytes()); const DataRate bitrate = DataRate::KilobitsPerSec(64) + kMaxOverheadRate; EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation(Field( @@ -804,36 +804,56 @@ TEST(AudioSendStreamTest, OnTransportOverheadChanged) { } } -TEST(AudioSendStreamTest, OnAudioOverheadChanged) { +TEST(AudioSendStreamTest, AudioOverheadChanged) { for (bool use_null_audio_processing : {false, true}) { ConfigHelper helper(false, true, use_null_audio_processing); + const size_t audio_overhead_per_packet_bytes = 555; + EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead) + .WillRepeatedly(Return(audio_overhead_per_packet_bytes)); auto send_stream = helper.CreateAudioSendStream(); auto new_config = helper.config(); - // CallEncoder will be called on overhead change. - EXPECT_CALL(*helper.channel_send(), CallEncoder(::testing::_)).Times(1); + BitrateAllocationUpdate update; + update.target_bitrate = + DataRate::BitsPerSec(helper.config().max_bitrate_bps) + + kMaxOverheadRate; + EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }, + RTC_FROM_HERE); - const size_t audio_overhead_per_packet_bytes = 555; - send_stream->OnOverheadChanged(audio_overhead_per_packet_bytes); EXPECT_EQ(audio_overhead_per_packet_bytes, send_stream->TestOnlyGetPerPacketOverheadBytes()); + + EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead) + .WillRepeatedly(Return(audio_overhead_per_packet_bytes + 20)); + EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }, + RTC_FROM_HERE); + + EXPECT_EQ(audio_overhead_per_packet_bytes + 20, + send_stream->TestOnlyGetPerPacketOverheadBytes()); } } TEST(AudioSendStreamTest, OnAudioAndTransportOverheadChanged) { for (bool use_null_audio_processing : {false, true}) { ConfigHelper helper(false, true, use_null_audio_processing); + const size_t audio_overhead_per_packet_bytes = 555; + EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead) + .WillRepeatedly(Return(audio_overhead_per_packet_bytes)); auto send_stream = helper.CreateAudioSendStream(); auto new_config = helper.config(); - // CallEncoder will be called when each of overhead changes. - EXPECT_CALL(*helper.channel_send(), CallEncoder(::testing::_)).Times(2); - const size_t transport_overhead_per_packet_bytes = 333; send_stream->SetTransportOverhead(transport_overhead_per_packet_bytes); - const size_t audio_overhead_per_packet_bytes = 555; - send_stream->OnOverheadChanged(audio_overhead_per_packet_bytes); + BitrateAllocationUpdate update; + update.target_bitrate = + DataRate::BitsPerSec(helper.config().max_bitrate_bps) + + kMaxOverheadRate; + EXPECT_CALL(*helper.channel_send(), OnBitrateAllocation); + helper.worker()->SendTask([&] { send_stream->OnBitrateUpdated(update); }, + RTC_FROM_HERE); EXPECT_EQ( transport_overhead_per_packet_bytes + audio_overhead_per_packet_bytes, diff --git a/audio/channel_send.cc b/audio/channel_send.cc index d2d4d2be6d..3387f271ba 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -69,7 +69,6 @@ class ChannelSend : public ChannelSendInterface, ChannelSend(Clock* clock, TaskQueueFactory* task_queue_factory, ProcessThread* module_process_thread, - OverheadObserver* overhead_observer, Transport* rtp_transport, RtcpRttStats* rtcp_rtt_stats, RtcEventLog* rtc_event_log, @@ -482,7 +481,6 @@ ChannelSend::ChannelSend( Clock* clock, TaskQueueFactory* task_queue_factory, ProcessThread* module_process_thread, - OverheadObserver* overhead_observer, Transport* rtp_transport, RtcpRttStats* rtcp_rtt_stats, RtcEventLog* rtc_event_log, @@ -515,7 +513,6 @@ ChannelSend::ChannelSend( audio_coding_.reset(AudioCodingModule::Create(AudioCodingModule::Config())); RtpRtcp::Configuration configuration; - configuration.overhead_observer = overhead_observer; configuration.bandwidth_callback = rtcp_observer_.get(); configuration.transport_feedback_callback = feedback_observer_proxy_.get(); configuration.clock = (clock ? clock : Clock::GetRealTimeClock()); @@ -980,7 +977,6 @@ std::unique_ptr CreateChannelSend( Clock* clock, TaskQueueFactory* task_queue_factory, ProcessThread* module_process_thread, - OverheadObserver* overhead_observer, Transport* rtp_transport, RtcpRttStats* rtcp_rtt_stats, RtcEventLog* rtc_event_log, @@ -991,9 +987,9 @@ std::unique_ptr CreateChannelSend( uint32_t ssrc, rtc::scoped_refptr frame_transformer) { return std::make_unique( - clock, task_queue_factory, module_process_thread, overhead_observer, - rtp_transport, rtcp_rtt_stats, rtc_event_log, frame_encryptor, - crypto_options, extmap_allow_mixed, rtcp_report_interval_ms, ssrc, + clock, task_queue_factory, module_process_thread, rtp_transport, + rtcp_rtt_stats, rtc_event_log, frame_encryptor, crypto_options, + extmap_allow_mixed, rtcp_report_interval_ms, ssrc, std::move(frame_transformer)); } diff --git a/audio/channel_send.h b/audio/channel_send.h index 94c554015e..cb3b99287b 100644 --- a/audio/channel_send.h +++ b/audio/channel_send.h @@ -128,7 +128,6 @@ std::unique_ptr CreateChannelSend( Clock* clock, TaskQueueFactory* task_queue_factory, ProcessThread* module_process_thread, - OverheadObserver* overhead_observer, Transport* rtp_transport, RtcpRttStats* rtcp_rtt_stats, RtcEventLog* rtc_event_log, diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 31c73856d6..ffe2d61b39 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -193,7 +193,6 @@ std::vector CreateRtpStreamSenders( const std::map& suspended_ssrcs, RtcEventLog* event_log, RateLimiter* retransmission_rate_limiter, - OverheadObserver* overhead_observer, FrameEncryptorInterface* frame_encryptor, const CryptoOptions& crypto_options, rtc::scoped_refptr frame_transformer, @@ -225,7 +224,6 @@ std::vector CreateRtpStreamSenders( configuration.send_packet_observer = observers.send_packet_observer; configuration.event_log = event_log; configuration.retransmission_rate_limiter = retransmission_rate_limiter; - configuration.overhead_observer = overhead_observer; configuration.rtp_stats_callback = observers.rtp_stats; configuration.frame_encryptor = frame_encryptor; configuration.require_frame_encryption = @@ -352,7 +350,6 @@ RtpVideoSender::RtpVideoSender( suspended_ssrcs_, event_log, retransmission_limiter, - this, frame_encryptor, crypto_options, std::move(frame_transformer), @@ -361,7 +358,6 @@ RtpVideoSender::RtpVideoSender( codec_type_(GetVideoCodecType(rtp_config)), transport_(transport), transport_overhead_bytes_per_packet_(0), - overhead_bytes_per_packet_(0), encoder_target_rate_bps_(0), frame_counts_(rtp_config.ssrcs.size()), frame_count_observer_(observers.frame_count_observer) { @@ -733,17 +729,24 @@ void RtpVideoSender::OnTransportOverheadChanged( } } -void RtpVideoSender::OnOverheadChanged(size_t overhead_bytes_per_packet) { - rtc::CritScope lock(&crit_); - overhead_bytes_per_packet_ = overhead_bytes_per_packet; -} - void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, int framerate) { // Substract overhead from bitrate. rtc::CritScope lock(&crit_); + size_t num_active_streams = 0; + size_t overhead_bytes_per_packet = 0; + for (const auto& stream : rtp_streams_) { + if (stream.rtp_rtcp->SendingMedia()) { + overhead_bytes_per_packet += stream.rtp_rtcp->ExpectedPerPacketOverhead(); + ++num_active_streams; + } + } + if (num_active_streams > 1) { + overhead_bytes_per_packet /= num_active_streams; + } + DataSize packet_overhead = DataSize::Bytes( - overhead_bytes_per_packet_ + transport_overhead_bytes_per_packet_); + overhead_bytes_per_packet + transport_overhead_bytes_per_packet_); DataSize max_total_packet_size = DataSize::Bytes( rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_); uint32_t payload_bitrate_bps = update.target_bitrate.bps(); @@ -790,7 +793,7 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, // calculations. DataRate encoder_overhead_rate = CalculateOverheadRate( DataRate::BitsPerSec(encoder_target_rate_bps_), - max_total_packet_size - DataSize::Bytes(overhead_bytes_per_packet_), + max_total_packet_size - DataSize::Bytes(overhead_bytes_per_packet), packet_overhead); encoder_overhead_rate_bps = std::min( encoder_overhead_rate.bps(), diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index d2a20a95c9..f7d8c763d2 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -70,7 +70,6 @@ struct RtpStreamSender { // RtpVideoSender routes outgoing data to the correct sending RTP module, based // on the simulcast layer in RTPVideoHeader. class RtpVideoSender : public RtpVideoSenderInterface, - public OverheadObserver, public VCMProtectionCallback, public StreamFeedbackObserver { public: @@ -136,8 +135,6 @@ class RtpVideoSender : public RtpVideoSenderInterface, void OnTransportOverheadChanged( size_t transport_overhead_bytes_per_packet) override; - // Implements OverheadObserver. - void OnOverheadChanged(size_t overhead_bytes_per_packet) override; void OnBitrateUpdated(BitrateAllocationUpdate update, int framerate) override; uint32_t GetPayloadBitrateBps() const override; uint32_t GetProtectionBitrateBps() const override; @@ -194,7 +191,6 @@ class RtpVideoSender : public RtpVideoSenderInterface, std::vector params_ RTC_GUARDED_BY(crit_); size_t transport_overhead_bytes_per_packet_ RTC_GUARDED_BY(crit_); - size_t overhead_bytes_per_packet_ RTC_GUARDED_BY(crit_); uint32_t protection_bitrate_bps_; uint32_t encoder_target_rate_bps_; diff --git a/common_types.h b/common_types.h index dedcbd5460..cd63f5f72b 100644 --- a/common_types.h +++ b/common_types.h @@ -31,14 +31,6 @@ class FrameCountObserver { uint32_t ssrc) = 0; }; -// Callback, used to notify an observer when the overhead per packet -// has changed. -class OverheadObserver { - public: - virtual ~OverheadObserver() = default; - virtual void OnOverheadChanged(size_t overhead_bytes_per_packet) = 0; -}; - // ================================================================== // Video specific types // ================================================================== diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 967ba663dc..598c09e0d4 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -38,7 +38,6 @@ namespace webrtc { // Forward declarations. class FrameEncryptorInterface; -class OverheadObserver; class RateLimiter; class ReceiveStatisticsProvider; class RemoteBitrateEstimator; @@ -113,7 +112,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { RtcEventLog* event_log = nullptr; SendPacketObserver* send_packet_observer = nullptr; RateLimiter* retransmission_rate_limiter = nullptr; - OverheadObserver* overhead_observer = nullptr; StreamDataCountersCallback* rtp_stats_callback = nullptr; int rtcp_report_interval_ms = 0; @@ -318,6 +316,13 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { virtual std::vector GetSentRtpPacketInfos( rtc::ArrayView sequence_numbers) const = 0; + // Returns an expected per packet overhead representing the main RTP header, + // any CSRCs, and the registered header extensions that are expected on all + // packets (i.e. disregarding things like abs capture time which is only + // populated on a subset of packets, but counting MID/RID type extensions + // when we expect to send them). + virtual size_t ExpectedPerPacketOverhead() const = 0; + // ************************************************************************** // RTCP // ************************************************************************** diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 1927e4af4a..27c2613661 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -97,6 +97,7 @@ class MockRtpRtcp : public RtpRtcp { MOCK_CONST_METHOD1(GetSentRtpPacketInfos, std::vector( rtc::ArrayView sequence_numbers)); + MOCK_CONST_METHOD0(ExpectedPerPacketOverhead, size_t(void)); MOCK_METHOD2(RegisterRtcpObservers, void(RtcpIntraFrameObserver* intra_frame_callback, RtcpBandwidthObserver* bandwidth_callback)); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 25fa545213..1cb61f5a61 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -401,6 +401,13 @@ ModuleRtpRtcpImpl::GetSentRtpPacketInfos( return rtp_sender_->packet_sender.GetSentRtpPacketInfos(sequence_numbers); } +size_t ModuleRtpRtcpImpl::ExpectedPerPacketOverhead() const { + if (!rtp_sender_) { + return 0; + } + return rtp_sender_->packet_generator.ExpectedPerPacketOverhead(); +} + size_t ModuleRtpRtcpImpl::MaxRtpPacketSize() const { RTC_DCHECK(rtp_sender_); return rtp_sender_->packet_generator.MaxRtpPacketSize(); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 17875e803f..8bda0e0f0c 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -146,6 +146,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { std::vector GetSentRtpPacketInfos( rtc::ArrayView sequence_numbers) const override; + size_t ExpectedPerPacketOverhead() const override; + // RTCP part. // Get RTCP status. diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 36a87c05ed..3d60552e9b 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -84,6 +84,52 @@ constexpr RtpExtensionSize kVideoExtensionSizes[] = { RtpGenericFrameDescriptorExtension00::kMaxSizeBytes}, }; +// Size info for header extensions that might be used in audio packets. +constexpr RtpExtensionSize kAudioExtensionSizes[] = { + CreateExtensionSize(), + CreateExtensionSize(), + CreateExtensionSize(), + CreateExtensionSize(), + CreateExtensionSize(), + CreateExtensionSize(), + CreateMaxExtensionSize(), + CreateMaxExtensionSize(), + CreateMaxExtensionSize(), +}; + +// Non-volatile extensions can be expected on all packets, if registered. +// Volatile ones, such as VideoContentTypeExtension which is only set on +// key-frames, are removed to simplify overhead calculations at the expense of +// some accuracy. +bool IsNonVolatile(RTPExtensionType type) { + switch (type) { + case kRtpExtensionTransmissionTimeOffset: + case kRtpExtensionAudioLevel: + case kRtpExtensionAbsoluteSendTime: + case kRtpExtensionTransportSequenceNumber: + case kRtpExtensionTransportSequenceNumber02: + case kRtpExtensionFrameMarking: + case kRtpExtensionRtpStreamId: + case kRtpExtensionRepairedRtpStreamId: + case kRtpExtensionMid: + case kRtpExtensionGenericFrameDescriptor00: + case kRtpExtensionGenericFrameDescriptor02: + return true; + case kRtpExtensionInbandComfortNoise: + case kRtpExtensionAbsoluteCaptureTime: + case kRtpExtensionVideoRotation: + case kRtpExtensionPlayoutDelay: + case kRtpExtensionVideoContentType: + case kRtpExtensionVideoTiming: + case kRtpExtensionColorSpace: + return false; + case kRtpExtensionNone: + case kRtpExtensionNumberOfExtensions: + RTC_NOTREACHED(); + return false; + } +} + bool HasBweExtension(const RtpHeaderExtensionMap& extensions_map) { return extensions_map.IsRegistered(kRtpExtensionTransportSequenceNumber) || extensions_map.IsRegistered(kRtpExtensionTransportSequenceNumber02) || @@ -125,6 +171,8 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config, max_packet_size_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. last_payload_type_(-1), rtp_header_extension_map_(config.extmap_allow_mixed), + max_media_packet_header_(kRtpHeaderSize), + max_padding_fec_packet_header_(kRtpHeaderSize), // RTP variables sequence_number_forced_(false), always_send_mid_and_rid_(config.always_send_mid_and_rid), @@ -170,6 +218,11 @@ rtc::ArrayView RTPSender::VideoExtensionSizes() { arraysize(kVideoExtensionSizes)); } +rtc::ArrayView RTPSender::AudioExtensionSizes() { + return rtc::MakeArrayView(kAudioExtensionSizes, + arraysize(kAudioExtensionSizes)); +} + void RTPSender::SetExtmapAllowMixed(bool extmap_allow_mixed) { rtc::CritScope lock(&send_critsect_); rtp_header_extension_map_.SetExtmapAllowMixed(extmap_allow_mixed); @@ -180,6 +233,7 @@ int32_t RTPSender::RegisterRtpHeaderExtension(RTPExtensionType type, rtc::CritScope lock(&send_critsect_); bool registered = rtp_header_extension_map_.RegisterByType(id, type); supports_bwe_extension_ = HasBweExtension(rtp_header_extension_map_); + UpdateHeaderSizes(); return registered ? 0 : -1; } @@ -187,6 +241,7 @@ bool RTPSender::RegisterRtpHeaderExtension(absl::string_view uri, int id) { rtc::CritScope lock(&send_critsect_); bool registered = rtp_header_extension_map_.RegisterByUri(id, uri); supports_bwe_extension_ = HasBweExtension(rtp_header_extension_map_); + UpdateHeaderSizes(); return registered; } @@ -197,15 +252,17 @@ bool RTPSender::IsRtpHeaderExtensionRegistered(RTPExtensionType type) const { int32_t RTPSender::DeregisterRtpHeaderExtension(RTPExtensionType type) { rtc::CritScope lock(&send_critsect_); - int32_t deregistered = rtp_header_extension_map_.Deregister(type); + rtp_header_extension_map_.Deregister(type); supports_bwe_extension_ = HasBweExtension(rtp_header_extension_map_); - return deregistered; + UpdateHeaderSizes(); + return 0; } void RTPSender::DeregisterRtpHeaderExtension(absl::string_view uri) { rtc::CritScope lock(&send_critsect_); rtp_header_extension_map_.Deregister(uri); supports_bwe_extension_ = HasBweExtension(rtp_header_extension_map_); + UpdateHeaderSizes(); } void RTPSender::SetMaxRtpPacketSize(size_t max_packet_size) { @@ -291,7 +348,11 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id) { void RTPSender::OnReceivedAckOnSsrc(int64_t extended_highest_sequence_number) { rtc::CritScope lock(&send_critsect_); + bool update_required = !ssrc_has_acked_; ssrc_has_acked_ = true; + if (update_required) { + UpdateHeaderSizes(); + } } void RTPSender::OnReceivedAckOnRtxSsrc( @@ -369,7 +430,8 @@ std::vector> RTPSender::GeneratePadding( } size_t padding_bytes_in_packet; - const size_t max_payload_size = max_packet_size_ - RtpHeaderLength(); + const size_t max_payload_size = + max_packet_size_ - FecOrPaddingPacketMaxRtpHeaderLength(); if (audio_configured_) { // Allow smaller padding packets for audio. padding_bytes_in_packet = rtc::SafeClamp( @@ -484,13 +546,14 @@ void RTPSender::EnqueuePackets( paced_sender_->EnqueuePackets(std::move(packets)); } -size_t RTPSender::RtpHeaderLength() const { +size_t RTPSender::FecOrPaddingPacketMaxRtpHeaderLength() const { rtc::CritScope lock(&send_critsect_); - size_t rtp_header_length = kRtpHeaderLength; - rtp_header_length += sizeof(uint32_t) * csrcs_.size(); - rtp_header_length += RtpHeaderExtensionSize(kFecOrPaddingExtensionSizes, - rtp_header_extension_map_); - return rtp_header_length; + return max_padding_fec_packet_header_; +} + +size_t RTPSender::ExpectedPerPacketOverhead() const { + rtc::CritScope lock(&send_critsect_); + return max_media_packet_header_; } uint16_t RTPSender::AllocateSequenceNumber(uint16_t packets_to_send) { @@ -590,6 +653,7 @@ void RTPSender::SetRid(const std::string& rid) { rtc::CritScope lock(&send_critsect_); RTC_DCHECK_LE(rid.length(), RtpStreamId::kMaxValueSizeBytes); rid_ = rid; + UpdateHeaderSizes(); } void RTPSender::SetMid(const std::string& mid) { @@ -597,12 +661,14 @@ void RTPSender::SetMid(const std::string& mid) { rtc::CritScope lock(&send_critsect_); RTC_DCHECK_LE(mid.length(), RtpMid::kMaxValueSizeBytes); mid_ = mid; + UpdateHeaderSizes(); } void RTPSender::SetCsrcs(const std::vector& csrcs) { RTC_DCHECK_LE(csrcs.size(), kRtpCsrcSize); rtc::CritScope lock(&send_critsect_); csrcs_ = csrcs; + UpdateHeaderSizes(); } void RTPSender::SetSequenceNumber(uint16_t seq) { @@ -756,6 +822,7 @@ void RTPSender::SetRtpState(const RtpState& rtp_state) { capture_time_ms_ = rtp_state.capture_time_ms; last_timestamp_time_ms_ = rtp_state.last_timestamp_time_ms; ssrc_has_acked_ = rtp_state.ssrc_has_acked; + UpdateHeaderSizes(); } RtpState RTPSender::GetRtpState() const { @@ -792,4 +859,42 @@ int64_t RTPSender::LastTimestampTimeMs() const { rtc::CritScope lock(&send_critsect_); return last_timestamp_time_ms_; } + +void RTPSender::UpdateHeaderSizes() { + const size_t rtp_header_length = + kRtpHeaderLength + sizeof(uint32_t) * csrcs_.size(); + + max_padding_fec_packet_header_ = + rtp_header_length + RtpHeaderExtensionSize(kFecOrPaddingExtensionSizes, + rtp_header_extension_map_); + + // RtpStreamId and Mid are treated specially in that we check if they + // currently are being sent. RepairedRtpStreamId is still ignored since we + // assume RTX will not make up large enough bitrate to treat overhead + // differently. + const bool send_mid_rid = always_send_mid_and_rid_ || !ssrc_has_acked_; + std::vector non_volatile_extensions; + for (auto& extension : + audio_configured_ ? AudioExtensionSizes() : VideoExtensionSizes()) { + if (IsNonVolatile(extension.type)) { + switch (extension.type) { + case RTPExtensionType::kRtpExtensionMid: + if (send_mid_rid && !mid_.empty()) { + non_volatile_extensions.push_back(extension); + } + break; + case RTPExtensionType::kRtpExtensionRtpStreamId: + if (send_mid_rid && !rid_.empty()) { + non_volatile_extensions.push_back(extension); + } + break; + default: + non_volatile_extensions.push_back(extension); + } + } + } + max_media_packet_header_ = + rtp_header_length + RtpHeaderExtensionSize(non_volatile_extensions, + rtp_header_extension_map_); +} } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 7fe4bfdb81..a14c3ae1a8 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -39,7 +39,6 @@ namespace webrtc { class FrameEncryptorInterface; -class OverheadObserver; class RateLimiter; class RtcEventLog; class RtpPacketToSend; @@ -109,6 +108,9 @@ class RTPSender { // Size info for header extensions used by video packets. static rtc::ArrayView VideoExtensionSizes(); + // Size info for header extensions used by audio packets. + static rtc::ArrayView AudioExtensionSizes(); + // Create empty packet, fills ssrc, csrcs and reserve place for header // extensions RtpSender updates before sending. std::unique_ptr AllocatePacket() const; @@ -116,9 +118,10 @@ class RTPSender { // Save packet's fields to generate padding that doesn't break media stream. // Return false if sending was turned off. bool AssignSequenceNumber(RtpPacketToSend* packet); - - // Used for padding and FEC packets only. - size_t RtpHeaderLength() const; + // Maximum header overhead per fec/padding packet. + size_t FecOrPaddingPacketMaxRtpHeaderLength() const; + // Expected header overhead per media packet. + size_t ExpectedPerPacketOverhead() const; uint16_t AllocateSequenceNumber(uint16_t packets_to_send); // Including RTP headers. size_t MaxRtpPacketSize() const; @@ -148,6 +151,8 @@ class RTPSender { bool IsFecPacket(const RtpPacketToSend& packet) const; + void UpdateHeaderSizes() RTC_EXCLUSIVE_LOCKS_REQUIRED(send_critsect_); + Clock* const clock_; Random random_ RTC_GUARDED_BY(send_critsect_); @@ -172,6 +177,8 @@ class RTPSender { RtpHeaderExtensionMap rtp_header_extension_map_ RTC_GUARDED_BY(send_critsect_); + size_t max_media_packet_header_ RTC_GUARDED_BY(send_critsect_); + size_t max_padding_fec_packet_header_ RTC_GUARDED_BY(send_critsect_); // RTP variables uint32_t timestamp_offset_ RTC_GUARDED_BY(send_critsect_); diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index f421f83f20..a64a5bddb6 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -71,7 +71,6 @@ RtpSenderEgress::RtpSenderEgress(const RtpRtcp::Configuration& config, transport_feedback_observer_(config.transport_feedback_callback), send_side_delay_observer_(config.send_side_delay_observer), send_packet_observer_(config.send_packet_observer), - overhead_observer_(config.overhead_observer), rtp_stats_callback_(config.rtp_stats_callback), bitrate_callback_(config.send_bitrate_observer), media_has_been_sent_(false), @@ -80,7 +79,6 @@ RtpSenderEgress::RtpSenderEgress(const RtpRtcp::Configuration& config, max_delay_it_(send_delays_.end()), sum_delays_ms_(0), total_packet_send_delay_ms_(0), - rtp_overhead_bytes_per_packet_(0), total_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale), nack_bitrate_sent_(kBitrateStatisticsWindowMs, RateStatistics::kBpsScale), @@ -412,7 +410,6 @@ bool RtpSenderEgress::SendPacketToNetwork(const RtpPacketToSend& packet, const PacedPacketInfo& pacing_info) { int bytes_sent = -1; if (transport_) { - UpdateRtpOverhead(packet); bytes_sent = transport_->SendRtp(packet.data(), packet.size(), options) ? static_cast(packet.size()) : -1; @@ -429,21 +426,6 @@ bool RtpSenderEgress::SendPacketToNetwork(const RtpPacketToSend& packet, return true; } -void RtpSenderEgress::UpdateRtpOverhead(const RtpPacketToSend& packet) { - if (!overhead_observer_) - return; - size_t overhead_bytes_per_packet; - { - rtc::CritScope lock(&lock_); - if (rtp_overhead_bytes_per_packet_ == packet.headers_size()) { - return; - } - rtp_overhead_bytes_per_packet_ = packet.headers_size(); - overhead_bytes_per_packet = rtp_overhead_bytes_per_packet_; - } - overhead_observer_->OnOverheadChanged(overhead_bytes_per_packet); -} - void RtpSenderEgress::UpdateRtpStats(const RtpPacketToSend& packet) { int64_t now_ms = clock_->TimeInMilliseconds(); diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.h b/modules/rtp_rtcp/source/rtp_sender_egress.h index 3d4999f964..131534039e 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.h +++ b/modules/rtp_rtcp/source/rtp_sender_egress.h @@ -96,7 +96,6 @@ class RtpSenderEgress { bool SendPacketToNetwork(const RtpPacketToSend& packet, const PacketOptions& options, const PacedPacketInfo& pacing_info); - void UpdateRtpOverhead(const RtpPacketToSend& packet); void UpdateRtpStats(const RtpPacketToSend& packet) RTC_EXCLUSIVE_LOCKS_REQUIRED(lock_); @@ -115,7 +114,6 @@ class RtpSenderEgress { TransportFeedbackObserver* const transport_feedback_observer_; SendSideDelayObserver* const send_side_delay_observer_; SendPacketObserver* const send_packet_observer_; - OverheadObserver* const overhead_observer_; StreamDataCountersCallback* const rtp_stats_callback_; BitrateStatisticsObserver* const bitrate_callback_; @@ -129,7 +127,6 @@ class RtpSenderEgress { // The sum of delays over a kSendSideDelayWindowMs sliding window. int64_t sum_delays_ms_ RTC_GUARDED_BY(lock_); uint64_t total_packet_send_delay_ms_ RTC_GUARDED_BY(lock_); - size_t rtp_overhead_bytes_per_packet_ RTC_GUARDED_BY(lock_); StreamDataCounters rtp_stats_ RTC_GUARDED_BY(lock_); StreamDataCounters rtx_rtp_stats_ RTC_GUARDED_BY(lock_); RateStatistics total_bitrate_sent_ RTC_GUARDED_BY(lock_); diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index eb1a48ba86..474810a88a 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -178,11 +178,6 @@ class MockTransportFeedbackObserver : public TransportFeedbackObserver { MOCK_METHOD1(OnTransportFeedback, void(const rtcp::TransportFeedback&)); }; -class MockOverheadObserver : public OverheadObserver { - public: - MOCK_METHOD1(OnOverheadChanged, void(size_t overhead_bytes_per_packet)); -}; - class StreamDataTestCallback : public StreamDataCountersCallback { public: StreamDataTestCallback() @@ -535,8 +530,7 @@ TEST_P(RtpSenderTestWithoutPacer, AssignSequenceNumberSetPaddingTimestamps) { TEST_P(RtpSenderTestWithoutPacer, TransportFeedbackObserverGetsCorrectByteCount) { - constexpr int kRtpOverheadBytesPerPacket = 12 + 8; - NiceMock mock_overhead_observer; + constexpr size_t kRtpOverheadBytesPerPacket = 12 + 8; RtpRtcp::Configuration config; config.clock = &fake_clock_; @@ -545,7 +539,6 @@ TEST_P(RtpSenderTestWithoutPacer, config.transport_feedback_callback = &feedback_observer_; config.event_log = &mock_rtc_event_log_; config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.overhead_observer = &mock_overhead_observer; config.field_trials = &field_trials_; rtp_sender_context_ = std::make_unique(config); @@ -568,9 +561,8 @@ TEST_P(RtpSenderTestWithoutPacer, Field(&RtpPacketSendInfo::length, expected_bytes), Field(&RtpPacketSendInfo::pacing_info, PacedPacketInfo())))) .Times(1); - EXPECT_CALL(mock_overhead_observer, - OnOverheadChanged(kRtpOverheadBytesPerPacket)) - .Times(1); + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), + kRtpOverheadBytesPerPacket); SendGenericPacket(); } @@ -1978,42 +1970,66 @@ TEST_P(RtpSenderTestWithoutPacer, RespectsNackBitrateLimit) { EXPECT_EQ(kNumPackets * 2, transport_.packets_sent()); } -TEST_P(RtpSenderTest, OnOverheadChanged) { - MockOverheadObserver mock_overhead_observer; +TEST_P(RtpSenderTest, UpdatingCsrcsUpdatedOverhead) { RtpRtcp::Configuration config; config.clock = &fake_clock_; config.outgoing_transport = &transport_; config.local_media_ssrc = kSsrc; config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.overhead_observer = &mock_overhead_observer; rtp_sender_context_ = std::make_unique(config); - // RTP overhead is 12B. - EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(12)).Times(1); - SendGenericPacket(); + // Base RTP overhead is 12B. + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 12u); + + // Adding two csrcs adds 2*4 bytes to the header. + rtp_sender()->SetCsrcs({1, 2}); + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 20u); +} + +TEST_P(RtpSenderTest, OnOverheadChanged) { + RtpRtcp::Configuration config; + config.clock = &fake_clock_; + config.outgoing_transport = &transport_; + config.local_media_ssrc = kSsrc; + config.retransmission_rate_limiter = &retransmission_rate_limiter_; + rtp_sender_context_ = std::make_unique(config); + + // Base RTP overhead is 12B. + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 12u); rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionTransmissionTimeOffset, kTransmissionTimeOffsetExtensionId); - // TransmissionTimeOffset extension has a size of 8B. - // 12B + 8B = 20B - EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(20)).Times(1); - SendGenericPacket(); + // TransmissionTimeOffset extension has a size of 3B, but with the addition + // of header index and rounding to 4 byte boundary we end up with 20B total. + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 20u); } -TEST_P(RtpSenderTest, DoesNotUpdateOverheadOnEqualSize) { - MockOverheadObserver mock_overhead_observer; +TEST_P(RtpSenderTest, CountMidOnlyUntilAcked) { RtpRtcp::Configuration config; config.clock = &fake_clock_; config.outgoing_transport = &transport_; config.local_media_ssrc = kSsrc; config.retransmission_rate_limiter = &retransmission_rate_limiter_; - config.overhead_observer = &mock_overhead_observer; rtp_sender_context_ = std::make_unique(config); - EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(_)).Times(1); - SendGenericPacket(); - SendGenericPacket(); + // Base RTP overhead is 12B. + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 12u); + + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionMid, kMidExtensionId); + rtp_sender()->RegisterRtpHeaderExtension(kRtpExtensionRtpStreamId, + kRidExtensionId); + + // Counted only if set. + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 12u); + rtp_sender()->SetMid("foo"); + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 36u); + rtp_sender()->SetRid("bar"); + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 52u); + + // Ack received, mid/rid no longer sent. + rtp_sender()->OnReceivedAckOnSsrc(0); + EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), 12u); } TEST_P(RtpSenderTest, SendPacketMatchesVideo) { diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 631feb343b..b903b9f001 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -206,7 +206,8 @@ size_t RTPSenderVideo::FecPacketOverhead() const { // This reason for the header extensions to be included here is that // from an FEC viewpoint, they are part of the payload to be protected. // (The base RTP header is already protected by the FEC header.) - overhead += rtp_sender_->RtpHeaderLength() - kRtpHeaderSize; + overhead += + rtp_sender_->FecOrPaddingPacketMaxRtpHeaderLength() - kRtpHeaderSize; } } return overhead;