Repository navigation
[FIX] Drop the previous file's bytes when opening a file - #2394
Merged
Merged
Conversation
init_file_buffer(), called by every open(), rewound filebuffer_pos to 0 but kept bytesinbuffer when the buffer already existed. Everything the previous input file left in the buffer - normally fully consumed - thus counted as unread data of the newly opened file and was replayed. With several inputs and binary concatenation this re-fed the start of the previous file (its stream-detection bytes, up to 1 MB) into the stream at every file boundary: split recordings produced repeated and mistimed captions after the join, or aborted. The same replay made a second RCWT input re-read the first. Reset bytesinbuffer as well, in both the C and the Rust version. Whatever the buffer holds at that point belongs to the previous file.
5 of 7 tasks
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.
In raising this pull request, I confirm the following (please check boxes):
Reason for this PR:
Sanity check:
Repro instructions:
Split any captioned TS at a 188-byte-aligned offset and pass both parts (binary concatenation is the default):
head -c $OFF in.ts > a.ts; tail -c +$((OFF+1)) in.ts > b.ts; ccextractor --autoprogram --out=srt a.ts b.ts -o split.srtand compare with
ccextractor --autoprogram --out=srt in.ts -o whole.srt. On master, captions after the join are repeated or mistimed (one 3 GB sample: every timestamp ~465 ms late), and some splits abort.init_file_buffer(), called by everyopen(), rewoundfilebuffer_posto 0 but keptbytesinbufferwhen the buffer already existed. Whatever the previous input file left in the buffer — already consumed — therefore counted as unread data of the newly opened file. With binary concatenation, stream detection on the new file re-read the start of the previous file (its detection bytes, up to 1 MB) and fed it back into the stream at the join.This resets
bytesinbuffertoo, in both the C (file_functions.c) and Rust (file.rs) versions; both are only called from the demuxer'sopen(). The existing Rust test that asserted the old behaviour now checks that the buffer is reused but both counters are reset; it fails without the fix.Verified:
-min-rustbuild.cargo test --lib(418), fmt and clippy clean; valgrind shows no invalid accesses on the concat path.