Skip to content

[FIX] Use seconds for the --stream timeout in the Rust code - #2426

Open
Deepak-negi11 wants to merge 1 commit into
CCExtractor:masterfrom
Deepak-negi11:fix/stream-timeout-units
Open

Deepak-negi11 wants to merge 1 commit into
CCExtractor:masterfrom
Deepak-negi11:fix/stream-timeout-units

Conversation

@Deepak-negi11

Copy link
Copy Markdown
Contributor

Repro instructions:

Needs #2415 (on master --stream can't be used with a file at all).
Sample: Closed Caption EIA.ts

./ccextractor --stream 3 "Closed Caption EIA.ts"
Command (with #2415) Before After
--stream 2 small.scc never exits exits about 2 s after the file stops growing
--stream 3 "Closed Caption EIA.ts" exits at once, the timeout is ignored waits 3 s for more data, then exits
normal run, no --stream unchanged unchanged

I found this while testing #2415.

Problem

The C code keeps the --stream timeout in seconds. The Rust Options keep it as a Timestamp. Two places mixed up the units:

  1. copy_to_rust (common.rs) read the C seconds as milliseconds, so 3 seconds became 3 ms.
  2. sleepandchecktimeout (file.rs) compared a time in seconds with start + live_stream.millis(), so a 2-second timeout became 2000 seconds.

So the timeout was either ignored or about 1000 times too long, depending on the input.

Fix

Use seconds in both places. copy_from_rust already wrote seconds, so the round trip now matches.

Testing

  • 2 new tests: a C → Rust → C round trip of the timeout, and sleepandchecktimeout with a timeout that has already passed. Each one fails if its half of the fix is removed.
  • cargo test (428 passed), cargo fmt --check, cargo clippy -- -D warnings.
  • The end-to-end runs in the table above.

Copilot AI balanced review requested due to automatic review settings October 24, 2026 23:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

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.

2 participants