Skip to content

Commit 336d14e

Browse files
Add data track tryPush for pointer/size to avoid a copy (#239)
* Adds tryPush(data*, size) API for data tracks * Reduces a copy when forwarding data from another API
1 parent c01ee92 commit 336d14e

3 files changed

Lines changed: 160 additions & 13 deletions

File tree

‎include/livekit/local_data_track.h‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,45 @@ class LIVEKIT_API LocalDataTrack {
6767

6868
/// Try to push a frame to all subscribers of this track.
6969
///
70-
/// @return success on delivery acceptance, or a typed error describing why
70+
/// An empty payload succeeds without sending a frame to subscribers.
71+
///
72+
/// @param frame The frame to push.
73+
/// @return Success on delivery acceptance, or a typed error describing why
7174
/// the frame could not be queued.
7275
Result<void, LocalDataTrackTryPushError> tryPush(const DataTrackFrame& frame);
7376

7477
/// Try to push a frame to all subscribers of this track.
7578
///
76-
/// @return success on delivery acceptance, or a typed error describing why
79+
/// An empty payload succeeds without sending a frame to subscribers.
80+
///
81+
/// @param payload The payload to push.
82+
/// @param user_timestamp Optional application-defined timestamp. The unit is
83+
/// caller-defined.
84+
/// @return Success on delivery acceptance, or a typed error describing why
7785
/// the frame could not be queued.
7886
Result<void, LocalDataTrackTryPushError> tryPush(std::vector<std::uint8_t>&& payload,
7987
std::optional<std::uint64_t> user_timestamp = std::nullopt);
8088

89+
/// Try to push a frame from a borrowed byte buffer.
90+
///
91+
/// Copies @p size bytes from @p data into an FFI request before returning;
92+
/// the SDK does not retain the buffer.
93+
/// An empty payload succeeds without sending a frame to subscribers.
94+
///
95+
/// @note This avoids an intermediate copy when the caller already owns a byte
96+
/// buffer, but it is not a zero-copy send.
97+
///
98+
/// @param data Pointer to @p size payload bytes. Must be non-null when
99+
/// @p size is non-zero.
100+
/// @param size Number of bytes at @p data. A value of zero is a successful
101+
/// no-op.
102+
/// @param user_timestamp Optional application-defined timestamp. The unit is
103+
/// caller-defined.
104+
/// @return Success on delivery acceptance, or a typed error describing why
105+
/// the frame could not be queued.
106+
Result<void, LocalDataTrackTryPushError> tryPush(const std::uint8_t* data, std::size_t size,
107+
std::optional<std::uint64_t> user_timestamp = std::nullopt);
108+
81109
/// Whether the track is still published in the room.
82110
bool isPublished() const;
83111

‎src/local_data_track.cpp‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,37 @@ LocalDataTrack::LocalDataTrack(const proto::OwnedLocalDataTrack& owned)
2929
: handle_(static_cast<uintptr_t>(owned.handle().id())), info_(fromProto(owned.info())) {}
3030

3131
Result<void, LocalDataTrackTryPushError> LocalDataTrack::tryPush(const DataTrackFrame& frame) {
32+
return tryPush(frame.payload.data(), frame.payload.size(), frame.user_timestamp);
33+
}
34+
35+
Result<void, LocalDataTrackTryPushError> LocalDataTrack::tryPush(std::vector<std::uint8_t>&& payload,
36+
std::optional<std::uint64_t> user_timestamp) {
37+
const DataTrackFrame frame(std::move(payload), user_timestamp);
38+
return tryPush(frame);
39+
}
40+
41+
Result<void, LocalDataTrackTryPushError> LocalDataTrack::tryPush(const std::uint8_t* data, std::size_t size,
42+
std::optional<std::uint64_t> user_timestamp) {
3243
if (!handle_.valid()) {
3344
return Result<void, LocalDataTrackTryPushError>::failure(LocalDataTrackTryPushError{
3445
LocalDataTrackTryPushErrorCode::INVALID_HANDLE, "LocalDataTrack::tryPush: invalid FFI handle"});
3546
}
47+
if (size == 0) {
48+
return Result<void, LocalDataTrackTryPushError>::success();
49+
}
50+
if (data == nullptr) {
51+
return Result<void, LocalDataTrackTryPushError>::failure(LocalDataTrackTryPushError{
52+
LocalDataTrackTryPushErrorCode::INTERNAL, "LocalDataTrack::tryPush: payload pointer is null"});
53+
}
3654

3755
try {
3856
proto::FfiRequest req;
3957
auto* msg = req.mutable_local_data_track_try_push();
4058
msg->set_track_handle(static_cast<uint64_t>(handle_.get()));
4159
auto* pf = msg->mutable_frame();
42-
pf->set_payload(frame.payload.data(), frame.payload.size());
43-
if (frame.user_timestamp.has_value()) {
44-
pf->set_user_timestamp(frame.user_timestamp.value());
60+
pf->set_payload(data, size);
61+
if (user_timestamp.has_value()) {
62+
pf->set_user_timestamp(user_timestamp.value());
4563
}
4664

4765
const proto::FfiResponse resp = FfiClient::instance().sendRequest(req);
@@ -56,14 +74,6 @@ Result<void, LocalDataTrackTryPushError> LocalDataTrack::tryPush(const DataTrack
5674
}
5775
}
5876

59-
Result<void, LocalDataTrackTryPushError> LocalDataTrack::tryPush(std::vector<std::uint8_t>&& payload,
60-
std::optional<std::uint64_t> user_timestamp) {
61-
DataTrackFrame frame;
62-
frame.payload = std::move(payload);
63-
frame.user_timestamp = user_timestamp;
64-
return tryPush(frame);
65-
}
66-
6777
bool LocalDataTrack::isPublished() const {
6878
if (!handle_.valid()) {
6979
return false;

‎src/tests/integration/test_data_track.cpp‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,115 @@ TEST_F(DataTrackE2ETest, PreservesUserTimestampEndToEnd) {
955955
EXPECT_EQ(frame.user_timestamp.value(), sent_timestamp);
956956
}
957957

958+
TEST_F(DataTrackE2ETest, RejectsNullBorrowedPayloadWithNonZeroSize) {
959+
const auto track_name = makeTrackName("null_borrowed_payload");
960+
auto rooms = testRooms(1);
961+
auto local_track = requirePublishedTrack(rooms[0]->localParticipant(), track_name);
962+
963+
const auto push_result = local_track->tryPush(static_cast<const std::uint8_t*>(nullptr), 4);
964+
ASSERT_FALSE(push_result);
965+
EXPECT_EQ(push_result.error().code, LocalDataTrackTryPushErrorCode::INTERNAL);
966+
EXPECT_FALSE(push_result.error().message.empty());
967+
968+
local_track->unpublishDataTrack();
969+
}
970+
971+
TEST_F(DataTrackE2ETest, AcceptsEmptyPayload) {
972+
const auto track_name = makeTrackName("empty_borrowed_payload");
973+
auto rooms = testRooms(1);
974+
auto local_track = requirePublishedTrack(rooms[0]->localParticipant(), track_name);
975+
976+
const std::uint8_t payload = 0;
977+
EXPECT_TRUE(local_track->tryPush(&payload, 0));
978+
EXPECT_TRUE(local_track->tryPush(nullptr, 0));
979+
EXPECT_TRUE(local_track->tryPush(std::vector<std::uint8_t>{}));
980+
981+
local_track->unpublishDataTrack();
982+
}
983+
984+
TEST_F(DataTrackE2ETest, CopiesBorrowedPayloadBeforeTryPushReturns) {
985+
const auto track_name = makeTrackName("borrowed_payload");
986+
const auto sent_timestamp = getTimestampUs();
987+
constexpr std::uint8_t kOriginal = 0xA5;
988+
constexpr std::uint8_t kMutated = 0x5A;
989+
constexpr std::size_t kPayloadSize = 64;
990+
991+
DataTrackPublishedDelegate subscriber_delegate;
992+
std::vector<TestRoomConnectionOptions> room_configs(2);
993+
room_configs[1].delegate = &subscriber_delegate;
994+
995+
auto rooms = testRooms(room_configs);
996+
auto& publisher_room = rooms[0];
997+
998+
auto publish_result = lockLocalParticipant(*publisher_room)->publishDataTrack(track_name);
999+
if (!publish_result) {
1000+
FAIL() << describeDataTrackError(publish_result.error());
1001+
}
1002+
auto local_track = publish_result.value();
1003+
ASSERT_TRUE(local_track->isPublished());
1004+
1005+
auto remote_track = subscriber_delegate.waitForTrack(kTrackWaitTimeout);
1006+
ASSERT_NE(remote_track, nullptr) << "Timed out waiting for remote data track";
1007+
1008+
auto subscribe_result = remote_track->subscribe();
1009+
if (!subscribe_result) {
1010+
FAIL() << describeDataTrackError(subscribe_result.error());
1011+
}
1012+
auto subscription = subscribe_result.value();
1013+
1014+
std::promise<DataTrackFrame> frame_promise;
1015+
auto frame_future = frame_promise.get_future();
1016+
std::thread reader([&]() {
1017+
try {
1018+
DataTrackFrame frame;
1019+
if (!subscription->read(frame)) {
1020+
throw std::runtime_error("Subscription ended before borrowed-payload frame arrived");
1021+
}
1022+
frame_promise.set_value(std::move(frame));
1023+
} catch (...) {
1024+
frame_promise.set_exception(std::current_exception());
1025+
}
1026+
});
1027+
1028+
bool pushed = false;
1029+
for (int attempt = 0; attempt < kTimestampFrameAttempts; ++attempt) {
1030+
std::array<std::uint8_t, kPayloadSize> payload{};
1031+
payload.fill(kOriginal);
1032+
auto push_result = local_track->tryPush(payload.data(), payload.size(), sent_timestamp);
1033+
payload.fill(kMutated);
1034+
pushed = static_cast<bool>(push_result) || pushed;
1035+
if (frame_future.wait_for(25ms) == std::future_status::ready) {
1036+
break;
1037+
}
1038+
}
1039+
const auto frame_status = frame_future.wait_for(5s);
1040+
1041+
if (frame_status != std::future_status::ready) {
1042+
subscription->close();
1043+
}
1044+
1045+
subscription->close();
1046+
reader.join();
1047+
local_track->unpublishDataTrack();
1048+
1049+
ASSERT_TRUE(pushed) << "Failed to push borrowed data frame";
1050+
ASSERT_EQ(frame_status, std::future_status::ready) << "Timed out waiting for borrowed-payload frame";
1051+
1052+
DataTrackFrame frame;
1053+
try {
1054+
frame = frame_future.get();
1055+
} catch (const std::exception& e) {
1056+
FAIL() << e.what();
1057+
}
1058+
1059+
ASSERT_EQ(frame.payload.size(), kPayloadSize);
1060+
EXPECT_TRUE(std::all_of(frame.payload.begin(), frame.payload.end(), [expected = kOriginal](std::uint8_t byte) {
1061+
return byte == expected;
1062+
})) << "Received payload reflects caller mutation after tryPush returned";
1063+
ASSERT_TRUE(frame.user_timestamp.has_value());
1064+
EXPECT_EQ(frame.user_timestamp.value(), sent_timestamp);
1065+
}
1066+
9581067
TEST_F(DataTrackE2ETest, PublishesAndReceivesEncryptedFramesEndToEnd) {
9591068
runEncryptedDataTrackRoundTrip(kDefaultKeyDerivationFunction, "e2ee_transport");
9601069
}

0 commit comments

Comments
 (0)