Skip to content

Conversation

@x15sr71
Copy link
Contributor

@x15sr71 x15sr71 commented Dec 22, 2025

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.
  • I have mentioned this change in the changelog.

My familiarity with the project is as follows (check one):

  • I have never used CCExtractor.
  • I have used CCExtractor just a couple of times.
  • I absolutely love CCExtractor, but have not contributed previously.
  • I am an active contributor to CCExtractor.

Fix #1478: Prevent CEA-708 service decoder panic when using -stdout on Windows

Issue Summary

Issue #1478 reports a panic in CCExtractor 0.94 when using -stdout with HDHomeRun-recorded .ts files on Windows:

Original crash reported:

thread '' panicked at 'index out of bounds: the len is 1 but the index is 1', 
src\decoder\service_decoder.rs:275:25

Reproduction

I reproduced a similar panic on Windows 0.94 portable build using -stdoutand sample file from #1478:

My reproduction:

PS> $env:RUST_BACKTRACE=1
PS> .\ccextractorwinfull.exe -stdout cctest1.ts > C:\Users\Public\ccStdOut.txt

CCExtractor 0.94, Carlos Fernandez Sanz, Volker Quetschke.
Teletext decoder: Petr Kutalek
...
Opening file: cctest1.ts
thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: "Filename missing"', 
src\decoder\service_decoder.rs:897:43

Root Cause Analysis

  • Although the panics are reported at different line numbers across builds, they stem from the same underlying issue in the CEA-708 service decoder.

  • On Windows, when running with -stdout, transcript output is effectively disabled or not fully initialized (no output filename, different writer setup). However, the decoder still executes code paths that assume a fully configured transcript writer.

  • This results in panics in different places depending on build and execution path:

  • Failed Writer::new() being unconditionally used (e.g. “Filename missing”)

  • Accessing a non-existent writer context (index out of bounds)

  • While the exact panic location varies, both failures indicate that writer-related state is accessed when transcript output is not valid.

  • This fix adds an explicit check for transcript_settings and exits early when transcript output is disabled, preventing invalid writer usage. Behavior when transcript output is enabled remains unchanged.

Changes Made

Modified src/rust/src/decoder/service_decoder.rs to add defensive checks in two methods:

1. screen_print() method:

  • Check if transcript_settings exists before attempting Writer creation
  • Return early if transcript output is disabled (instead of panicking)
  • Remove .unwrap() call that caused the panic

2. flush() method:

  • Add same defensive check for transcript_settings
  • Return early if transcript output is not configured
  • Prevent similar panic scenarios during cleanup

Testing

Cross-platform tested:

  • macOS (M4): Successfully built and tested with the sample file provided in the issue; -stdout generates valid SRT output with no regressions observed.
  • Linux (Ubuntu 22.04): Built and tested using the same approach as macOS; output is correct and unchanged.

Request for Windows testing

  • I was unable to test whether this fix resolves the crash on Windows, as I currently don't have access to a Windows build environment. Testing on Linux and macOS shows no regressions, but Windows-specific validation via community testing would be valuable.
  • If anyone with a Windows machine can test with the sample file from #1478, that would be greatly appreciated!

Why This Fix Should Work

Even though the panic line numbers differ between the original report (275) and my reproduction (897), this fix addresses what appears to be the root cause based on available evidence:

The Fix Strategy:

  • Before any Writer operations, check if transcript output is properly configured

  • If not configured, skip transcript output gracefully instead of attempting to use an uninitialized Writer

  • This should prevent panics at any point where Writer is accessed (creation, usage, or cleanup)

Even if this doesn't address the exact root cause of the line 275 panic (due to build differences or additional factors), the fix provides important safety guarantees:

  • Prevents panics from null/missing transcript_settings

  • Fails gracefully instead of crashing

At minimum, this fix makes the codebase more robust against Writer initialization issues on Windows.

@x15sr71 x15sr71 changed the title Fix: Avoid panic in CEA-708 service decoder when transcript output is… Fix #1478: Prevent CEA-708 decoder panic with -stdout on Windows Dec 22, 2025
@x15sr71 x15sr71 changed the title Fix #1478: Prevent CEA-708 decoder panic with -stdout on Windows [FIX]: Prevent CEA-708 decoder panic with -stdout on Windows Dec 22, 2025
Copy link
Contributor

@cfsmp3 cfsmp3 left a comment

Choose a reason for hiding this comment

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

Review Summary

Thanks for this fix! The code changes look good and testing confirms the fix works correctly.

✅ What's Good

  • Proper use of as_ref() for null-pointer safety
  • Error handling with if let Err(e) instead of .unwrap() panic
  • Consistent fix in both screen_print() and flush() methods
  • All 299 Rust tests pass
  • Tested with CEA-708 and CEA-608 samples - no regressions

📋 Required Before Merge

  1. Rebase on master - Your branch is 12 commits behind. Run:

    git fetch origin
    git rebase origin/master
  2. Update CHANGES.TXT - After rebase, move your changelog entry to the 0.96 section (version was bumped while your PR was open)

Once rebased, this is ready to merge! 🎉

@x15sr71 x15sr71 force-pushed the fix/issue-1478-cea708-writer-unwrap-panic branch from 7a9f87d to 14aa72c Compare December 23, 2025 10:29
@x15sr71
Copy link
Contributor Author

x15sr71 commented Dec 23, 2025

Thanks for the review. I’ve rebased the branch on master and pushed the updated commits. Waiting for CI to complete.

@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit d573548...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 7/7
DVD 3/3
DVR-MS 2/2
General 27/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 86/86
Teletext 21/21
WTV 13/13
XDS 34/34

Congratulations: Merging this PR would fix the following tests:

  • ccextractor --autoprogram --out=ttxt --latin1 1974a299f0..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 --ucla dab1c1bd65..., Last passed: Never
  • ccextractor --out=srt --latin1 --autoprogram 29e5ffd34b..., Last passed: Never
  • ccextractor --xdsdebug --out=srt c83f765c66..., Last passed: Never
  • ccextractor --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsnotbefore 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsnotafter 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsforatleast 1 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never
  • ccextractor --startcreditsforatmost 2 --startcreditstext "CCextractor Start crdit Testing" c4dd893cb9..., Last passed: Never

All tests passing on the master branch were passed completely.

Check the result page for more info.

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.

[BUG] -stdout .ts crashes ccextractorwinfull.exe 094

3 participants