Skip to content

RSDK-13297 Update latency param for audio playback #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
oliviamiller wants to merge 3 commits into
base: main
Choose a base branch
from
Open

RSDK-13297 Update latency param for audio playback #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
446d2ea
fix latency issue
oliviamiller Mar 9, 2026
16ced6f
lint
oliviamiller Mar 9, 2026
0eddf09
mic set after restart
oliviamiller Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions src/audio_utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ struct StreamParams {
std::string device_name;
int sample_rate;
int num_channels;
double latency_seconds;
// Hint passed to PortAudio when opening the stream. PortAudio may choose the closest
// viable latency instead, rounding up to the next practical value.
// Use get_stream_latency() after opening the stream to retrieve the actual latency PortAudio selected.
double suggested_latency_seconds;
bool is_input;
PaStreamCallback* callback;
void* user_data; // Points to AudioStreamContext* or PlaybackBuffer*
Expand Down Expand Up @@ -171,9 +174,9 @@ inline StreamParams setupStreamFromConfig(const ConfigParams& params,
const double default_latency =
(direction == StreamDirection::Input) ? deviceInfo->defaultLowInputLatency : deviceInfo->defaultLowOutputLatency;

stream_params.latency_seconds = params.latency_ms.has_value() ? params.latency_ms.value() / 1000.0 : default_latency;
stream_params.suggested_latency_seconds = params.latency_ms.has_value() ? params.latency_ms.value() / 1000.0 : default_latency;

VIAM_SDK_LOG(debug) << "[setupStreamFromConfig] Using latency " << stream_params.latency_seconds;
VIAM_SDK_LOG(debug) << "[setupStreamFromConfig] Using latency " << stream_params.suggested_latency_seconds;

// Validate num_channels against device's max channels
const int max_channels = (direction == StreamDirection::Input) ? deviceInfo->maxInputChannels : deviceInfo->maxOutputChannels;
Expand Down Expand Up @@ -202,7 +205,7 @@ inline void openStream(PaStream*& stream, const StreamParams& params, const audi
stream_params.device = params.device_index;
stream_params.channelCount = params.num_channels;
stream_params.sampleFormat = paInt16;
stream_params.suggestedLatency = params.latency_seconds;
stream_params.suggestedLatency = params.suggested_latency_seconds;
stream_params.hostApiSpecificStreamInfo = nullptr;

// Determine which parameter is input and which is output
Expand All @@ -218,7 +221,7 @@ inline void openStream(PaStream*& stream, const StreamParams& params, const audi
<< " - Sample rate: " << params.sample_rate << " Hz\n"
<< " - Channels: " << params.num_channels << "\n"
<< " - Format: 16-bit PCM\n"
<< " - Latency: " << params.latency_seconds << " seconds";
<< " - Latency: " << params.suggested_latency_seconds << " seconds";
VIAM_SDK_LOG(error) << buffer.str();
throw std::runtime_error(buffer.str());
}
Expand Down Expand Up @@ -280,6 +283,18 @@ inline void shutdown_stream(PaStream* stream, const audio::portaudio::PortAudioI
}
}

// Returns the actual latency reported by PortAudio after stream open (inputLatency for
// input streams, outputLatency for output streams). Falls back to suggested_latency_seconds
// if stream info is unavailable.
inline double get_stream_latency(PaStream* stream, const StreamParams& params, const audio::portaudio::PortAudioInterface* pa = nullptr) {
Copy link

@seanavery seanavery Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[question] why are audio_utils inline fns in a header file instead of breaking out into a cpp file?

Copy link
Collaborator Author

@oliviamiller oliviamiller Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the other functions in this file are templates which must be defined in a header file. This function could be moved to a .cpp file but I dont think its necessary rn.

audio::portaudio::RealPortAudio real_pa;
const audio::portaudio::PortAudioInterface& audio_interface = pa ? *pa : real_pa;
const PaStreamInfo* stream_info = audio_interface.getStreamInfo(stream);
Copy link

@seanavery seanavery Mar 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Do we need null check on the stream arg?

Copy link
Collaborator Author

@oliviamiller oliviamiller Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not neccessary since in in pa_getstreaminfo docs:
If the stream parameter is invalid, or an error is encountered, the function returns NULL.
so its covered by check below.

if (!stream_info)
return params.suggested_latency_seconds;
return params.is_input ? stream_info->inputLatency : stream_info->outputLatency;
}

inline void restart_stream(PaStream*& stream, const StreamParams& params, const audio::portaudio::PortAudioInterface* pa = nullptr) {
// In production pa is nullptr and real_pa is used. For testing, pa is the mock pa
audio::portaudio::RealPortAudio real_pa;
Expand Down
4 changes: 2 additions & 2 deletions src/microphone.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@ Microphone::Microphone(viam::sdk::Dependencies deps, viam::sdk::ResourceConfig c
requested_sample_rate_ =
setup.config_params.sample_rate.value_or(setup.stream_params.sample_rate); // User's requested rate, defaults to device rate
num_channels_ = setup.stream_params.num_channels;
latency_ = setup.stream_params.latency_seconds;
audio_context_ = setup.audio_context;
historical_throttle_ms_ = setup.config_params.historical_throttle_ms.value_or(DEFAULT_HISTORICAL_THROTTLE_MS);

audio::utils::restart_stream(stream_, setup.stream_params, pa_);
latency_ = audio::utils::get_stream_latency(stream_, setup.stream_params, pa_);
}
}

Expand Down Expand Up @@ -275,7 +275,7 @@ void Microphone::reconfigure(const viam::sdk::Dependencies& deps, const viam::sd
requested_sample_rate_ = setup.config_params.sample_rate.value_or(
setup.stream_params.sample_rate); // User's requested rate, defaults to device rate
num_channels_ = setup.stream_params.num_channels;
latency_ = setup.stream_params.latency_seconds;
latency_ = audio::utils::get_stream_latency(stream_, setup.stream_params, pa_);
audio_context_ = setup.audio_context;
historical_throttle_ms_ = setup.config_params.historical_throttle_ms.value_or(DEFAULT_HISTORICAL_THROTTLE_MS);
}
Expand Down
4 changes: 2 additions & 2 deletions src/speaker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ Speaker::Speaker(viam::sdk::Dependencies deps, viam::sdk::ResourceConfig cfg, au
device_name_ = setup.stream_params.device_name;
sample_rate_ = setup.stream_params.sample_rate;
num_channels_ = setup.stream_params.num_channels;
latency_ = setup.stream_params.latency_seconds;
audio_context_ = setup.audio_context;
audio::utils::restart_stream(stream_, setup.stream_params, pa_);
latency_ = audio::utils::get_stream_latency(stream_, setup.stream_params, pa_);
volume_ = setup.config_params.volume;
if (volume_) {
audio::volume::set_volume(device_name_, *volume_);
Expand Down Expand Up @@ -390,7 +390,7 @@ void Speaker::reconfigure(const vsdk::Dependencies& deps, const vsdk::ResourceCo
device_name_ = setup.stream_params.device_name;
sample_rate_ = setup.stream_params.sample_rate;
num_channels_ = setup.stream_params.num_channels;
latency_ = setup.stream_params.latency_seconds;
latency_ = audio::utils::get_stream_latency(stream_, setup.stream_params, pa_);
audio_context_ = setup.audio_context;
volume_ = setup.config_params.volume;
if (volume_) {
Expand Down
44 changes: 43 additions & 1 deletion test/audio_utils_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ TEST_F(AudioUtilsTest, SetupStreamFromConfigUsesProvidedValues) {

EXPECT_EQ(stream_params.sample_rate, 44100);
EXPECT_EQ(stream_params.num_channels, 2);
EXPECT_DOUBLE_EQ(stream_params.latency_seconds, 0.1);
EXPECT_DOUBLE_EQ(stream_params.suggested_latency_seconds, 0.1);
}

TEST_F(AudioUtilsTest, SetupStreamFromConfigOutputDirection) {
Expand Down Expand Up @@ -364,6 +364,48 @@ TEST_F(AudioUtilsTest, SetupAudioDeviceUsesConfigParams) {
EXPECT_EQ(setup.config_params.device_name, "My Device");
}

TEST_F(AudioUtilsTest, GetStreamLatencyFallsBackWhenStreamInfoNull) {
using ::testing::Return;
ON_CALL(*mock_pa_, getStreamInfo(::testing::_)).WillByDefault(Return(nullptr));

audio::utils::StreamParams params;
params.suggested_latency_seconds = 0.05;
params.is_input = false;

double latency = audio::utils::get_stream_latency(nullptr, params, mock_pa_.get());
EXPECT_DOUBLE_EQ(latency, 0.05);
}

TEST_F(AudioUtilsTest, GetStreamLatencyReturnsOutputLatency) {
using ::testing::Return;
PaStreamInfo stream_info;
stream_info.inputLatency = 0.02;
stream_info.outputLatency = 0.04;
ON_CALL(*mock_pa_, getStreamInfo(::testing::_)).WillByDefault(Return(&stream_info));

audio::utils::StreamParams params;
params.suggested_latency_seconds = 0.01;
params.is_input = false;

double latency = audio::utils::get_stream_latency(nullptr, params, mock_pa_.get());
EXPECT_DOUBLE_EQ(latency, 0.04);
}

TEST_F(AudioUtilsTest, GetStreamLatencyReturnsInputLatency) {
using ::testing::Return;
PaStreamInfo stream_info;
stream_info.inputLatency = 0.02;
stream_info.outputLatency = 0.04;
ON_CALL(*mock_pa_, getStreamInfo(::testing::_)).WillByDefault(Return(&stream_info));

audio::utils::StreamParams params;
params.suggested_latency_seconds = 0.01;
params.is_input = true;

double latency = audio::utils::get_stream_latency(nullptr, params, mock_pa_.get());
EXPECT_DOUBLE_EQ(latency, 0.02);
}

int main(int argc, char **argv) {
::testing::InitGoogleTest(&argc, argv);
::testing::AddGlobalTestEnvironment(new test_utils::AudioTestEnvironment);
Expand Down