Skip to content

Conversation

@hrideshmg
Copy link
Contributor

@hrideshmg hrideshmg commented Jun 28, 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.

The regression tests under the XDS category have been failing on Windows due to segmentation faults for a while now. The cause for these errors stem from a small mistake that was made when the CEA-708 decoder functions were ported to Rust.

In specific, here is the offending line:

// Add symbol to window
window.rows[window.pen_row as usize] = Box::into_raw(Box::new(sym));

To contrast, here is the equivalent C section:

window->rows[window->pen_row][window->pen_column] = symbol;

window.rows has the following definition:

dtvcc_symbol *rows[15];
rows[i] = (dtvcc_symbol *)malloc(64 * sizeof(dtvcc_symbol));

As we can see, it is an array of pointers where each pointer points to a heap allocated row of dtvcc_symbols. In rust, this structure is incorrectly assumed to be an array of dtvcc_symbols. I've fixed this by using pointer arithmetic in rust to do the proper operation:

unsafe {
    let ptr: *mut dtvcc_symbol =
        window.rows[window.pen_row as usize].add(window.pen_column as usize);
    *ptr = sym;
}

The unit tests which directly utilize this operation were also leaking memory due to this, and thus I've made helper functions to properly handle those as well.

@hrideshmg
Copy link
Contributor Author

hrideshmg commented Jun 29, 2025

@prateekmedia This PR is also ready, could you take a look? Thanks!

@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 2260165...:

Report Name Tests Passed
Broken 11/13
CEA-708 6/14
DVB 6/7
DVD 0/3
DVR-MS 2/2
General 15/27
Hauppage 1/3
MP4 3/3
NoCC 10/10
Options 79/86
Teletext 7/21
WTV 13/13
XDS 24/34

All tests passing on the master branch were passed completely.

NOTE: The following tests have been failing on the master branch as well as the PR:

Congratulations: Merging this PR would fix the following tests:


Check the result page for more info.

@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 77b93e5...:

Report Name Tests Passed
Broken 12/13
CEA-708 7/14
DVB 4/7
DVD 3/3
DVR-MS 2/2
General 15/27
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 84/86
Teletext 21/21
WTV 10/13
XDS 23/34

All tests passing on the master branch were passed completely.

NOTE: The following tests have been failing on the master branch as well as the PR:

Congratulations: Merging this PR would fix the following tests:


Check the result page for more info.

@cfsmp3 cfsmp3 self-requested a review June 30, 2025 03:12
@cfsmp3 cfsmp3 merged commit e663eca into CCExtractor:master Jun 30, 2025
18 checks passed
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.

3 participants