From f4aadf37749a00ed6f182c76ae61c356deb64e2b Mon Sep 17 00:00:00 2001 From: Per K Date: Tue, 27 Feb 2024 09:01:15 +0100 Subject: [PATCH] Change RtpTransport and DsctTransport to receives packets through ReceivedPacketCallback Instead of using PacketTransportInternal::SignalReadPacket. Bug: webrtc:15368 Change-Id: Icdc2d7f85df6db944f0ba0232891e6c5a8986a66 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/340440 Commit-Queue: Per Kjellander Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#41823} --- media/BUILD.gn | 1 + media/base/rtp_utils.cc | 8 +- media/base/rtp_utils.h | 4 +- media/base/rtp_utils_unittest.cc | 15 ++-- media/sctp/dcsctp_transport.cc | 24 +++--- media/sctp/dcsctp_transport.h | 6 +- p2p/BUILD.gn | 1 + p2p/base/fake_dtls_transport.h | 23 +++--- pc/BUILD.gn | 1 + pc/channel_unittest.cc | 134 +++++++++++++++++-------------- pc/jsep_transport_unittest.cc | 47 ++++++----- pc/rtp_transport.cc | 40 +++++---- pc/rtp_transport.h | 5 +- 13 files changed, 170 insertions(+), 139 deletions(-) diff --git a/media/BUILD.gn b/media/BUILD.gn index e4cb4f29fc..4a3b018a0f 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -622,6 +622,7 @@ if (rtc_build_dcsctp) { "../rtc_base:stringutils", "../rtc_base:threading", "../rtc_base/containers:flat_map", + "../rtc_base/network:received_packet", "../rtc_base/third_party/sigslot:sigslot", "../system_wrappers", ] diff --git a/media/base/rtp_utils.cc b/media/base/rtp_utils.cc index c630cbc7e4..9546ed6e30 100644 --- a/media/base/rtp_utils.cc +++ b/media/base/rtp_utils.cc @@ -12,6 +12,7 @@ #include +#include #include // PacketTimeUpdateParams is defined in asyncpacketsocket.h. @@ -172,12 +173,11 @@ absl::string_view RtpPacketTypeToString(RtpPacketType packet_type) { RTC_CHECK_NOTREACHED(); } -RtpPacketType InferRtpPacketType(rtc::ArrayView packet) { - if (webrtc::IsRtcpPacket( - rtc::reinterpret_array_view(packet))) { +RtpPacketType InferRtpPacketType(rtc::ArrayView packet) { + if (webrtc::IsRtcpPacket(packet)) { return RtpPacketType::kRtcp; } - if (webrtc::IsRtpPacket(rtc::reinterpret_array_view(packet))) { + if (webrtc::IsRtpPacket(packet)) { return RtpPacketType::kRtp; } return RtpPacketType::kUnknown; diff --git a/media/base/rtp_utils.h b/media/base/rtp_utils.h index a501fd7af3..8ed321206f 100644 --- a/media/base/rtp_utils.h +++ b/media/base/rtp_utils.h @@ -11,6 +11,8 @@ #ifndef MEDIA_BASE_RTP_UTILS_H_ #define MEDIA_BASE_RTP_UTILS_H_ +#include + #include "absl/strings/string_view.h" #include "api/array_view.h" #include "rtc_base/byte_order.h" @@ -46,7 +48,7 @@ bool GetRtcpType(const void* data, size_t len, int* value); bool GetRtcpSsrc(const void* data, size_t len, uint32_t* value); // Checks the packet header to determine if it can be an RTP or RTCP packet. -RtpPacketType InferRtpPacketType(rtc::ArrayView packet); +RtpPacketType InferRtpPacketType(rtc::ArrayView packet); // True if |payload type| is 0-127. bool IsValidRtpPayloadType(int payload_type); diff --git a/media/base/rtp_utils_unittest.cc b/media/base/rtp_utils_unittest.cc index a594f944c0..7d48e72244 100644 --- a/media/base/rtp_utils_unittest.cc +++ b/media/base/rtp_utils_unittest.cc @@ -72,15 +72,12 @@ static const int kAstIndexInOneByteRtpMsg = 21; // and in message `kRtpMsgWithTwoByteAbsSendTimeExtension`. static const int kAstIndexInTwoByteRtpMsg = 21; -static const rtc::ArrayView kPcmuFrameArrayView = - rtc::MakeArrayView(reinterpret_cast(kPcmuFrame), - sizeof(kPcmuFrame)); -static const rtc::ArrayView kRtcpReportArrayView = - rtc::MakeArrayView(reinterpret_cast(kRtcpReport), - sizeof(kRtcpReport)); -static const rtc::ArrayView kInvalidPacketArrayView = - rtc::MakeArrayView(reinterpret_cast(kInvalidPacket), - sizeof(kInvalidPacket)); +static const rtc::ArrayView kPcmuFrameArrayView = + rtc::MakeArrayView(kPcmuFrame, sizeof(kPcmuFrame)); +static const rtc::ArrayView kRtcpReportArrayView = + rtc::MakeArrayView(kRtcpReport, sizeof(kRtcpReport)); +static const rtc::ArrayView kInvalidPacketArrayView = + rtc::MakeArrayView(kInvalidPacket, sizeof(kInvalidPacket)); TEST(RtpUtilsTest, GetRtcp) { int pt; diff --git a/media/sctp/dcsctp_transport.cc b/media/sctp/dcsctp_transport.cc index 525075468c..fa1d99de3a 100644 --- a/media/sctp/dcsctp_transport.cc +++ b/media/sctp/dcsctp_transport.cc @@ -27,6 +27,7 @@ #include "p2p/base/packet_transport_internal.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "rtc_base/network/received_packet.h" #include "rtc_base/socket.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/thread.h" @@ -605,8 +606,11 @@ void DcSctpTransport::ConnectTransportSignals() { } transport_->SignalWritableState.connect( this, &DcSctpTransport::OnTransportWritableState); - transport_->SignalReadPacket.connect(this, - &DcSctpTransport::OnTransportReadPacket); + transport_->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnTransportReadPacket(transport, packet); + }); transport_->SignalClosed.connect(this, &DcSctpTransport::OnTransportClosed); } @@ -616,7 +620,7 @@ void DcSctpTransport::DisconnectTransportSignals() { return; } transport_->SignalWritableState.disconnect(this); - transport_->SignalReadPacket.disconnect(this); + transport_->DeregisterReceivedPacketCallback(this); transport_->SignalClosed.disconnect(this); } @@ -632,21 +636,17 @@ void DcSctpTransport::OnTransportWritableState( void DcSctpTransport::OnTransportReadPacket( rtc::PacketTransportInternal* transport, - const char* data, - size_t length, - const int64_t& /* packet_time_us */, - int flags) { + const rtc::ReceivedPacket& packet) { RTC_DCHECK_RUN_ON(network_thread_); - if (flags) { + if (packet.decryption_info() != rtc::ReceivedPacket::kDtlsDecrypted) { // We are only interested in SCTP packets. return; } - RTC_DLOG(LS_VERBOSE) << debug_name_ - << "->OnTransportReadPacket(), length=" << length; + RTC_DLOG(LS_VERBOSE) << debug_name_ << "->OnTransportReadPacket(), length=" + << packet.payload().size(); if (socket_) { - socket_->ReceivePacket(rtc::ArrayView( - reinterpret_cast(data), length)); + socket_->ReceivePacket(packet.payload()); } } diff --git a/media/sctp/dcsctp_transport.h b/media/sctp/dcsctp_transport.h index 7ae0d64134..4dfcaeba1e 100644 --- a/media/sctp/dcsctp_transport.h +++ b/media/sctp/dcsctp_transport.h @@ -27,6 +27,7 @@ #include "p2p/base/packet_transport_internal.h" #include "rtc_base/containers/flat_map.h" #include "rtc_base/copy_on_write_buffer.h" +#include "rtc_base/network/received_packet.h" #include "rtc_base/random.h" #include "rtc_base/third_party/sigslot/sigslot.h" #include "rtc_base/thread.h" @@ -94,10 +95,7 @@ class DcSctpTransport : public cricket::SctpTransportInternal, void DisconnectTransportSignals(); void OnTransportWritableState(rtc::PacketTransportInternal* transport); void OnTransportReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t length, - const int64_t& /* packet_time_us */, - int flags); + const rtc::ReceivedPacket& packet); void OnTransportClosed(rtc::PacketTransportInternal* transport); void MaybeConnectSocket(); diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 9d25078bd8..b5cd2c95fc 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -1100,6 +1100,7 @@ if (rtc_include_tests) { "../rtc_base:socket_server", "../rtc_base:ssl", "../rtc_base:threading", + "../rtc_base/network:received_packet", "../rtc_base/third_party/sigslot", "../test:test_support", ] diff --git a/p2p/base/fake_dtls_transport.h b/p2p/base/fake_dtls_transport.h index 5a3db4886b..a088030d52 100644 --- a/p2p/base/fake_dtls_transport.h +++ b/p2p/base/fake_dtls_transport.h @@ -22,6 +22,7 @@ #include "p2p/base/dtls_transport_internal.h" #include "p2p/base/fake_ice_transport.h" #include "rtc_base/fake_ssl_identity.h" +#include "rtc_base/network/received_packet.h" #include "rtc_base/rtc_certificate.h" namespace cricket { @@ -37,8 +38,11 @@ class FakeDtlsTransport : public DtlsTransportInternal { component_(ice_transport->component()), dtls_fingerprint_("", nullptr) { RTC_DCHECK(ice_transport_); - ice_transport_->SignalReadPacket.connect( - this, &FakeDtlsTransport::OnIceTransportReadPacket); + ice_transport_->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnIceTransportReadPacket(transport, packet); + }); ice_transport_->SignalNetworkRouteChanged.connect( this, &FakeDtlsTransport::OnNetworkRouteChanged); } @@ -49,8 +53,11 @@ class FakeDtlsTransport : public DtlsTransportInternal { component_(owned_ice_transport_->component()), dtls_fingerprint_("", rtc::ArrayView()) { ice_transport_ = owned_ice_transport_.get(); - ice_transport_->SignalReadPacket.connect( - this, &FakeDtlsTransport::OnIceTransportReadPacket); + ice_transport_->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnIceTransportReadPacket(transport, packet); + }); ice_transport_->SignalNetworkRouteChanged.connect( this, &FakeDtlsTransport::OnNetworkRouteChanged); } @@ -71,6 +78,7 @@ class FakeDtlsTransport : public DtlsTransportInternal { if (dest_ && dest_->dest_ == this) { dest_->dest_ = nullptr; } + ice_transport_->DeregisterReceivedPacketCallback(this); } // Get inner fake ICE transport. @@ -264,11 +272,8 @@ class FakeDtlsTransport : public DtlsTransportInternal { private: void OnIceTransportReadPacket(PacketTransportInternal* ice_, - const char* data, - size_t len, - const int64_t& packet_time_us, - int flags) { - SignalReadPacket(this, data, len, packet_time_us, flags); + const rtc::ReceivedPacket& packet) { + NotifyPacketReceived(packet); } void set_receiving(bool receiving) { diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 6f21d00bc9..df011ee2fa 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2121,6 +2121,7 @@ if (rtc_include_tests && !build_with_chromium) { "../rtc_base:threading", "../rtc_base:unique_id_generator", "../rtc_base/containers:flat_set", + "../rtc_base/network:received_packet", "../rtc_base/third_party/sigslot", "../system_wrappers:metrics", "../test:explicit_key_value_config", diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 98a61ea673..21a147c78c 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -152,6 +152,19 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { SendTask(network_thread_, [this]() { network_thread_safety_->SetNotAlive(); DeinitChannels(); + + // Transports must be created and destroyed on the network thread. + fake_rtp_dtls_transport1_ = nullptr; + fake_rtcp_dtls_transport1_ = nullptr; + fake_rtp_dtls_transport2_ = nullptr; + fake_rtcp_dtls_transport2_ = nullptr; + fake_rtp_packet_transport1_ = nullptr; + fake_rtcp_packet_transport1_ = nullptr; + fake_rtp_packet_transport2_ = nullptr; + fake_rtcp_packet_transport2_ = nullptr; + rtp_transport1_ = nullptr; + rtp_transport2_ = nullptr; + new_rtp_transport_ = nullptr; }); } } @@ -187,66 +200,70 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { // channels. RTC_DCHECK_EQ(flags1 & RAW_PACKET_TRANSPORT, flags2 & RAW_PACKET_TRANSPORT); rtc::Thread* worker_thread = rtc::Thread::Current(); - // Based on flags, create fake DTLS or raw packet transports. - if (flags1 & RAW_PACKET_TRANSPORT) { - fake_rtp_packet_transport1_.reset( - new rtc::FakePacketTransport("channel1_rtp")); - if (!(flags1 & RTCP_MUX)) { - fake_rtcp_packet_transport1_.reset( - new rtc::FakePacketTransport("channel1_rtcp")); - } - } else { - // Confirmed to work with KT_RSA and KT_ECDSA. - fake_rtp_dtls_transport1_.reset(new cricket::FakeDtlsTransport( - "channel1", cricket::ICE_CANDIDATE_COMPONENT_RTP, network_thread_)); - if (!(flags1 & RTCP_MUX)) { - fake_rtcp_dtls_transport1_.reset(new cricket::FakeDtlsTransport( - "channel1", cricket::ICE_CANDIDATE_COMPONENT_RTCP, - network_thread_)); - } - if (flags1 & DTLS) { - auto cert1 = rtc::RTCCertificate::Create( - rtc::SSLIdentity::Create("session1", rtc::KT_DEFAULT)); - fake_rtp_dtls_transport1_->SetLocalCertificate(cert1); - if (fake_rtcp_dtls_transport1_) { - fake_rtcp_dtls_transport1_->SetLocalCertificate(cert1); + + network_thread_->BlockingCall([&] { + // Based on flags, create fake DTLS or raw packet transports. + + if (flags1 & RAW_PACKET_TRANSPORT) { + fake_rtp_packet_transport1_.reset( + new rtc::FakePacketTransport("channel1_rtp")); + if (!(flags1 & RTCP_MUX)) { + fake_rtcp_packet_transport1_.reset( + new rtc::FakePacketTransport("channel1_rtcp")); + } + } else { + // Confirmed to work with KT_RSA and KT_ECDSA. + fake_rtp_dtls_transport1_.reset(new cricket::FakeDtlsTransport( + "channel1", cricket::ICE_CANDIDATE_COMPONENT_RTP, network_thread_)); + if (!(flags1 & RTCP_MUX)) { + fake_rtcp_dtls_transport1_.reset(new cricket::FakeDtlsTransport( + "channel1", cricket::ICE_CANDIDATE_COMPONENT_RTCP, + network_thread_)); + } + if (flags1 & DTLS) { + auto cert1 = rtc::RTCCertificate::Create( + rtc::SSLIdentity::Create("session1", rtc::KT_DEFAULT)); + fake_rtp_dtls_transport1_->SetLocalCertificate(cert1); + if (fake_rtcp_dtls_transport1_) { + fake_rtcp_dtls_transport1_->SetLocalCertificate(cert1); + } } } - } - // Based on flags, create fake DTLS or raw packet transports. - if (flags2 & RAW_PACKET_TRANSPORT) { - fake_rtp_packet_transport2_.reset( - new rtc::FakePacketTransport("channel2_rtp")); - if (!(flags2 & RTCP_MUX)) { - fake_rtcp_packet_transport2_.reset( - new rtc::FakePacketTransport("channel2_rtcp")); - } - } else { - // Confirmed to work with KT_RSA and KT_ECDSA. - fake_rtp_dtls_transport2_.reset(new cricket::FakeDtlsTransport( - "channel2", cricket::ICE_CANDIDATE_COMPONENT_RTP, network_thread_)); - if (!(flags2 & RTCP_MUX)) { - fake_rtcp_dtls_transport2_.reset(new cricket::FakeDtlsTransport( - "channel2", cricket::ICE_CANDIDATE_COMPONENT_RTCP, - network_thread_)); - } - if (flags2 & DTLS) { - auto cert2 = rtc::RTCCertificate::Create( - rtc::SSLIdentity::Create("session2", rtc::KT_DEFAULT)); - fake_rtp_dtls_transport2_->SetLocalCertificate(cert2); - if (fake_rtcp_dtls_transport2_) { - fake_rtcp_dtls_transport2_->SetLocalCertificate(cert2); + // Based on flags, create fake DTLS or raw packet transports. + if (flags2 & RAW_PACKET_TRANSPORT) { + fake_rtp_packet_transport2_.reset( + new rtc::FakePacketTransport("channel2_rtp")); + if (!(flags2 & RTCP_MUX)) { + fake_rtcp_packet_transport2_.reset( + new rtc::FakePacketTransport("channel2_rtcp")); + } + } else { + // Confirmed to work with KT_RSA and KT_ECDSA. + fake_rtp_dtls_transport2_.reset(new cricket::FakeDtlsTransport( + "channel2", cricket::ICE_CANDIDATE_COMPONENT_RTP, network_thread_)); + if (!(flags2 & RTCP_MUX)) { + fake_rtcp_dtls_transport2_.reset(new cricket::FakeDtlsTransport( + "channel2", cricket::ICE_CANDIDATE_COMPONENT_RTCP, + network_thread_)); + } + if (flags2 & DTLS) { + auto cert2 = rtc::RTCCertificate::Create( + rtc::SSLIdentity::Create("session2", rtc::KT_DEFAULT)); + fake_rtp_dtls_transport2_->SetLocalCertificate(cert2); + if (fake_rtcp_dtls_transport2_) { + fake_rtcp_dtls_transport2_->SetLocalCertificate(cert2); + } } } - } - rtp_transport1_ = CreateRtpTransportBasedOnFlags( - fake_rtp_packet_transport1_.get(), fake_rtcp_packet_transport1_.get(), - fake_rtp_dtls_transport1_.get(), fake_rtcp_dtls_transport1_.get(), - flags1); - rtp_transport2_ = CreateRtpTransportBasedOnFlags( - fake_rtp_packet_transport2_.get(), fake_rtcp_packet_transport2_.get(), - fake_rtp_dtls_transport2_.get(), fake_rtcp_dtls_transport2_.get(), - flags2); + rtp_transport1_ = CreateRtpTransportBasedOnFlags( + fake_rtp_packet_transport1_.get(), fake_rtcp_packet_transport1_.get(), + fake_rtp_dtls_transport1_.get(), fake_rtcp_dtls_transport1_.get(), + flags1); + rtp_transport2_ = CreateRtpTransportBasedOnFlags( + fake_rtp_packet_transport2_.get(), fake_rtcp_packet_transport2_.get(), + fake_rtp_dtls_transport2_.get(), fake_rtcp_dtls_transport2_.get(), + flags2); + }); channel1_ = CreateChannel(worker_thread, network_thread_, std::move(ch1s), std::move(ch1r), rtp_transport1_.get(), flags1); @@ -1351,12 +1368,11 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> { CreateChannels(DTLS, DTLS); - new_rtp_transport_ = CreateDtlsSrtpTransport( - fake_rtp_dtls_transport2_.get(), fake_rtcp_dtls_transport2_.get()); - bool rcv_success, send_success; int rcv_buf, send_buf; SendTask(network_thread_, [&] { + new_rtp_transport_ = CreateDtlsSrtpTransport( + fake_rtp_dtls_transport2_.get(), fake_rtcp_dtls_transport2_.get()); channel1_->SetOption(cricket::BaseChannel::ST_RTP, rtc::Socket::Option::OPT_SNDBUF, kSndBufSize); channel2_->SetOption(cricket::BaseChannel::ST_RTP, diff --git a/pc/jsep_transport_unittest.cc b/pc/jsep_transport_unittest.cc index 18dd2d8896..8112c97788 100644 --- a/pc/jsep_transport_unittest.cc +++ b/pc/jsep_transport_unittest.cc @@ -31,6 +31,7 @@ #include "rtc_base/helpers.h" #include "rtc_base/logging.h" #include "rtc_base/net_helper.h" +#include "rtc_base/network/received_packet.h" #include "rtc_base/socket_address.h" #include "rtc_base/ssl_certificate.h" #include "rtc_base/ssl_identity.h" @@ -1039,42 +1040,44 @@ class JsepTransport2HeaderExtensionTest auto fake_dtls2 = static_cast(jsep_transport2_->rtp_dtls_transport()); - fake_dtls1->fake_ice_transport()->SignalReadPacket.connect( - this, &JsepTransport2HeaderExtensionTest::OnReadPacket1); - fake_dtls2->fake_ice_transport()->SignalReadPacket.connect( - this, &JsepTransport2HeaderExtensionTest::OnReadPacket2); + fake_dtls1->fake_ice_transport()->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnReadPacket1(transport, packet); + }); + fake_dtls2->fake_ice_transport()->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnReadPacket2(transport, packet); + }); - auto cert1 = rtc::RTCCertificate::Create( - rtc::SSLIdentity::Create("session1", rtc::KT_DEFAULT)); - jsep_transport1_->rtp_dtls_transport()->SetLocalCertificate(cert1); - auto cert2 = rtc::RTCCertificate::Create( - rtc::SSLIdentity::Create("session1", rtc::KT_DEFAULT)); - jsep_transport2_->rtp_dtls_transport()->SetLocalCertificate(cert2); + auto cert1 = rtc::RTCCertificate::Create( + rtc::SSLIdentity::Create("session1", rtc::KT_DEFAULT)); + jsep_transport1_->rtp_dtls_transport()->SetLocalCertificate(cert1); + auto cert2 = rtc::RTCCertificate::Create( + rtc::SSLIdentity::Create("session1", rtc::KT_DEFAULT)); + jsep_transport2_->rtp_dtls_transport()->SetLocalCertificate(cert2); } void OnReadPacket1(rtc::PacketTransportInternal* transport, - const char* data, - size_t size, - const int64_t& /* packet_time_us */, - int flags) { + const rtc::ReceivedPacket& packet) { RTC_LOG(LS_INFO) << "JsepTransport 1 Received a packet."; CompareHeaderExtensions( reinterpret_cast(kPcmuFrameWithExtensions), - sizeof(kPcmuFrameWithExtensions), data, size, recv_encrypted_headers1_, - false); + sizeof(kPcmuFrameWithExtensions), + reinterpret_cast(packet.payload().data()), + packet.payload().size(), recv_encrypted_headers1_, false); received_packet_count_++; } void OnReadPacket2(rtc::PacketTransportInternal* transport, - const char* data, - size_t size, - const int64_t& /* packet_time_us */, - int flags) { + const rtc::ReceivedPacket& packet) { RTC_LOG(LS_INFO) << "JsepTransport 2 Received a packet."; CompareHeaderExtensions( reinterpret_cast(kPcmuFrameWithExtensions), - sizeof(kPcmuFrameWithExtensions), data, size, recv_encrypted_headers2_, - false); + sizeof(kPcmuFrameWithExtensions), + reinterpret_cast(packet.payload().data()), + packet.payload().size(), recv_encrypted_headers2_, false); received_packet_count_++; } diff --git a/pc/rtp_transport.cc b/pc/rtp_transport.cc index 7cf9fe0ace..ab8f73a9bb 100644 --- a/pc/rtp_transport.cc +++ b/pc/rtp_transport.cc @@ -54,7 +54,7 @@ void RtpTransport::SetRtpPacketTransport( } if (rtp_packet_transport_) { rtp_packet_transport_->SignalReadyToSend.disconnect(this); - rtp_packet_transport_->SignalReadPacket.disconnect(this); + rtp_packet_transport_->DeregisterReceivedPacketCallback(this); rtp_packet_transport_->SignalNetworkRouteChanged.disconnect(this); rtp_packet_transport_->SignalWritableState.disconnect(this); rtp_packet_transport_->SignalSentPacket.disconnect(this); @@ -64,8 +64,11 @@ void RtpTransport::SetRtpPacketTransport( if (new_packet_transport) { new_packet_transport->SignalReadyToSend.connect( this, &RtpTransport::OnReadyToSend); - new_packet_transport->SignalReadPacket.connect(this, - &RtpTransport::OnReadPacket); + new_packet_transport->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnReadPacket(transport, packet); + }); new_packet_transport->SignalNetworkRouteChanged.connect( this, &RtpTransport::OnNetworkRouteChanged); new_packet_transport->SignalWritableState.connect( @@ -90,7 +93,7 @@ void RtpTransport::SetRtcpPacketTransport( } if (rtcp_packet_transport_) { rtcp_packet_transport_->SignalReadyToSend.disconnect(this); - rtcp_packet_transport_->SignalReadPacket.disconnect(this); + rtcp_packet_transport_->DeregisterReceivedPacketCallback(this); rtcp_packet_transport_->SignalNetworkRouteChanged.disconnect(this); rtcp_packet_transport_->SignalWritableState.disconnect(this); rtcp_packet_transport_->SignalSentPacket.disconnect(this); @@ -100,8 +103,11 @@ void RtpTransport::SetRtcpPacketTransport( if (new_packet_transport) { new_packet_transport->SignalReadyToSend.connect( this, &RtpTransport::OnReadyToSend); - new_packet_transport->SignalReadPacket.connect(this, - &RtpTransport::OnReadPacket); + new_packet_transport->RegisterReceivedPacketCallback( + this, [&](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + OnReadPacket(transport, packet); + }); new_packet_transport->SignalNetworkRouteChanged.connect( this, &RtpTransport::OnNetworkRouteChanged); new_packet_transport->SignalWritableState.connect( @@ -251,30 +257,34 @@ void RtpTransport::OnRtcpPacketReceived(rtc::CopyOnWriteBuffer packet, } void RtpTransport::OnReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t len, - const int64_t& packet_time_us, - int flags) { + const rtc::ReceivedPacket& received_packet) { TRACE_EVENT0("webrtc", "RtpTransport::OnReadPacket"); // When using RTCP multiplexing we might get RTCP packets on the RTP // transport. We check the RTP payload type to determine if it is RTCP. - auto array_view = rtc::MakeArrayView(data, len); - cricket::RtpPacketType packet_type = cricket::InferRtpPacketType(array_view); + cricket::RtpPacketType packet_type = + cricket::InferRtpPacketType(received_packet.payload()); // Filter out the packet that is neither RTP nor RTCP. if (packet_type == cricket::RtpPacketType::kUnknown) { return; } // Protect ourselves against crazy data. - if (!cricket::IsValidRtpPacketSize(packet_type, len)) { + if (!cricket::IsValidRtpPacketSize(packet_type, + received_packet.payload().size())) { RTC_LOG(LS_ERROR) << "Dropping incoming " << cricket::RtpPacketTypeToString(packet_type) - << " packet: wrong size=" << len; + << " packet: wrong size=" + << received_packet.payload().size(); return; } - rtc::CopyOnWriteBuffer packet(data, len); + rtc::CopyOnWriteBuffer packet(received_packet.payload()); + int64_t packet_time_us = received_packet.arrival_time() + ? received_packet.arrival_time()->us() + : -1; + // TODO(bugs.webrtc.org/15368): Propagate timestamp and received packet + // metadata further. if (packet_type == cricket::RtpPacketType::kRtcp) { OnRtcpPacketReceived(std::move(packet), packet_time_us); } else { diff --git a/pc/rtp_transport.h b/pc/rtp_transport.h index 6d5d4bff57..5192543c3d 100644 --- a/pc/rtp_transport.h +++ b/pc/rtp_transport.h @@ -113,10 +113,7 @@ class RtpTransport : public RtpTransportInternal { void OnSentPacket(rtc::PacketTransportInternal* packet_transport, const rtc::SentPacket& sent_packet); void OnReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t len, - const int64_t& packet_time_us, - int flags); + const rtc::ReceivedPacket& received_packet); // Updates "ready to send" for an individual channel and fires // SignalReadyToSend.