fix(file-output): clean stop on a closed pipe, release the pacer on writer retirement - #357
Merged
Conversation
…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>
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
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.
Follow-ups from #113, tracked in #116.
1.
BrokenPipeon thefilesink ends the run cleanlyWhen the consumer of the
filesink closes its end (... | head,ffmpegexiting, a relay dying on a FIFO), theEPIPEpropagated fromwrite_allup to the session loop as a hard error and the renderer left withorender exiting with error.AudioWriter::write_pcm_samplesmapsErrorKind::BrokenPipefrom the file sink to a typedOutputClosederror; any other I/O error keeps its meaning.process_decoder_messagesrecognises it on both write paths (decoded frames and the idle feed): logs one info line, callssys::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.finish/close_and_drop) treats aBrokenPipeas nothing left to deliver, so the shutdown flush does not turn back into a warning.complete_render_rundrops the frame receiver before joining the decoder thread. The channel is bounded and the decoder may be blocked insendon 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 itsPacerHandleonInputControl. Both consumers of that handle (the PipeWire input RT callback and the pipe-bridge drain thread) kept callingdrain()on it every chunk: thousands of failed pops/pushes per chunk on a dead FIFO and ring, anddiag_underrun_totalgrowing without bound, i.e. phantom underruns in the pacer telemetry while infilemode.InputControl::clear_output_pacer()added.OutputState::invalidate_writernow takes theInputControland 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.create_audio_writer_if_neededintobuild_audio_writer, so every path that builds a writer installs the handle of the writer it is about to play through. This also covershandle_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 --checkandcargo test -p omniphony-renderer -p audio_input -p audio_outputgreen. Three new unit tests cover theOutputClosedmapping, the non-EPIPEerror path and the tolerant final flush.orender render dumps/dts51_core.dts --output-backend file --output-file - --enable-vbap --no-osc | head -c 400000exits 0 with one info line (Output consumer closed the pipe; stopping the render loop) followed byDecoder stopped cleanly. Same with a reader that sleeps 3 s before reading 100 kB (decoder blocked 2.7 s insendat the moment the pipe closed): clean exit, no hang.clear_output_pacer()call on retirement; the rebuild on switching back re-installs the handle as before.Closes #116
🤖 Generated with Claude Code