Repository navigation
fix(core): stop dropping the first chunk of ffmpeg output in read_lines - #90
Merged
andykais merged 2 commits intoOct 13, 2026
Merged
Conversation
andykais
marked this pull request as ready for review
October 13, 2026 15:32
andykais
requested changes
Oct 13, 2026
| if (lines.length > 0) { | ||
| buffer = lines.at(-1)! | ||
| } | ||
| async function * read_lines(stream: ReadableStream<Uint8Array>) { |
Owner
There was a problem hiding this comment.
I see you exported a function to make this testable. I am fine with a unit test, but I still want class encapsulation. Move this to a static FileProcessor method
…g lines #readlines read one chunk before the loop and then immediately overwrote it with the next read, so every line in ffmpeg's first stderr chunk was silently discarded (and stderr that arrived in a single chunk produced no lines at all). Moves the generator to a module-level read_lines function so it can be unit tested directly. Co-authored-by: kaisea.rpi <kaisea.rpi@gmail.com>
Co-authored-by: kaisea.rpi <kaisea.rpi@gmail.com>
cursor
Bot
force-pushed
the
cursor/fix-file-processor-readlines-first-chunk-4aab
branch
from
October 13, 2026 16:01
0da52db to
6fbf2c8
Compare
andykais
approved these changes
Oct 13, 2026
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.
Summary
FileProcessor::#readlinesdropping the first chunk of ffmpeg stderr: the pre-loopreader.read()result was overwritten by the nextread()before its value was ever appended to the bufferdoneresult, so the buffer stayed empty)FileProcessor.read_linesmethod so it stays encapsulated in the class but can be unit tested directlypackages/core/test/file_processor.test.tscovering first-chunk lines, single-chunk streams, lines split across chunk boundaries, a trailing line with no newline, and an empty streamThe dropped output matters because this generator feeds thumbnail timestamp parsing (
Parsed_showinfo ... pts_time:) and theNothing was written into output fileerror detection, so lost lines can turn into wrong thumbnail counts or a generic subprocess error instead of a useful message.Vision Testing
[], first chunk lines missing)Input #0, ..., fixed reader reads all 60 lines starting atffmpeg version 6.1.1-...— exactly the 12 lines of the first 1894-byte chunk were being discardedmain;deno test -P=test --check --trace-leaks testinpackages/core: 29 passed (101 steps) / 4 failed (5 steps), the same 4 failures as unmodifiedmain(28 passed / same 4 failed), all pre-existing ffmpeg-version float precision mismatches documented inAGENTS.mddeno lintinpackages/coreis clean