Skip to content

[CLEANUP]: Remove redundant es_functions.c C code superseded by the Rust ES port - #2327

Open
x15sr71 wants to merge 1 commit into
CCExtractor:masterfrom
x15sr71:cleanup/es-functions-dead-code
Open

[CLEANUP]: Remove redundant es_functions.c C code superseded by the Rust ES port#2327
x15sr71 wants to merge 1 commit into
CCExtractor:masterfrom
x15sr71:cleanup/es-functions-dead-code

Conversation

@x15sr71

@x15sr71 x15sr71 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

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

Reason for this PR:

  • This PR adds new functionality.
  • This PR fixes a bug that I have personally experienced or that a real user has reported and for which a sample exists.
  • This PR is porting code from C to Rust.

Sanity check:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • If the PR adds new functionality, I've added it to the changelog. If it's just a bug fix, I have NOT added it to the changelog.
  • I am NOT adding new C code unless it's to fix an existing, reproducible bug.

Note — this is a verification, not a bug repro (this PR removes dead code, there's no bug to reproduce): it verifies the deletion is behavior-preserving by running the sample through the exact functions this PR touches (process_m2v → es_video_sequence → delegates) and confirming baseline vs branch output is byte-identical.

Verification

Sample: (the filename 725a49f871dc5a2 ~19 MB MPEG-2 program stream, NTSC line-21 EIA-608). Such streams route CCX_PES → process_m2v → es_video_sequence → delegates → Rust (general_loop.c:892), exercising exactly the functions this PR touches.

Deadness itself is proven statically (every deleted statement sits after an unconditional return ccxr_…(), unreachable by C semantics). The steps below are corroboration: they confirm the sample actually drives the touched path, and that deleting the unreachable code is behavior-preserving.

# Build master (baseline) and this branch
git worktree add /tmp/ccx-base master && (cd /tmp/ccx-base/linux && cmake ../src && make -j4)
(cd linux && cmake ../src && make -j4)

# 1. Confirm the sample drives the touched path (expect thousands of hits)
./linux/ccextractor --debug sample.mpg -o /dev/null 2>&1 | grep -c 'es_video_sequence'

# 2. Diff output — expect byte-identical across every format
for f in srt ttxt sami; do
  /tmp/ccx-base/linux/ccextractor --out=$f sample.mpg -o base.$f
  ./linux/ccextractor             --out=$f sample.mpg -o pr.$f
  diff base.$f pr.$f && echo "$f: identical"
done

Expected: step 1 prints a large count (8,722 in my run — the ES path is heavily used); step 2 prints srt: identical, ttxt: identical, sami: identical.


What this removes

  1. The dead bodies of the 7 ES entry points. Each begins with an unconditional return ccxr_…() that forwards to the Rust implementation; every statement after it is unreachable. This PR reduces each to that single delegating line.
  2. The 6 now-orphaned C parser helpers, whose only callers were those dead bodies:
    sequence_header, sequence_ext, gop_header, pic_header, pic_coding_ext, extension_and_user_data — plus their forward declarations.

So the deleted code is unreachable in two forms: each of the 7 delegate bodies sits after an unconditional return ccxr_…(), and the 6 helpers are called only from those dead bodies.

What is kept (all live, untouched)

  • process_m2v — the entry point (real C body: bitstream init + dispatch).
  • es_video_sequence — the C state-machine dispatcher reached from process_m2v. It calls only the delegates, never the helpers.
  • The 7 one-line delegates, which continue to forward to Rust via FFI.

What made this code redundant

The MPEG-2 elementary-stream parser was ported to Rust across two merged PRs:

  • [FEAT] added demuxer and file_functions module #1662[FEAT] added demuxer and file_functions module. Added the extern declarations for the ES ccxr_* functions and converted the 7 C entry points into thin delegates by prepending return ccxr_…(). This is what rendered the original C parser bodies unreachable.
  • [Rust]Ported ES Module to Rust #1736[Rust] Ported ES Module to Rust. Added the actual Rust ES parsing implementation (src/rust/src/es/) that those delegates now call.

The old C parser bodies and helpers were left in place as unreachable reference code. The compiler can't flag them (-Wunused-function) because the helpers are still statically referenced from the dead bodies — which is why this survived the earlier cleanup pass (#1738).

What runs in its place today

The live runtime path is unchanged by this PR:

process_m2v (C)  →  es_video_sequence (C dispatcher)  →  7 thin C delegates  →  ccxr_* (Rust)

The delegates forward via FFI to the Rust ES module at src/rust/src/es/mod.rs. The deleted C helpers correspond directly to Rust functions that already do the work:

Deleted C helper(s) Lived inside (dead body of) Now handled in Rust by
sequence_header, sequence_ext read_seq_info ccxr_read_seq_info (es/mod.rs:37, es/seq.rs)
gop_header read_gop_info ccxr_read_gop_info (es/mod.rs:53, es/gop.rs)
pic_header, pic_coding_ext read_pic_info ccxr_read_pic_info (es/mod.rs:67, es/pic.rs)
extension_and_user_data read_eau_info ccxr_read_eau_info (es/mod.rs:81)
(start-code scanning) next_start_code / search_start_code ccxr_next_start_code (es/mod.rs:19) / ccxr_search_start_code (es/mod.rs:28)
(slice scanning) read_pic_data ccxr_read_pic_data (es/mod.rs:98)

The C→Rust behavioral equivalence for these functions was established and tested by #1662/#1736 when the port landed; this PR neither relies on nor changes it, since the deleted C wasn't executing in either case.


Why the removed code is never run, in any condition

1. Static / language-level (input-independent certainty).

  • Every helper call site sits strictly after an unconditional return ccxr_…() — unreachable by C semantics. No input, CLI flag, mode, or build configuration can reach it.
  • No conditional compilation gates those returns. The only preprocessor directive in the file is a permanently-disabled #if 0 block inside a removed helper body.
  • No indirect/function-pointer use: grep for &sequence_header etc. across the tree — zero address-of any helper. All calls were direct, and all were in the dead bodies.
  • No references anywhere else: whole-tree grep across C (.c/.h) and Rust (src/rust/) finds zero live references to the 6 helpers. The only mention outside the deleted code is one comment in general_loop.c.
  • Post-deletion file has zero dangling references and compiles with zero warnings (es_functions.c.o).

2. Empirical (executed the exact path we touched).
Built a baseline binary (master) and a branch binary (this PR), and ran both on an MPEG-2 program-stream sample carrying NTSC line-21 (EIA-608) captions — which route CCX_PES → process_m2v → es_video_sequence → delegates → Rust (general_loop.c:892). Confirmed via --debug that this sample drives es_video_sequence 8,722 times.

Check Result
Full --debug execution trace byte-identical (matching md5)
--out=srt byte-identical
--out=ttxt byte-identical
--out=sami byte-identical

Identical execution traces prove the runtime path through the delegates→Rust is unchanged; identical output confirms no functional regression.

Note on tooling: clang -Wunreachable-code-aggressive emits nothing here — clang's unreachable-code analysis is deliberately conservative and does not flag code after return. gcov would only show "0 hits on the samples run" (input-dependent). Both are strictly weaker than the static argument above, which holds for all inputs.

@ccextractor-bot

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on linux. 167/237 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 22/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 31/34

70 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.


Compared with the tip of mastertest 9499, commit 3af3fc2:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 70 fail on both, byte for byte the same

Compared with the commit this branch was cut from: the same run as the tip of master (test 9499), so the comparison above already covers it.


No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

@ccextractor-bot

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on windows. 167/237 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 22/27
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 31/34

70 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.


Compared with the tip of mastertest 9495, commit 3af3fc2:

  • 1 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 69 fail on both, byte for byte the same

Pass there, fail here:


Compared with the commit this branch was cut from: the same run as the tip of master (test 9495), so the comparison above already covers it.


This branch changes the behaviour of 1 test(s) relative to the tip of master. Those are the ones worth looking at; anything else in the list fails the same way on both sides.

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