RSDK-13979 Implement PlayStream#41
Conversation
seanavery
left a comment
seanavery
left a comment
There was a problem hiding this comment.
Producers must feed at ≤ real-time.
Trying to understand the significance of this. Seems like the use cases that motivate PlayStream TTS replies, file playback, anything cached in memory would produce faster than real-time.
Would it be easy to implement backpressure handling in process_and_write_pcm?
good point, I added a check to not write any more samples until the callback drains the buffer if the buffer is too full. So now the clients can feed at any time. |
seanavery
left a comment
seanavery
left a comment
There was a problem hiding this comment.
Looking good! Thanks for adding the backpressure handling.
Would it be possible to add a test case to verify backpressure is working as intended?
| } | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(10)); | ||
| } | ||
| playback_context->write_sample(samples[i]); |
There was a problem hiding this comment.
Ok to write_sample outside of the stream_mu lock?
There was a problem hiding this comment.
yes, write_sample is atomic
| for (size_t i = 0; i < num_samples; ++i) { | ||
| while (playback_context->get_write_position() - playback_context->playback_position.load() >= max_ahead) { | ||
| if (stop_requested_.load()) { | ||
| return i; |
There was a problem hiding this comment.
[possible nit] Does this complicate the assumption that 0 return means context swap? Maybe need to update comments
There was a problem hiding this comment.
yeah the 0 = swap is kind of awkward so I refactored to just always return the num samples written
added test |
| } | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(10)); | ||
| } | ||
| playback_context->write_sample(samples[i]); |
There was a problem hiding this comment.
[possible nit] Do we also need to check for stop at the top of the for loop here so we do not inadvertently write_sample here after stop?
There was a problem hiding this comment.
its fine if we write a sample after stop, we will clear all samples shortly after
2 participants
Summary
Implements
PlayStreamfor theSpeakercomponent. Chunks pulled from theclient are decoded, channel-converted, resampled, and written into the playback
ring buffer as they arrive; the call blocks until the source signals
end-of-stream and the buffer drains.
play_stream(audio_info, chunk_source, extra)override.playintoa
process_and_write_pcmhelper, and the drain-wait loop intowait_for_playback(also moves the post-drainlatency_sleep inside,where
stream_mu_can guard the read).PCM_16,PCM_32,PCM_32_FLOAT.Will do MP3 support in a subsequent PR.
Blocked on SDK release
Requires
viam-cpp-sdkv0.37.0 (PR viamrobotics/viam-cpp-sdk#637). Once cut,bump
conanfile.pyfrom0.21.0→0.37.0; no other changes needed.Notes for reviewers
write, so overruns silently drop the oldest samples. Adding server-side
backpressure is a possible follow-up.
Test plan
PlayStream_*unit tests cover MP3 rejection, empty source, multi-chunk PCM_16, PCM_32, PCM_32_FLOAT, channel conversion, resampling, mid-
stream stop, and stop-flag reset on entry.
mid-stream stop + client context cancel exit cleanly, back-to-back
PlayStreamcalls serialize viaplayback_mu_.