Skip to content

fix(file-output): clean stop on a closed pipe, release the pacer on writer retirement - #357

Merged
mgth merged 1 commit into
mainfrom
fix/file-output-followups
Sep 3, 2026
Merged

fix(file-output): clean stop on a closed pipe, release the pacer on writer retirement#357
mgth merged 1 commit into
mainfrom
fix/file-output-followups

Conversation

@mgth

@mgth mgth commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Follow-ups from #113, tracked in #116.

1. BrokenPipe on the file sink ends the run cleanly

When the consumer of the file sink closes its end (... | head, ffmpeg exiting, a relay dying on a FIFO), the EPIPE propagated from write_all up to the session loop as a hard error and the renderer left with orender exiting with error.

  • AudioWriter::write_pcm_samples maps ErrorKind::BrokenPipe from the file sink to a typed OutputClosed error; any other I/O error keeps its meaning.
  • process_decoder_messages recognises it on both write paths (decoded frames and the idle feed): logs one info line, calls sys::shutdown::request_shutdown() so the decoder thread wakes out of its input read, and leaves the loop. The run then goes through the normal shutdown teardown and exits 0.
  • The final flush of the file sink (finish / close_and_drop) treats a BrokenPipe as nothing left to deliver, so the shutdown flush does not turn back into a warning.
  • complete_render_run drops the frame receiver before joining the decoder thread. The channel is bounded and the decoder may be blocked in send on a full one (the file sink applies backpressure) with nobody receiving any more; the closed channel makes that send fail and the decoder stops on it. Without this the join could hang.

2. Pacer handle released when the writer is retired

flush_and_invalidate_writer (backend/device/rate switches), the width-change and stream-restart paths in the handler, and standby all dropped the writer but left its PacerHandle on InputControl. Both consumers of that handle (the PipeWire input RT callback and the pipe-bridge drain thread) kept calling drain() on it every chunk: thousands of failed pops/pushes per chunk on a dead FIFO and ring, and diag_underrun_total growing without bound, i.e. phantom underruns in the pacer telemetry while in file mode.

  • InputControl::clear_output_pacer() added. OutputState::invalidate_writer now takes the InputControl and clears the handle in the same step; the signature change makes every retirement site pass it, and the standby path uses it too instead of a bare = None.
  • The install moves from create_audio_writer_if_needed into build_audio_writer, so every path that builds a writer installs the handle of the writer it is about to play through. This also covers handle_stream_restart, which built a new PipeWire writer directly and never installed its handle, leaving the input thread draining the retired one.

Verification

  • cargo fmt --check and cargo test -p omniphony-renderer -p audio_input -p audio_output green. Three new unit tests cover the OutputClosed mapping, the non-EPIPE error path and the tolerant final flush.
  • End to end: orender render dumps/dts51_core.dts --output-backend file --output-file - --enable-vbap --no-osc | head -c 400000 exits 0 with one info line (Output consumer closed the pipe; stopping the render loop) followed by Decoder stopped cleanly. Same with a reader that sleeps 3 s before reading 100 kB (decoder blocked 2.7 s in send at the moment the pipe closed): clean exit, no hang.
  • The Studio device→file switch on a live PipeWire session (the pacer clearing path) was not exercised in this session. The change there is the clear_output_pacer() call on retirement; the rebuild on switching back re-installs the handle as before.

Closes #116

🤖 Generated with Claude Code

…riter retirement

Follow-ups from #113 tracked in #116.

- `file` sink: a consumer that closes its end (EPIPE) used to surface as a
  hard render error. `AudioWriter::write_pcm_samples` now maps it to
  `OutputClosed`; the session loop logs it once, requests a clean shutdown
  and leaves the loop, and the final flush ignores the same condition. The
  frame channel is closed before the decoder thread is joined, so a decoder
  blocked in `send` on a full channel (file-sink backpressure) cannot hang
  the join.
- Pacer: retiring the audio writer now clears the pacer handle on
  `InputControl` (`OutputState::invalidate_writer` takes the control), so
  the input thread stops draining a dead FIFO into a dead ring on every
  chunk after a device->file switch, a device/rate change or standby. The
  install moves into `build_audio_writer`, so the stream-restart path
  installs the new writer's handle too instead of leaving the retired one.

Closes #116

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mgth
mgth merged commit 232cb0f into main Sep 3, 2026
3 checks passed
@mgth
mgth deleted the fix/file-output-followups branch September 3, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

file output backend: graceful BrokenPipe + release PipeWire pacer on device→file switch

1 participant