From 89ca2991617c95de65bdd71f3a2e99c6cf15669b Mon Sep 17 00:00:00 2001 From: Per K Date: Tue, 10 Jan 2023 14:28:25 +0100 Subject: [PATCH] Use parsed packet from RtpTransport::DemuxPacket in engine and call With this cl, a packet is only parsed once in RtpTransport::DemuxPacket and the metadata is reused. Extensions are still identified twice- one for demuxing based on mid. The second time in Channel::OnReceivedPacket in order to use extensions specific to that mid. Bug: webrtc:7135, webrtc:14795 Change-Id: I50e3814af92ca4378f148876b20a54bcfac1e146 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290540 Reviewed-by: Danil Chapovalov Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#39058} --- call/packet_receiver.h | 6 +- media/BUILD.gn | 1 + media/base/fake_media_engine.h | 7 +- media/base/fake_network_interface.h | 11 +- media/base/media_channel.h | 4 +- media/base/media_channel_impl.h | 24 +-- media/engine/fake_webrtc_call.cc | 1 + media/engine/fake_webrtc_call.h | 4 + media/engine/webrtc_video_engine.cc | 188 ++++++++++--------- media/engine/webrtc_video_engine.h | 11 +- media/engine/webrtc_video_engine_unittest.cc | 34 +++- media/engine/webrtc_voice_engine.cc | 139 ++++++++------ media/engine/webrtc_voice_engine.h | 9 +- media/engine/webrtc_voice_engine_unittest.cc | 38 +++- pc/BUILD.gn | 1 + pc/channel.cc | 6 +- pc/test/mock_voice_media_channel.h | 3 +- 17 files changed, 291 insertions(+), 196 deletions(-) diff --git a/call/packet_receiver.h b/call/packet_receiver.h index a97bb965ff..a36ab44ea7 100644 --- a/call/packet_receiver.h +++ b/call/packet_receiver.h @@ -26,9 +26,13 @@ class PacketReceiver { DELIVERY_PACKET_ERROR, }; + // TODO(perkj, https://bugs.webrtc.org/7135): Remove this method. This method + // is no longer used by PeerConnections. Some tests still use it. virtual DeliveryStatus DeliverPacket(MediaType media_type, rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) = 0; + int64_t packet_time_us) { + RTC_CHECK_NOTREACHED(); + } // Demux RTCP packets. Must be called on the worker thread. virtual void DeliverRtcpPacket(rtc::CopyOnWriteBuffer packet) { diff --git a/media/BUILD.gn b/media/BUILD.gn index 4ba6f382bf..ec2a27f1ec 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -541,6 +541,7 @@ rtc_library("rtc_audio_video") { ] absl_deps = [ "//third_party/abseil-cpp/absl/algorithm:container", + "//third_party/abseil-cpp/absl/functional:bind_front", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", ] diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index e1376c287b..82795a504f 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -28,6 +28,7 @@ #include "media/base/stream_params.h" #include "media/engine/webrtc_video_engine.h" #include "modules/audio_processing/include/audio_processing.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/copy_on_write_buffer.h" #include "rtc_base/network_route.h" #include "rtc_base/thread.h" @@ -296,9 +297,9 @@ class RtpHelper : public Base { void set_recv_rtcp_parameters(const RtcpParameters& params) { recv_rtcp_parameters_ = params; } - void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) override { - rtp_packets_.push_back(std::string(packet.cdata(), packet.size())); + void OnPacketReceived(const webrtc::RtpPacketReceived& packet) override { + rtp_packets_.push_back( + std::string(packet.Buffer().cdata(), packet.size())); } void OnPacketSent(const rtc::SentPacket& sent_packet) override {} void OnReadyToSend(bool ready) override { ready_to_send_ = ready; } diff --git a/media/base/fake_network_interface.h b/media/base/fake_network_interface.h index 53c5563935..993c6e1aff 100644 --- a/media/base/fake_network_interface.h +++ b/media/base/fake_network_interface.h @@ -20,6 +20,7 @@ #include "api/task_queue/task_queue_base.h" #include "media/base/media_channel.h" #include "media/base/rtp_utils.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_util.h" #include "rtc_base/byte_order.h" #include "rtc_base/checks.h" @@ -27,6 +28,7 @@ #include "rtc_base/dscp.h" #include "rtc_base/synchronization/mutex.h" #include "rtc_base/thread.h" +#include "rtc_base/time_utils.h" namespace cricket { @@ -167,7 +169,14 @@ class FakeNetworkInterface : public MediaChannelNetworkInterface { thread_->PostTask( SafeTask(safety_.flag(), [this, packet = std::move(packet)]() mutable { if (dest_) { - dest_->OnPacketReceived(std::move(packet), rtc::TimeMicros()); + webrtc::RtpPacketReceived parsed_packet; + if (parsed_packet.Parse(packet)) { + parsed_packet.set_arrival_time( + webrtc::Timestamp::Micros(rtc::TimeMicros())); + dest_->OnPacketReceived(std::move(parsed_packet)); + } else { + RTC_DCHECK_NOTREACHED(); + } } })); } diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 30e7571ab8..43e09290bd 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -46,6 +46,7 @@ #include "media/base/stream_params.h" #include "modules/audio_processing/include/audio_processing_statistics.h" #include "modules/rtp_rtcp/include/report_block_data.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/async_packet_socket.h" #include "rtc_base/buffer.h" #include "rtc_base/copy_on_write_buffer.h" @@ -194,8 +195,7 @@ class MediaBaseChannelInterface { // channel). // Called on the network when an RTP packet is received. - virtual void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) = 0; + virtual void OnPacketReceived(const webrtc::RtpPacketReceived& packet) = 0; // Called on the network thread after a transport has finished sending a // packet. virtual void OnPacketSent(const rtc::SentPacket& sent_packet) = 0; diff --git a/media/base/media_channel_impl.h b/media/base/media_channel_impl.h index 91b91c118a..8142dd45b6 100644 --- a/media/base/media_channel_impl.h +++ b/media/base/media_channel_impl.h @@ -45,6 +45,7 @@ #include "media/base/codec.h" #include "media/base/media_channel.h" #include "media/base/stream_params.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/async_packet_socket.h" #include "rtc_base/checks.h" #include "rtc_base/copy_on_write_buffer.h" @@ -89,8 +90,7 @@ class MediaChannel : public MediaSendChannelInterface, // even when abstract, to tell the compiler that all instances of the name // referred to by subclasses of this share the same implementation. cricket::MediaType media_type() const override = 0; - void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) override = 0; + void OnPacketReceived(const webrtc::RtpPacketReceived& packet) override = 0; void OnPacketSent(const rtc::SentPacket& sent_packet) override = 0; void OnReadyToSend(bool ready) override = 0; void OnNetworkRouteChanged(absl::string_view transport_name, @@ -305,9 +305,8 @@ class VoiceMediaSendChannel : public VoiceMediaSendChannelInterface { // Implementation of MediaBaseChannelInterface cricket::MediaType media_type() const override { return MEDIA_TYPE_AUDIO; } - void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) override { - impl()->OnPacketReceived(packet, packet_time_us); + void OnPacketReceived(const webrtc::RtpPacketReceived& packet) override { + impl()->OnPacketReceived(packet); } void OnPacketSent(const rtc::SentPacket& sent_packet) override { impl()->OnPacketSent(sent_packet); @@ -386,9 +385,8 @@ class VoiceMediaReceiveChannel : public VoiceMediaReceiveChannelInterface { virtual ~VoiceMediaReceiveChannel() {} // Implementation of MediaBaseChannelInterface cricket::MediaType media_type() const override { return MEDIA_TYPE_AUDIO; } - void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) override { - impl()->OnPacketReceived(packet, packet_time_us); + void OnPacketReceived(const webrtc::RtpPacketReceived& packet) override { + impl()->OnPacketReceived(packet); } void OnPacketSent(const rtc::SentPacket& sent_packet) override { impl()->OnPacketSent(sent_packet); @@ -491,9 +489,8 @@ class VideoMediaSendChannel : public VideoMediaSendChannelInterface { // Implementation of MediaBaseChannelInterface cricket::MediaType media_type() const override { return MEDIA_TYPE_VIDEO; } - void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) override { - impl()->OnPacketReceived(packet, packet_time_us); + void OnPacketReceived(const webrtc::RtpPacketReceived& packet) override { + impl()->OnPacketReceived(packet); } void OnPacketSent(const rtc::SentPacket& sent_packet) override { impl()->OnPacketSent(sent_packet); @@ -580,9 +577,8 @@ class VideoMediaReceiveChannel : public VideoMediaReceiveChannelInterface { explicit VideoMediaReceiveChannel(VideoMediaChannel* impl) : impl_(impl) {} // Implementation of MediaBaseChannelInterface cricket::MediaType media_type() const override { return MEDIA_TYPE_VIDEO; } - void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) override { - impl()->OnPacketReceived(packet, packet_time_us); + void OnPacketReceived(const webrtc::RtpPacketReceived& packet) override { + impl()->OnPacketReceived(packet); } void OnPacketSent(const rtc::SentPacket& sent_packet) override { impl()->OnPacketSent(sent_packet); diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc index c2a068d2f2..ef9224efc9 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -696,6 +696,7 @@ void FakeCall::DeliverRtpPacket( packet.arrival_time()); } } + last_received_rtp_packet_ = packet; } bool FakeCall::DeliverPacketInternal(webrtc::MediaType media_type, diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 5fc2745316..0301952693 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -384,6 +384,9 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { const std::vector& GetFlexfecReceiveStreams(); rtc::SentPacket last_sent_packet() const { return last_sent_packet_; } + const webrtc::RtpPacketReceived& last_received_rtp_packet() const { + return last_received_rtp_packet_; + } size_t GetDeliveredPacketsForSsrc(uint32_t ssrc) const { auto it = delivered_packets_by_ssrc_.find(ssrc); return it != delivered_packets_by_ssrc_.end() ? it->second : 0u; @@ -489,6 +492,7 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { webrtc::NetworkState audio_network_state_; webrtc::NetworkState video_network_state_; rtc::SentPacket last_sent_packet_; + webrtc::RtpPacketReceived last_received_rtp_packet_; int last_sent_nonnegative_packet_id_ = -1; int next_stream_id_ = 665; webrtc::Call::Stats stats_; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index cd80b3f027..dd5ccf6ebd 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -18,6 +18,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/functional/bind_front.h" #include "absl/strings/match.h" #include "api/media_stream_interface.h" #include "api/video/video_codec_constants.h" @@ -29,6 +30,7 @@ #include "call/call.h" #include "media/engine/webrtc_media_engine.h" #include "media/engine/webrtc_voice_engine.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtp_util.h" #include "modules/video_coding/codecs/vp9/svc_config.h" #include "modules/video_coding/svc/scalability_mode_util.h" @@ -1196,6 +1198,8 @@ bool WebRtcVideoChannel::SetRecvParameters(const VideoRecvParameters& params) { } if (changed_params.rtp_header_extensions) { recv_rtp_extensions_ = *changed_params.rtp_header_extensions; + recv_rtp_extension_map_ = + webrtc::RtpHeaderExtensionMap(recv_rtp_extensions_); } if (changed_params.codec_settings) { RTC_DLOG(LS_INFO) << "Changing recv codecs from " @@ -1718,111 +1722,111 @@ void WebRtcVideoChannel::FillReceiveCodecStats( } } -void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) { +void WebRtcVideoChannel::OnPacketReceived( + const webrtc::RtpPacketReceived& packet) { RTC_DCHECK_RUN_ON(&network_thread_checker_); + // TODO(bugs.webrtc.org/11993): This code is very similar to what // WebRtcVoiceMediaChannel::OnPacketReceived does. For maintainability and // consistency it would be good to move the interaction with call_->Receiver() // to a common implementation and provide a callback on the worker thread // for the exception case (DELIVERY_UNKNOWN_SSRC) and how retry is attempted. worker_thread_->PostTask( - SafeTask(task_safety_.flag(), [this, packet, packet_time_us] { + SafeTask(task_safety_.flag(), [this, packet = packet]() mutable { RTC_DCHECK_RUN_ON(&thread_checker_); - const webrtc::PacketReceiver::DeliveryStatus delivery_result = - call_->Receiver()->DeliverPacket(webrtc::MediaType::VIDEO, packet, - packet_time_us); - switch (delivery_result) { - case webrtc::PacketReceiver::DELIVERY_OK: - return; - case webrtc::PacketReceiver::DELIVERY_PACKET_ERROR: - return; - case webrtc::PacketReceiver::DELIVERY_UNKNOWN_SSRC: - break; + + // TODO(bugs.webrtc.org/7135): extensions in `packet` is currently set + // in RtpTransport and does not neccessarily include extensions specific + // to this channel/MID. Also see comment in + // BaseChannel::MaybeUpdateDemuxerAndRtpExtensions_w. + // It would likely be good if extensions where merged per BUNDLE and + // applied directly in RtpTransport::DemuxPacket; + packet.IdentifyExtensions(recv_rtp_extension_map_); + packet.set_payload_type_frequency(webrtc::kVideoPayloadTypeFrequency); + if (!packet.arrival_time().IsFinite()) { + packet.set_arrival_time(webrtc::Timestamp::Micros(rtc::TimeMicros())); } - absl::optional rtx_ssrc; - uint32_t ssrc = ParseRtpSsrc(packet); - - - if (discard_unknown_ssrc_packets_) { - return; - } - - int payload_type = ParseRtpPayloadType(packet); - - // See if this payload_type is registered as one that usually gets its - // own SSRC (RTX) or at least is safe to drop either way (FEC). If it - // is, and it wasn't handled above by DeliverPacket, that means we don't - // know what stream it associates with, and we shouldn't ever create an - // implicit channel for these. - for (auto& codec : recv_codecs_) { - if (payload_type == codec.ulpfec.red_rtx_payload_type || - payload_type == codec.ulpfec.ulpfec_payload_type) { - return; - } - if (payload_type == codec.rtx_payload_type) { - // As we don't support receiving simulcast there can only be one RTX - // stream, which will be associated with unsignaled media stream. - // It is not possible to update the ssrcs of a receive stream, so we - // recreate it insead if found. - auto default_ssrc = GetUnsignaledSsrc(); - if (!default_ssrc) { - return; - } - rtx_ssrc = ssrc; - ssrc = *default_ssrc; - // Allow recreating the receive stream even if the RTX packet is - // received just after the media packet. - last_unsignalled_ssrc_creation_time_ms_.reset(); - break; - } - } - if (payload_type == recv_flexfec_payload_type_) { - return; - } - - // Ignore unknown ssrcs if there is a demuxer criteria update pending. - // During a demuxer update we may receive ssrcs that were recently - // removed or we may receve ssrcs that were recently configured for a - // different video channel. - if (demuxer_criteria_id_ != demuxer_criteria_completed_id_) { - return; - } - // Ignore unknown ssrcs if we recently created an unsignalled receive - // stream since this shouldn't happen frequently. Getting into a state - // of creating decoders on every packet eats up processing time (e.g. - // https://crbug.com/1069603) and this cooldown prevents that. - if (last_unsignalled_ssrc_creation_time_ms_.has_value()) { - int64_t now_ms = rtc::TimeMillis(); - if (now_ms - last_unsignalled_ssrc_creation_time_ms_.value() < - kUnsignaledSsrcCooldownMs) { - // We've already created an unsignalled ssrc stream within the last - // 0.5 s, ignore with a warning. - RTC_LOG(LS_WARNING) - << "Another unsignalled ssrc packet arrived shortly after the " - << "creation of an unsignalled ssrc stream. Dropping packet."; - return; - } - } - // Let the unsignalled ssrc handler decide whether to drop or deliver. - switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc, - rtx_ssrc)) { - case UnsignalledSsrcHandler::kDropPacket: - return; - case UnsignalledSsrcHandler::kDeliverPacket: - break; - } - - if (call_->Receiver()->DeliverPacket(webrtc::MediaType::VIDEO, packet, - packet_time_us) != - webrtc::PacketReceiver::DELIVERY_OK) { - RTC_LOG(LS_WARNING) << "Failed to deliver RTP packet on re-delivery."; - } - last_unsignalled_ssrc_creation_time_ms_ = rtc::TimeMillis(); + call_->Receiver()->DeliverRtpPacket( + webrtc::MediaType::VIDEO, std::move(packet), + absl::bind_front( + &WebRtcVideoChannel::MaybeCreateDefaultReceiveStream, this)); })); } +bool WebRtcVideoChannel::MaybeCreateDefaultReceiveStream( + const webrtc::RtpPacketReceived& packet) { + if (discard_unknown_ssrc_packets_) { + return false; + } + + absl::optional rtx_ssrc; + uint32_t ssrc = packet.Ssrc(); + // See if this payload_type is registered as one that usually gets its + // own SSRC (RTX) or at least is safe to drop either way (FEC). If it + // is, and it wasn't handled above by DeliverPacket, that means we don't + // know what stream it associates with, and we shouldn't ever create an + // implicit channel for these. + for (auto& codec : recv_codecs_) { + if (packet.PayloadType() == codec.ulpfec.red_rtx_payload_type || + packet.PayloadType() == codec.ulpfec.ulpfec_payload_type) { + return false; + } + if (packet.PayloadType() == codec.rtx_payload_type) { + // As we don't support receiving simulcast there can only be one RTX + // stream, which will be associated with unsignaled media stream. + // It is not possible to update the ssrcs of a receive stream, so we + // recreate it insead if found. + auto default_ssrc = GetUnsignaledSsrc(); + if (!default_ssrc) { + return false; + } + rtx_ssrc = ssrc; + ssrc = *default_ssrc; + // Allow recreating the receive stream even if the RTX packet is + // received just after the media packet. + last_unsignalled_ssrc_creation_time_ms_.reset(); + break; + } + } + if (packet.PayloadType() == recv_flexfec_payload_type_) { + return false; + } + + // Ignore unknown ssrcs if there is a demuxer criteria update pending. + // During a demuxer update we may receive ssrcs that were recently + // removed or we may receve ssrcs that were recently configured for a + // different video channel. + if (demuxer_criteria_id_ != demuxer_criteria_completed_id_) { + return false; + } + // Ignore unknown ssrcs if we recently created an unsignalled receive + // stream since this shouldn't happen frequently. Getting into a state + // of creating decoders on every packet eats up processing time (e.g. + // https://crbug.com/1069603) and this cooldown prevents that. + if (last_unsignalled_ssrc_creation_time_ms_.has_value()) { + int64_t now_ms = rtc::TimeMillis(); + if (now_ms - last_unsignalled_ssrc_creation_time_ms_.value() < + kUnsignaledSsrcCooldownMs) { + // We've already created an unsignalled ssrc stream within the last + // 0.5 s, ignore with a warning. + RTC_LOG(LS_WARNING) + << "Another unsignalled ssrc packet arrived shortly after the " + << "creation of an unsignalled ssrc stream. Dropping packet."; + return false; + } + } + // Let the unsignalled ssrc handler decide whether to drop or deliver. + switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc, rtx_ssrc)) { + case UnsignalledSsrcHandler::kDropPacket: + return false; + case UnsignalledSsrcHandler::kDeliverPacket: + break; + } + last_unsignalled_ssrc_creation_time_ms_ = rtc::TimeMillis(); + return true; +} + 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 diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index bf27defc92..43d53f9c87 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -176,8 +176,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, bool GetSendStats(VideoMediaSendInfo* info) override; bool GetReceiveStats(VideoMediaReceiveInfo* info) override; - void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) override; + void OnPacketReceived(const webrtc::RtpPacketReceived& packet) override; void OnPacketSent(const rtc::SentPacket& sent_packet) override; void OnReadyToSend(bool ready) override; void OnNetworkRouteChanged(absl::string_view transport_name, @@ -316,6 +315,12 @@ class WebRtcVideoChannel : public VideoMediaChannel, ChangedRecvParameters* changed_params) const RTC_EXCLUSIVE_LOCKS_REQUIRED(thread_checker_); + // Expected to be invoked once per packet that belongs to this channel that + // can not be demuxed. + // Returns true if a new default stream has been created. + bool MaybeCreateDefaultReceiveStream( + const webrtc::RtpPacketReceived& parsed_packet) + RTC_EXCLUSIVE_LOCKS_REQUIRED(thread_checker_); void ConfigureReceiverRtp( webrtc::VideoReceiveStreamInterface::Config* config, webrtc::FlexfecReceiveStream::Config* flexfec_config, @@ -646,6 +651,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, webrtc::VideoBitrateAllocatorFactory* const bitrate_allocator_factory_ RTC_GUARDED_BY(thread_checker_); std::vector recv_codecs_ RTC_GUARDED_BY(thread_checker_); + webrtc::RtpHeaderExtensionMap recv_rtp_extension_map_ + RTC_GUARDED_BY(thread_checker_); std::vector recv_rtp_extensions_ RTC_GUARDED_BY(thread_checker_); // See reason for keeping track of the FlexFEC payload type separately in diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 8de0917551..482253631c 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -11,6 +11,7 @@ #include "media/engine/webrtc_video_engine.h" #include +#include #include #include #include @@ -1471,7 +1472,7 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { buf_ptr[8] = height & 255; buf_ptr[9] = height >> 8; - channel_->OnPacketReceived(packet.Buffer(), /*packet_time_us=*/-1); + channel_->OnPacketReceived(packet); } void DeliverKeyFrameAndWait(uint32_t ssrc) { @@ -2065,7 +2066,7 @@ TEST_F(WebRtcVideoChannelBaseTest, SetSink) { EXPECT_TRUE(SetDefaultCodec()); EXPECT_TRUE(SetSend(true)); EXPECT_EQ(0, renderer_.num_rendered_frames()); - channel_->OnPacketReceived(packet.Buffer(), /*packet_time_us=*/-1); + channel_->OnPacketReceived(packet); channel_->SetDefaultSink(&renderer_); SendFrame(); EXPECT_FRAME_WAIT(1, kVideoWidth, kVideoHeight, kTimeout); @@ -2501,7 +2502,7 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { // After receciving and processing the packet, enough time is advanced that // the unsignalled receive stream cooldown is no longer in effect. void ReceivePacketAndAdvanceTime(const RtpPacketReceived& packet) { - receive_channel_->OnPacketReceived(packet.Buffer(), /*packet_time_us=*/-1); + receive_channel_->OnPacketReceived(packet); rtc::Thread::Current()->ProcessMessages(0); time_controller_.AdvanceTime( webrtc::TimeDelta::Millis(kUnsignalledReceiveStreamCooldownMs)); @@ -3106,6 +3107,27 @@ TEST_F(WebRtcVideoChannelTest, SetRecvRtpHeaderExtensionsRejectsDuplicateIds) { EXPECT_FALSE(channel_->SetRecvParameters(recv_parameters_)); } +TEST_F(WebRtcVideoChannelTest, OnPacketReceivedIdentifiesExtensions) { + cricket::VideoRecvParameters parameters = recv_parameters_; + parameters.extensions.push_back( + RtpExtension(RtpExtension::kVideoRotationUri, /*id=*/1)); + ASSERT_TRUE(channel_->SetRecvParameters(parameters)); + webrtc::RtpHeaderExtensionMap extension_map(parameters.extensions); + RtpPacketReceived reference_packet(&extension_map); + reference_packet.SetExtension( + webrtc::VideoRotation::kVideoRotation_270); + // Create a packet without the extension map but with the same content. + RtpPacketReceived received_packet; + ASSERT_TRUE(received_packet.Parse(reference_packet.Buffer())); + + receive_channel_->OnPacketReceived(received_packet); + rtc::Thread::Current()->ProcessMessages(0); + + EXPECT_EQ(fake_call_->last_received_rtp_packet() + .GetExtension(), + webrtc::VideoRotation::kVideoRotation_270); +} + TEST_F(WebRtcVideoChannelTest, AddRecvStreamOnlyUsesOneReceiveStream) { EXPECT_TRUE( receive_channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(1))); @@ -6999,7 +7021,7 @@ TEST_F(WebRtcVideoChannelTest, UnsignalledSsrcHasACooldown) { // Receive a packet for kSsrc1. RtpPacketReceived packet; packet.SetSsrc(kSsrc1); - receive_channel_->OnPacketReceived(packet.Buffer(), /*packet_time_us=*/-1); + receive_channel_->OnPacketReceived(packet); } rtc::Thread::Current()->ProcessMessages(0); time_controller_.AdvanceTime( @@ -7014,7 +7036,7 @@ TEST_F(WebRtcVideoChannelTest, UnsignalledSsrcHasACooldown) { // Receive a packet for kSsrc2. RtpPacketReceived packet; packet.SetSsrc(kSsrc2); - receive_channel_->OnPacketReceived(packet.Buffer(), /*packet_time_us=*/-1); + receive_channel_->OnPacketReceived(packet); } rtc::Thread::Current()->ProcessMessages(0); @@ -7031,7 +7053,7 @@ TEST_F(WebRtcVideoChannelTest, UnsignalledSsrcHasACooldown) { // Receive a packet for kSsrc2. RtpPacketReceived packet; packet.SetSsrc(kSsrc2); - receive_channel_->OnPacketReceived(packet.Buffer(), /*packet_time_us=*/-1); + receive_channel_->OnPacketReceived(packet); } rtc::Thread::Current()->ProcessMessages(0); diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 694b8b9196..c425916114 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -19,6 +19,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/functional/bind_front.h" #include "absl/strings/match.h" #include "api/audio/audio_frame_processor.h" #include "api/audio_codecs/audio_codec_pair_id.h" @@ -36,6 +37,8 @@ #include "modules/audio_mixer/audio_mixer_impl.h" #include "modules/audio_processing/aec_dump/aec_dump_factory.h" #include "modules/audio_processing/include/audio_processing.h" +#include "modules/rtp_rtcp/include/rtp_header_extension_map.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_util.h" #include "rtc_base/arraysize.h" #include "rtc_base/byte_order.h" @@ -1353,6 +1356,8 @@ bool WebRtcVoiceMediaChannel::SetRecvParameters( call_->trials()); if (recv_rtp_extensions_ != filtered_extensions) { recv_rtp_extensions_.swap(filtered_extensions); + recv_rtp_extension_map_ = + webrtc::RtpHeaderExtensionMap(recv_rtp_extensions_); for (auto& it : recv_streams_) { it.second->SetRtpExtensions(recv_rtp_extensions_); } @@ -2129,74 +2134,84 @@ bool WebRtcVoiceMediaChannel::InsertDtmf(uint32_t ssrc, event, duration); } -void WebRtcVoiceMediaChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) { +void WebRtcVoiceMediaChannel::OnPacketReceived( + const webrtc::RtpPacketReceived& packet) { RTC_DCHECK_RUN_ON(&network_thread_checker_); + // TODO(bugs.webrtc.org/11993): This code is very similar to what // WebRtcVideoChannel::OnPacketReceived does. For maintainability and - // consistency it would be good to move the interaction with call_->Receiver() - // to a common implementation and provide a callback on the worker thread - // for the exception case (DELIVERY_UNKNOWN_SSRC) and how retry is attempted. - worker_thread_->PostTask(SafeTask(task_safety_.flag(), [this, packet, - packet_time_us] { - RTC_DCHECK_RUN_ON(worker_thread_); + // consistency it would be good to move the interaction with + // call_->Receiver() to a common implementation and provide a callback on + // the worker thread for the exception case (DELIVERY_UNKNOWN_SSRC) and + // how retry is attempted. + worker_thread_->PostTask( + SafeTask(task_safety_.flag(), [this, packet = packet]() mutable { + RTC_DCHECK_RUN_ON(worker_thread_); - webrtc::PacketReceiver::DeliveryStatus delivery_result = - call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO, packet, - packet_time_us); + // TODO(bugs.webrtc.org/7135): extensions in `packet` is currently set + // in RtpTransport and does not neccessarily include extensions specific + // to this channel/MID. Also see comment in + // BaseChannel::MaybeUpdateDemuxerAndRtpExtensions_w. + // It would likely be good if extensions where merged per BUNDLE and + // applied directly in RtpTransport::DemuxPacket; + packet.IdentifyExtensions(recv_rtp_extension_map_); + if (!packet.arrival_time().IsFinite()) { + packet.set_arrival_time(webrtc::Timestamp::Micros(rtc::TimeMicros())); + } - if (delivery_result != webrtc::PacketReceiver::DELIVERY_UNKNOWN_SSRC) { - return; + call_->Receiver()->DeliverRtpPacket( + webrtc::MediaType::AUDIO, std::move(packet), + absl::bind_front( + &WebRtcVoiceMediaChannel::MaybeCreateDefaultReceiveStream, + this)); + })); +} + +bool WebRtcVoiceMediaChannel::MaybeCreateDefaultReceiveStream( + const webrtc::RtpPacketReceived& packet) { + // Create an unsignaled receive stream for this previously not received + // ssrc. If there already is N unsignaled receive streams, delete the + // oldest. See: https://bugs.chromium.org/p/webrtc/issues/detail?id=5208 + uint32_t ssrc = packet.Ssrc(); + RTC_DCHECK(!absl::c_linear_search(unsignaled_recv_ssrcs_, ssrc)); + + // Add new stream. + StreamParams sp = unsignaled_stream_params_; + sp.ssrcs.push_back(ssrc); + RTC_LOG(LS_INFO) << "Creating unsignaled receive stream for SSRC=" << ssrc; + if (!AddRecvStream(sp)) { + RTC_LOG(LS_WARNING) << "Could not create unsignaled receive stream."; + return false; + } + unsignaled_recv_ssrcs_.push_back(ssrc); + RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.NumOfUnsignaledStreams", + unsignaled_recv_ssrcs_.size(), 1, 100, 101); + + // Remove oldest unsignaled stream, if we have too many. + if (unsignaled_recv_ssrcs_.size() > kMaxUnsignaledRecvStreams) { + uint32_t remove_ssrc = unsignaled_recv_ssrcs_.front(); + RTC_DLOG(LS_INFO) << "Removing unsignaled receive stream with SSRC=" + << remove_ssrc; + RemoveRecvStream(remove_ssrc); + } + RTC_DCHECK_GE(kMaxUnsignaledRecvStreams, unsignaled_recv_ssrcs_.size()); + + SetOutputVolume(ssrc, default_recv_volume_); + SetBaseMinimumPlayoutDelayMs(ssrc, default_recv_base_minimum_delay_ms_); + + // The default sink can only be attached to one stream at a time, so we hook + // it up to the *latest* unsignaled stream we've seen, in order to support + // the case where the SSRC of one unsignaled stream changes. + if (default_sink_) { + for (uint32_t drop_ssrc : unsignaled_recv_ssrcs_) { + auto it = recv_streams_.find(drop_ssrc); + it->second->SetRawAudioSink(nullptr); } - - // Create an unsignaled receive stream for this previously not received - // ssrc. If there already is N unsignaled receive streams, delete the - // oldest. See: https://bugs.chromium.org/p/webrtc/issues/detail?id=5208 - uint32_t ssrc = ParseRtpSsrc(packet); - RTC_DCHECK(!absl::c_linear_search(unsignaled_recv_ssrcs_, ssrc)); - - // Add new stream. - StreamParams sp = unsignaled_stream_params_; - sp.ssrcs.push_back(ssrc); - RTC_LOG(LS_INFO) << "Creating unsignaled receive stream for SSRC=" << ssrc; - if (!AddRecvStream(sp)) { - RTC_LOG(LS_WARNING) << "Could not create unsignaled receive stream."; - return; - } - unsignaled_recv_ssrcs_.push_back(ssrc); - RTC_HISTOGRAM_COUNTS_LINEAR("WebRTC.Audio.NumOfUnsignaledStreams", - unsignaled_recv_ssrcs_.size(), 1, 100, 101); - - // Remove oldest unsignaled stream, if we have too many. - if (unsignaled_recv_ssrcs_.size() > kMaxUnsignaledRecvStreams) { - uint32_t remove_ssrc = unsignaled_recv_ssrcs_.front(); - RTC_DLOG(LS_INFO) << "Removing unsignaled receive stream with SSRC=" - << remove_ssrc; - RemoveRecvStream(remove_ssrc); - } - RTC_DCHECK_GE(kMaxUnsignaledRecvStreams, unsignaled_recv_ssrcs_.size()); - - SetOutputVolume(ssrc, default_recv_volume_); - SetBaseMinimumPlayoutDelayMs(ssrc, default_recv_base_minimum_delay_ms_); - - // The default sink can only be attached to one stream at a time, so we hook - // it up to the *latest* unsignaled stream we've seen, in order to support - // the case where the SSRC of one unsignaled stream changes. - if (default_sink_) { - for (uint32_t drop_ssrc : unsignaled_recv_ssrcs_) { - auto it = recv_streams_.find(drop_ssrc); - it->second->SetRawAudioSink(nullptr); - } - std::unique_ptr proxy_sink( - new ProxySink(default_sink_.get())); - SetRawAudioSink(ssrc, std::move(proxy_sink)); - } - - delivery_result = call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO, - packet, packet_time_us); - RTC_DCHECK_NE(webrtc::PacketReceiver::DELIVERY_UNKNOWN_SSRC, - delivery_result); - })); + std::unique_ptr proxy_sink( + new ProxySink(default_sink_.get())); + SetRawAudioSink(ssrc, std::move(proxy_sink)); + } + return true; } void WebRtcVoiceMediaChannel::OnPacketSent(const rtc::SentPacket& sent_packet) { diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index 835be360e7..25f8e3dc1e 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -202,8 +202,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, bool CanInsertDtmf() override; bool InsertDtmf(uint32_t ssrc, int event, int duration) override; - void OnPacketReceived(rtc::CopyOnWriteBuffer packet, - int64_t packet_time_us) override; + void OnPacketReceived(const webrtc::RtpPacketReceived& packet) override; void OnPacketSent(const rtc::SentPacket& sent_packet) override; void OnNetworkRouteChanged(absl::string_view transport_name, const rtc::NetworkRoute& network_route) override; @@ -253,6 +252,11 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, bool DeleteVoEChannel(int channel); bool SetMaxSendBitrate(int bps); void SetupRecording(); + + // Expected to be invoked once per packet that belongs to this channel that + // can not be demuxed. Returns true if a default receive stream has been + // created. + bool MaybeCreateDefaultReceiveStream(const webrtc::RtpPacketReceived& packet); // Check if 'ssrc' is an unsignaled stream, and if so mark it as not being // unsignaled anymore (i.e. it is now removed, or signaled), and return true. bool MaybeDeregisterUnsignaledRecvStream(uint32_t ssrc); @@ -311,6 +315,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, class WebRtcAudioReceiveStream; std::map recv_streams_; std::vector recv_rtp_extensions_; + webrtc::RtpHeaderExtensionMap recv_rtp_extension_map_; absl::optional send_codec_spec_; diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index 422906f3b1..adc6e646e8 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -31,6 +31,8 @@ #include "modules/audio_device/include/mock_audio_device.h" #include "modules/audio_mixer/audio_mixer_impl.h" #include "modules/audio_processing/include/mock_audio_processing.h" +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/arraysize.h" #include "rtc_base/byte_order.h" #include "rtc_base/numerics/safe_conversions.h" @@ -275,9 +277,9 @@ class WebRtcVoiceEngineTestFake : public ::testing::TestWithParam { } void DeliverPacket(const void* data, int len) { - rtc::CopyOnWriteBuffer packet(reinterpret_cast(data), len); - receive_channel_->OnPacketReceived(packet, - /* packet_time_us */ -1); + webrtc::RtpPacketReceived packet; + packet.Parse(reinterpret_cast(data), len); + receive_channel_->OnPacketReceived(packet); rtc::Thread::Current()->ProcessMessages(0); } @@ -1474,6 +1476,31 @@ TEST_P(WebRtcVoiceEngineTestFake, GetRtpReceiveParametersWithUnsignaledSsrc) { EXPECT_FALSE(rtp_parameters.encodings[0].ssrc); } +TEST_P(WebRtcVoiceEngineTestFake, OnPacketReceivedIdentifiesExtensions) { + ASSERT_TRUE(SetupChannel()); + cricket::AudioRecvParameters parameters = recv_parameters_; + parameters.extensions.push_back( + RtpExtension(RtpExtension::kAudioLevelUri, /*id=*/1)); + ASSERT_TRUE(channel_->SetRecvParameters(parameters)); + webrtc::RtpHeaderExtensionMap extension_map(parameters.extensions); + webrtc::RtpPacketReceived reference_packet(&extension_map); + constexpr uint8_t kAudioLevel = 123; + reference_packet.SetExtension(/*voice_activity=*/true, + kAudioLevel); + // Create a packet without the extension map but with the same content. + webrtc::RtpPacketReceived received_packet; + ASSERT_TRUE(received_packet.Parse(reference_packet.Buffer())); + + receive_channel_->OnPacketReceived(received_packet); + rtc::Thread::Current()->ProcessMessages(0); + + bool voice_activity; + uint8_t audio_level; + EXPECT_TRUE(call_.last_received_rtp_packet().GetExtension( + &voice_activity, &audio_level)); + EXPECT_EQ(audio_level, kAudioLevel); +} + // Test that we apply codecs properly. TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecs) { EXPECT_TRUE(SetupSendStream()); @@ -3419,8 +3446,9 @@ TEST_P(WebRtcVoiceEngineTestFake, DeliverAudioPacket_Call) { const cricket::FakeAudioReceiveStream* s = call_.GetAudioReceiveStream(kAudioSsrc); EXPECT_EQ(0, s->received_packets()); - receive_channel_->OnPacketReceived(kPcmuPacket, - /* packet_time_us */ -1); + webrtc::RtpPacketReceived parsed_packet; + RTC_CHECK(parsed_packet.Parse(kPcmuPacket)); + receive_channel_->OnPacketReceived(parsed_packet); rtc::Thread::Current()->ProcessMessages(0); EXPECT_EQ(1, s->received_packets()); diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 4ee2321148..2ecf4b230d 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2803,6 +2803,7 @@ if (rtc_include_tests && !build_with_chromium) { "../modules/audio_device", "../modules/audio_processing", "../modules/audio_processing:api", + "../modules/rtp_rtcp:rtp_rtcp_format", "../p2p:fake_port_allocator", "../p2p:p2p_test_utils", "../p2p:rtc_p2p", diff --git a/pc/channel.cc b/pc/channel.cc index 09ed037ee4..6d261ece98 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -433,11 +433,7 @@ void BaseChannel::OnRtpPacket(const webrtc::RtpPacketReceived& parsed_packet) { << ToString(); return; } - - webrtc::Timestamp packet_time = parsed_packet.arrival_time(); - media_channel_->OnPacketReceived( - parsed_packet.Buffer(), - packet_time.IsMinusInfinity() ? -1 : packet_time.us()); + media_channel_->OnPacketReceived(parsed_packet); } bool BaseChannel::MaybeUpdateDemuxerAndRtpExtensions_w( diff --git a/pc/test/mock_voice_media_channel.h b/pc/test/mock_voice_media_channel.h index e01e235a6f..2e5a8b5801 100644 --- a/pc/test/mock_voice_media_channel.h +++ b/pc/test/mock_voice_media_channel.h @@ -17,6 +17,7 @@ #include "api/call/audio_sink.h" #include "media/base/media_channel.h" #include "media/base/media_channel_impl.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/gunit.h" #include "test/gmock.h" #include "test/gtest.h" @@ -36,7 +37,7 @@ class MockVoiceMediaChannel : public VoiceMediaChannel { (override)); MOCK_METHOD(void, OnPacketReceived, - (rtc::CopyOnWriteBuffer packet, int64_t packet_time_us), + (const webrtc::RtpPacketReceived& packet), (override)); MOCK_METHOD(void, OnPacketSent,