RSDK-13297 Update latency param for audio playback#29
Open
oliviamiller wants to merge 3 commits intoviam-modules:mainfrom
Open
RSDK-13297 Update latency param for audio playback#29oliviamiller wants to merge 3 commits intoviam-modules:mainfrom
oliviamiller wants to merge 3 commits intoviam-modules:mainfrom
Conversation
oliviamiller
requested review from
SebastianMunozP,
hexbabe and
seanavery
and removed request for
hexbabe
seanavery
reviewed
Mar 10, 2026
| inline double get_stream_latency(PaStream* stream, const StreamParams& params, const audio::portaudio::PortAudioInterface* pa = nullptr) { | ||
| audio::portaudio::RealPortAudio real_pa; | ||
| const audio::portaudio::PortAudioInterface& audio_interface = pa ? *pa : real_pa; | ||
| const PaStreamInfo* stream_info = audio_interface.getStreamInfo(stream); |
There was a problem hiding this comment.
[nit] Do we need null check on the stream arg?
Collaborator
Author
There was a problem hiding this comment.
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.
| // 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) { |
There was a problem hiding this comment.
[question] why are audio_utils inline fns in a header file instead of breaking out into a cpp file?
Collaborator
Author
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sleep_for(latency_)at the end of play() for blocking until audio playback is complete. PortAudio's callback API does not expose a "finished playing" signal. Since we keep the stream running at all times, theres no better way to determine when audio is playing vs not.Pa_OpenStream. PortAudio may choose a different latency than the one requested.latency_secondstosuggested_latency_secondsinStreamParamsto make clear this is a request to PortAudio, not the actual latency selected.get_stream_latencyhelper in audio_utils.hpp used by both speaker and microphone to read actual latency (output or input) after stream open.get_stream_latency.Approvals
1 approval needed.