Skip to content

feat: rename StreamTrait::play to start, add draining stop#1258

Open
roderickvd wants to merge 13 commits into
developfrom
feat/start-pause-stop
Open

feat: rename StreamTrait::play to start, add draining stop#1258
roderickvd wants to merge 13 commits into
developfrom
feat/start-pause-stop

Conversation

@roderickvd

@roderickvd roderickvd commented Jul 4, 2026

Copy link
Copy Markdown
Member

This PR introduces start/pause/stop semantics on StreamTrait with draining stop.

Playback

On playback streams, the difference between pausing and stopping is that:

  • pause halts as soon as possible, if necessary discarding buffered audio
  • stop(timeout) drains, letting buffered output finish before halting.

stop thus blocks until the buffer empties or timeout elapses.

Capture

Capture streams have nothing to drain and halt immediately on stop.

Behavior details

  • play is now deprecated and forwards to start.
  • A timeout of None waits indefinitely; Some(Duration::ZERO) halts without draining.
  • Stopped and paused streams are resumable with start.
  • Dropping a stream halts immediately without draining.

Backend support

  • All backends support draining stop, except WebAudio and AudioWorklet where the calling thread can't block.
  • On ALSA, pause now implements a soft-pause fallback when there is no hardware support.
  • While there:
    • relaxed StreamState atomics in ASIO and JACK from Acquire/Release to Relaxed (no associated data payload).
    • fixed PipeWire to no longer auto-start streams.

Related

Fixes:

@roderickvd roderickvd mentioned this pull request Jul 4, 2026
17 tasks
@roderickvd roderickvd force-pushed the feat/start-pause-stop branch 2 times, most recently from 0101f48 to f5cbdac Compare July 4, 2026 20:34
@roderickvd roderickvd requested a review from Copilot July 4, 2026 20:39

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Comment thread src/host/alsa/mod.rs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Comment thread src/host/pulseaudio/stream.rs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.

Comment thread src/traits.rs Outdated
Comment thread UPGRADING.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Comment on lines +163 to +165
fn stop(&self, timeout: Option<std::time::Duration>) -> Result<(), Error> {
let do_drain = self.is_output && timeout != Some(std::time::Duration::ZERO);
if do_drain {

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Comment thread src/host/wasapi/stream.rs Outdated
Comment on lines +520 to +533
fn stop(&self, timeout: Option<Duration>) -> Result<(), Error> {
self.draining.store(true, Ordering::Relaxed);
crate::host::com::com_initialized();

if timeout != Some(Duration::ZERO) {
let wait = if let Some(ref client) = self.drain_audio_client {
unsafe {
let padding = client.GetCurrentPadding().unwrap_or(0);
let latency_hns = client.GetStreamLatency().unwrap_or(0);
let mut buffered =
Duration::from_nanos((latency_hns.max(0) as u64).saturating_mul(100));
if let Ok(fmt) = client.GetMixFormat() {
let sample_rate = (*fmt).nSamplesPerSec;
Com::CoTaskMemFree(Some(fmt.cast()));

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Comment thread src/host/alsa/mod.rs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/host/wasapi/stream.rs:231

  • The unsafe Send justification comment is now out of date: Stream gained draining and fill_usec fields, but they are not mentioned in the list of Send fields. For unsafe impls, keeping this rationale accurate is important for future maintenance and auditing.
// SAFETY: Windows Event HANDLEs are safe to send between threads - they are designed for
// synchronization. All fields of Stream are Send:
// - JoinHandle<()> is Send
// - Sender<Command> is Send
// - Foundation::HANDLE is Send (Windows synchronization primitive)
// - Latch is Send

Comment thread src/host/wasapi/stream.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants