Sderosa/data tracks throughput

[stephen-derosa]

benchmark/data_track_throughput: throughput executables and post processing tooling for visualizing Data Track MBPS throughput

[Copilot]

Pull request overview

Adds a data-track throughput benchmarking experiment (producer/consumer executables + plotting tooling) and updates repo docs/ignores to support running it.

Changes:

• Add benchmarks/data_track_throughput standalone CMake project with producer/consumer executables that coordinate via RPC and write raw CSV measurements.
• Add Python post-processing script to generate throughput/latency plots from the CSV output.
• Update documentation and .gitignore entries for Docker build notes and benchmark output directories.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
include/livekit/data_track_subscription.h	Introduces a new public header for a blocking data-track subscription type.
benchmarks/data_track_throughput/producer.cpp	Producer benchmark executable that publishes a data track and runs scenario sweeps/single scenario.
benchmarks/data_track_throughput/consumer.cpp	Consumer benchmark executable that records arrivals, validates headers, and writes CSVs.
benchmarks/data_track_throughput/common.h	Shared scenario planning, payload/header encoding, CSV helpers, and validation.
benchmarks/data_track_throughput/scripts/plot_throughput.py	CSV ingestion + derived metrics + plot generation.
benchmarks/data_track_throughput/scripts/.gitignore	Ignores venv/pycache artifacts for the plotting scripts.
benchmarks/data_track_throughput/README.md	Documents benchmark design, bounds, and usage/plot outputs.
benchmarks/data_track_throughput/CMakeLists.txt	Standalone build for benchmark + dependency fetching and library copying.
README.md	Updates Docker build instructions to use base + sdk Dockerfiles.
.gitignore	Ignores benchmark output directory data_track_throughput_results/.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The block comment for frame_ is not well-formed (the /** opener isn't followed by *-prefixed lines and the closing */ is attached to text), and it also uses inconsistent capitalization ("rust" vs "Rust"). This can break generated docs / tooling; please rewrite it as a proper multi-line doc comment.

Suggested change

	/** Received frame awaiting read().
	NOTE: the rust side handles buffering, so we should only really ever have one
	item*/
	/**
	* Received frame awaiting read().
	* NOTE: the Rust side handles buffering, so we should only really ever have
	* one item.
	*/

[Copilot]

finalizeRun() waits on cv_, but onDataFrame() never calls cv_.notify_*() after pushing a new observation. This forces finalizeRun() to rely on the 100ms polling timeout, increasing run finalization latency and potentially delaying shutdown. Consider notifying the condition variable after appending a message (and after marking producer_finished) so the waiter can re-check completion immediately.

Suggested change

	run.observations.push_back(std::move(observation));
	run.observations.push_back(std::move(observation));
	cv_.notify_all();

[Copilot]

parseArgs() can throw (e.g., std::stoll / std::stod on malformed or missing flag values), but unlike producer.cpp, main() here doesn't wrap argument parsing in a try/catch to print usage and exit cleanly. Consider mirroring the producer’s pattern so invalid CLI input doesn’t terminate the process via an uncaught exception.

Suggested change

	if (!parseArgs(argc, argv, options)) {
	try {
	if (!parseArgs(argc, argv, options)) {
	printUsage(argv[0]);
	return 1;
	}
	} catch (const std::exception &e) {
	std::cerr << e.what() << std::endl;

[Copilot]

Using get_target_property(... LOCATION) is deprecated/disallowed under newer CMake policies (and can produce warnings/errors for imported targets). Since you already fall back to IMPORTED_LOCATION*, consider dropping the LOCATION lookup entirely and/or using the imported location properties consistently to avoid policy/CMake-version sensitivity.

Suggested change

	get_target_property(_lk_location LiveKit::livekit LOCATION)
	if(_lk_location)
	get_filename_component(_lk_lib_dir "${_lk_location}" DIRECTORY)
	else()
	get_target_property(_lk_location LiveKit::livekit IMPORTED_LOCATION)
	if(NOT _lk_location)
	get_target_property(_lk_location LiveKit::livekit IMPORTED_LOCATION_RELEASE)
	endif()
	if(NOT _lk_location)
	get_target_property(_lk_location LiveKit::livekit IMPORTED_LOCATION_DEBUG)
	endif()
	if(_lk_location)
	get_filename_component(_lk_lib_dir "${_lk_location}" DIRECTORY)
	endif()
	get_target_property(_lk_location LiveKit::livekit IMPORTED_LOCATION)
	if(NOT _lk_location)
	get_target_property(_lk_location LiveKit::livekit IMPORTED_LOCATION_RELEASE)
	endif()
	if(NOT _lk_location)
	get_target_property(_lk_location LiveKit::livekit IMPORTED_LOCATION_DEBUG)
	endif()
	if(_lk_location)
	get_filename_component(_lk_lib_dir "${_lk_location}" DIRECTORY)

[Copilot]

This section is duplicated verbatim (the "The budget clips larger payloads..." paragraph and the payload/rate table appear twice). Please remove the duplicate copy to avoid confusion and keep the README concise.

Suggested change

	The budget clips larger payloads to lower rates. For example:
	| Payload | Max rate allowed |
	|---------|-----------------|
	| 1 KiB | 50k Hz (all rates) |
	| 16 KiB | 50k Hz (all rates) |
	| 64 KiB | 10k Hz |
	| 256 KiB | 1k Hz |
	| 1 MiB | 1k Hz |
	| 4 MiB | 200 Hz |
	| 64 MiB | 10 Hz |
	| 256 MiB | 1 Hz |

[Copilot]

#include "livekit/data_frame.h" and the DataFrame type used throughout this header don't appear to exist in this codebase (there is include/livekit/data_track_frame.h / DataTrackFrame and DataTrackStream instead). As-is, any consumer including this public header will fail to compile. Either update this header to include/use the existing data-track frame types (and add the corresponding .cpp implementation), or remove the unused header if it was added accidentally.