Introduce PacketTransportInternal::NotifyPacketReceived
Plan is to replace SignalReadPacket with this new method and have clients register as listeners.
rtc::ReceivedPacket is ammended with Descryption information so that receivers can know how to treat a received packet.
This will replace the current "flag". Also see https://webrtc-review.googlesource.com/c/src/+/340301/3
Bug: webrtc:15368
Change-Id: I9ea1f34e8b1e923d67c2e92e36a22b3dd10dbd73
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/340181
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41795}
diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn
index 1601b49..657e73d 100644
--- a/p2p/BUILD.gn
+++ b/p2p/BUILD.gn
@@ -628,9 +628,12 @@
deps = [
":connection",
":port",
+ "../api:sequence_checker",
"../rtc_base:async_packet_socket",
+ "../rtc_base:callback_list",
"../rtc_base:network_route",
"../rtc_base:socket",
+ "../rtc_base/network:received_packet",
"../rtc_base/system:rtc_export",
"../rtc_base/third_party/sigslot",
]
@@ -1015,6 +1018,8 @@
"../api/units:time_delta",
"../rtc_base:copy_on_write_buffer",
"../rtc_base:task_queue_for_test",
+ "../rtc_base:timeutils",
+ "../rtc_base/network:received_packet",
]
absl_deps = [
"//third_party/abseil-cpp/absl/algorithm:container",
@@ -1107,6 +1112,7 @@
"base/dtls_transport_unittest.cc",
"base/ice_credentials_iterator_unittest.cc",
"base/p2p_transport_channel_unittest.cc",
+ "base/packet_transport_internal_unittest.cc",
"base/port_allocator_unittest.cc",
"base/port_unittest.cc",
"base/pseudo_tcp_unittest.cc",
diff --git a/p2p/base/fake_ice_transport.h b/p2p/base/fake_ice_transport.h
index 6172ebb..285bfff 100644
--- a/p2p/base/fake_ice_transport.h
+++ b/p2p/base/fake_ice_transport.h
@@ -24,7 +24,9 @@
#include "api/units/time_delta.h"
#include "p2p/base/ice_transport_internal.h"
#include "rtc_base/copy_on_write_buffer.h"
+#include "rtc_base/network/received_packet.h"
#include "rtc_base/task_queue_for_test.h"
+#include "rtc_base/time_utils.h"
namespace cricket {
using ::webrtc::SafeTask;
@@ -391,8 +393,8 @@
RTC_EXCLUSIVE_LOCKS_REQUIRED(network_thread_) {
if (dest_) {
last_sent_packet_ = packet;
- dest_->SignalReadPacket(dest_, packet.data<char>(), packet.size(),
- rtc::TimeMicros(), 0);
+ dest_->NotifyPacketReceived(rtc::ReceivedPacket::CreateFromLegacy(
+ packet.data(), packet.size(), rtc::TimeMicros()));
}
}
diff --git a/p2p/base/fake_packet_transport.h b/p2p/base/fake_packet_transport.h
index e80af0e..29e9bc7 100644
--- a/p2p/base/fake_packet_transport.h
+++ b/p2p/base/fake_packet_transport.h
@@ -98,6 +98,8 @@
SignalNetworkRouteChanged(network_route);
}
+ using PacketTransportInternal::NotifyPacketReceived;
+
private:
void set_writable(bool writable) {
if (writable_ == writable) {
@@ -121,8 +123,8 @@
void SendPacketInternal(const CopyOnWriteBuffer& packet) {
last_sent_packet_ = packet;
if (dest_) {
- dest_->SignalReadPacket(dest_, packet.data<char>(), packet.size(),
- TimeMicros(), 0);
+ dest_->NotifyPacketReceived(rtc::ReceivedPacket::CreateFromLegacy(
+ packet.data(), packet.size(), rtc::TimeMicros()));
}
}
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index b0e19f6..e3ac48c 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -2211,17 +2211,14 @@
last_data_received_ms_ =
std::max(last_data_received_ms_, connection->last_data_received());
- SignalReadPacket(
- this, reinterpret_cast<const char*>(packet.payload().data()),
- packet.payload().size(),
- packet.arrival_time() ? packet.arrival_time()->us() : -1, 0);
+ NotifyPacketReceived(packet);
- // May need to switch the sending connection based on the receiving media
- // path if this is the controlled side.
- if (ice_role_ == ICEROLE_CONTROLLED && connection != selected_connection_) {
- ice_controller_->OnImmediateSwitchRequest(IceSwitchReason::DATA_RECEIVED,
- connection);
- }
+ // May need to switch the sending connection based on the receiving media
+ // path if this is the controlled side.
+ if (ice_role_ == ICEROLE_CONTROLLED && connection != selected_connection_) {
+ ice_controller_->OnImmediateSwitchRequest(IceSwitchReason::DATA_RECEIVED,
+ connection);
+ }
}
void P2PTransportChannel::OnSentPacket(const rtc::SentPacket& sent_packet) {
diff --git a/p2p/base/packet_transport_internal.cc b/p2p/base/packet_transport_internal.cc
index 0904cb2..2e8c00a 100644
--- a/p2p/base/packet_transport_internal.cc
+++ b/p2p/base/packet_transport_internal.cc
@@ -10,6 +10,9 @@
#include "p2p/base/packet_transport_internal.h"
+#include "api/sequence_checker.h"
+#include "rtc_base/network/received_packet.h"
+
namespace rtc {
PacketTransportInternal::PacketTransportInternal() = default;
@@ -24,4 +27,23 @@
return absl::optional<NetworkRoute>();
}
+void PacketTransportInternal::NotifyPacketReceived(
+ const rtc::ReceivedPacket& packet) {
+ RTC_DCHECK_RUN_ON(&network_checker_);
+ if (!SignalReadPacket.is_empty()) {
+ // TODO(bugs.webrtc.org:15368): Replace with
+ // received_packet_callbacklist_.
+ int flags = 0;
+ if (packet.decryption_info() == rtc::ReceivedPacket::kSrtpEncrypted) {
+ flags = 1;
+ }
+ SignalReadPacket(
+ this, reinterpret_cast<const char*>(packet.payload().data()),
+ packet.payload().size(),
+ packet.arrival_time() ? packet.arrival_time()->us() : -1, flags);
+ } else {
+ received_packet_callback_list_.Send(this, packet);
+ }
+}
+
} // namespace rtc
diff --git a/p2p/base/packet_transport_internal.h b/p2p/base/packet_transport_internal.h
index 2ca47d5..981554a 100644
--- a/p2p/base/packet_transport_internal.h
+++ b/p2p/base/packet_transport_internal.h
@@ -12,11 +12,14 @@
#define P2P_BASE_PACKET_TRANSPORT_INTERNAL_H_
#include <string>
+#include <utility>
#include <vector>
#include "absl/types/optional.h"
#include "p2p/base/port.h"
#include "rtc_base/async_packet_socket.h"
+#include "rtc_base/callback_list.h"
+#include "rtc_base/network/received_packet.h"
#include "rtc_base/network_route.h"
#include "rtc_base/socket.h"
#include "rtc_base/system/rtc_export.h"
@@ -78,7 +81,19 @@
// Emitted when receiving state changes to true.
sigslot::signal1<PacketTransportInternal*> SignalReceivingState;
+ template <typename F>
+ void RegisterReceivedPacketCallback(void* id, F&& callback) {
+ RTC_DCHECK_RUN_ON(&network_checker_);
+ received_packet_callback_list_.AddReceiver(id, std::forward<F>(callback));
+ }
+ void DeregisterReceivedPacketCallback(void* id) {
+ RTC_DCHECK_RUN_ON(&network_checker_);
+ received_packet_callback_list_.RemoveReceivers(id);
+ }
+
// Signalled each time a packet is received on this channel.
+ // TODO(bugs.webrtc.org:15368): Deprecate and remove. Replace with
+ // RegisterReceivedPacketCallback.
sigslot::signal5<PacketTransportInternal*,
const char*,
size_t,
@@ -101,6 +116,14 @@
protected:
PacketTransportInternal();
~PacketTransportInternal() override;
+
+ void NotifyPacketReceived(const rtc::ReceivedPacket& packet);
+
+ webrtc::SequenceChecker network_checker_{webrtc::SequenceChecker::kDetached};
+
+ private:
+ webrtc::CallbackList<PacketTransportInternal*, const rtc::ReceivedPacket&>
+ received_packet_callback_list_ RTC_GUARDED_BY(&network_checker_);
};
} // namespace rtc
diff --git a/p2p/base/packet_transport_internal_unittest.cc b/p2p/base/packet_transport_internal_unittest.cc
new file mode 100644
index 0000000..f17e43f
--- /dev/null
+++ b/p2p/base/packet_transport_internal_unittest.cc
@@ -0,0 +1,101 @@
+/*
+ * Copyright 2024 The WebRTC Project Authors. All rights reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "p2p/base/packet_transport_internal.h"
+
+#include "p2p/base/fake_packet_transport.h"
+#include "rtc_base/gunit.h"
+#include "rtc_base/network/received_packet.h"
+#include "rtc_base/third_party/sigslot/sigslot.h"
+#include "test/gmock.h"
+
+namespace {
+
+using ::testing::MockFunction;
+
+class SigslotPacketReceiver : public sigslot::has_slots<> {
+ public:
+ bool packet_received() const { return packet_received_; }
+
+ void OnPacketReceived(rtc::PacketTransportInternal*,
+ const char*,
+ size_t,
+ const int64_t&,
+ int flags) {
+ packet_received_ = true;
+ flags_ = flags;
+ }
+
+ bool packet_received_ = false;
+ int flags_ = -1;
+};
+
+TEST(PacketTransportInternal,
+ PacketFlagsCorrectWithUnDecryptedPacketUsingSigslot) {
+ rtc::FakePacketTransport packet_transport("test");
+ SigslotPacketReceiver receiver;
+ packet_transport.SignalReadPacket.connect(
+ &receiver, &SigslotPacketReceiver::OnPacketReceived);
+
+ packet_transport.NotifyPacketReceived(
+ rtc::ReceivedPacket({}, rtc::SocketAddress(), absl::nullopt,
+ rtc::ReceivedPacket::kNotDecrypted));
+ ASSERT_TRUE(receiver.packet_received_);
+ EXPECT_EQ(receiver.flags_, 0);
+}
+
+TEST(PacketTransportInternal, PacketFlagsCorrectWithSrtpPacketUsingSigslot) {
+ rtc::FakePacketTransport packet_transport("test");
+ SigslotPacketReceiver receiver;
+ packet_transport.SignalReadPacket.connect(
+ &receiver, &SigslotPacketReceiver::OnPacketReceived);
+
+ packet_transport.NotifyPacketReceived(
+ rtc::ReceivedPacket({}, rtc::SocketAddress(), absl::nullopt,
+ rtc::ReceivedPacket::kSrtpEncrypted));
+ ASSERT_TRUE(receiver.packet_received_);
+ EXPECT_EQ(receiver.flags_, 1);
+}
+
+TEST(PacketTransportInternal, PacketFlagsCorrectWithDtlsDecryptedUsingSigslot) {
+ rtc::FakePacketTransport packet_transport("test");
+ SigslotPacketReceiver receiver;
+ packet_transport.SignalReadPacket.connect(
+ &receiver, &SigslotPacketReceiver::OnPacketReceived);
+
+ packet_transport.NotifyPacketReceived(
+ rtc::ReceivedPacket({}, rtc::SocketAddress(), absl::nullopt,
+ rtc::ReceivedPacket::kDtlsDecrypted));
+ ASSERT_TRUE(receiver.packet_received_);
+ EXPECT_EQ(receiver.flags_, 0);
+}
+
+TEST(PacketTransportInternal,
+ NotifyPacketReceivedPassThrougPacketToRegisterListener) {
+ rtc::FakePacketTransport packet_transport("test");
+ MockFunction<void(rtc::PacketTransportInternal*, const rtc::ReceivedPacket&)>
+ receiver;
+
+ packet_transport.RegisterReceivedPacketCallback(&receiver,
+ receiver.AsStdFunction());
+ EXPECT_CALL(receiver, Call)
+ .WillOnce(
+ [](rtc::PacketTransportInternal*, const rtc::ReceivedPacket& packet) {
+ EXPECT_EQ(packet.decryption_info(),
+ rtc::ReceivedPacket::kDtlsDecrypted);
+ });
+ packet_transport.NotifyPacketReceived(
+ rtc::ReceivedPacket({}, rtc::SocketAddress(), absl::nullopt,
+ rtc::ReceivedPacket::kDtlsDecrypted));
+
+ packet_transport.DeregisterReceivedPacketCallback(&receiver);
+}
+
+} // namespace
diff --git a/rtc_base/network/received_packet.cc b/rtc_base/network/received_packet.cc
index 95f5e22..bf8a07c 100644
--- a/rtc_base/network/received_packet.cc
+++ b/rtc_base/network/received_packet.cc
@@ -13,15 +13,24 @@
#include <utility>
#include "absl/types/optional.h"
+#include "rtc_base/socket_address.h"
namespace rtc {
ReceivedPacket::ReceivedPacket(rtc::ArrayView<const uint8_t> payload,
const SocketAddress& source_address,
- absl::optional<webrtc::Timestamp> arrival_time)
+ absl::optional<webrtc::Timestamp> arrival_time,
+ DecryptionInfo decryption)
: payload_(payload),
arrival_time_(std::move(arrival_time)),
- source_address_(source_address) {}
+ source_address_(source_address),
+ decryption_info_(decryption) {}
+
+ReceivedPacket ReceivedPacket::CopyAndSet(
+ DecryptionInfo decryption_info) const {
+ return ReceivedPacket(payload_, source_address_, arrival_time_,
+ decryption_info);
+}
// static
ReceivedPacket ReceivedPacket::CreateFromLegacy(
diff --git a/rtc_base/network/received_packet.h b/rtc_base/network/received_packet.h
index d898ccb..b5a6092 100644
--- a/rtc_base/network/received_packet.h
+++ b/rtc_base/network/received_packet.h
@@ -26,12 +26,21 @@
// example it may contains STUN, SCTP, SRTP, RTP, RTCP.... etc.
class RTC_EXPORT ReceivedPacket {
public:
+ enum DecryptionInfo {
+ kNotDecrypted, // Payload has not yet been decrypted or encryption is not
+ // used.
+ kDtlsDecrypted, // Payload has been Dtls decrypted
+ kSrtpEncrypted // Payload is SRTP encrypted.
+ };
+
// Caller must keep memory pointed to by payload and address valid for the
// lifetime of this ReceivedPacket.
- ReceivedPacket(
- rtc::ArrayView<const uint8_t> payload,
- const SocketAddress& source_address,
- absl::optional<webrtc::Timestamp> arrival_time = absl::nullopt);
+ ReceivedPacket(rtc::ArrayView<const uint8_t> payload,
+ const SocketAddress& source_address,
+ absl::optional<webrtc::Timestamp> arrival_time = absl::nullopt,
+ DecryptionInfo decryption = kNotDecrypted);
+
+ ReceivedPacket CopyAndSet(DecryptionInfo decryption_info) const;
// Address/port of the packet sender.
const SocketAddress& source_address() const { return source_address_; }
@@ -43,6 +52,8 @@
return arrival_time_;
}
+ const DecryptionInfo& decryption_info() const { return decryption_info_; }
+
static ReceivedPacket CreateFromLegacy(
const char* data,
size_t size,
@@ -62,6 +73,7 @@
rtc::ArrayView<const uint8_t> payload_;
absl::optional<webrtc::Timestamp> arrival_time_;
const SocketAddress& source_address_;
+ DecryptionInfo decryption_info_;
};
} // namespace rtc
