Resume transcode pipeline feed when stdin pipe drains - #1656
Open
jbylsma wants to merge 1 commit into
Open
Conversation
Pipeline::sysread() feeds a transcoder's stdin only until the OS pipe buffer fills, then bails on EWOULDBLOCK. Source::_readNextChunk() only re-arms the select loop on the pipeline's read side, which can't become readable until the transcoder produces output - so a stalled transcoder (e.g. flac parsing a multi-MB embedded picture) only gets fed again on the unrelated 0.4s HTTP retry timer, throttling input to roughly one pipe buffer per tick. Register a write watcher on the pipeline's stdin whenever it has pending bytes, so the feed resumes as soon as the transcoder drains its input instead of waiting on the timer. Signed-off-by: Jonny Bylsma <jmbylsma@gmail.com>
Member
|
@bpa-code @philippe44 @ralph-irving I'd appreciate your feedback on this PR. Thanks! |
Contributor
Author
|
👍 One more interesting tidbit: enabling debugging output prevents the issue. I believe it's because the debug calls in Pipeline->sysread add small pauses to the pipe-stuffing loop, giving more time to get to the actual audio content. My "production" Lyrion is on a 4-core Synology DS1019+ under Docker, but I also reproduced it on a modern, multi-core notebook, so I don't think it's purely a CPU scheduling issue. I've been using a 9.2 dev stock Docker image (no custom I was able to discernibly reproduce the issue with FLACs that had album covers over 2MB. I'm happy to provide a test file if desired. Thanks! |
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.
FLAC files with a large embedded picture (cover art at several megabytes) have multiple seconds of silence before audio starts when transcoded (e.g.
flc mp3), even though the same file plays almost instantly when streamed without transcoding. I had first noticed this when LyrPlay was refusing to play specific FLAC files from the beginning, but I could seek within the track successfully.The cause is in the transcode pump loop (
Pipeline::sysread/Source::_readNextChunk). It feeds the transcoder's stdin only until the OS pipe buffer fills then waits to be told to resume, but can only re-arm when the transcoder's output is readable, which can't happen until the transcoder has consumed enough header to emit its first audio frame. In practice this meant refills only happened via an unrelated 0.4s HTTP retry timer, throttling a multi-MB header to roughly one pipe buffer (~64 KB on Linux) per tick.This PR adds a write-watcher on the transcoder's stdin whenever the pump loop has bytes it couldn't push through (EWOULDBLOCK on write), so the feed resumes as soon as the transcoder drains its input instead of waiting on the timer. It's a pure event-loop change without any transcoder-specific behavior and should only ever activate when there's a backlog, so it shouldn't affect any case where transcoding was already keeping up.
I've been running with this patch for over a month and everything has seemed OK. Claude-assisted, especially during debugging.