ccextractor icon indicating copy to clipboard operation
ccextractor copied to clipboard

[FEATURE] Port 708 decoder encoding module to RUST

Open IshanGrover2004 opened this issue 10 months ago • 3 comments

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

  • [x] I have read and understood the contributors guide.
  • [x] I have checked that another pull request for this purpose does not exist.
  • [x] I have considered, and confirmed that this submission will be valuable to others.
  • [x] I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • [x] I give this submission freely, and claim no ownership to its content.
  • [x] 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.
  • [x] I am an active contributor to CCExtractor.

I have migrated the logic and functions from the lib_ccx/ccx_decoder_708_encoding.c file to Rust, located in decoder/encoding.rs. This conversion maintains the same functionality that was originally written in C, now implemented in Rust.

To know more See ccx_decoder_708_encoding.c and compare with File Changed in PR, they are both same

Background & Testing

  • Current codebase scenerio: Only C code of ccx_decoder_708_encoding.c file is available. So while making & running Rust test cases which requires these functions do not have body because cargo test is just seeing the rust code only.
  • After PR changes: Exact same logic of rust code will be there. And I did also added the conditional compilation so that if user build with -without-rust then only C code will work or else Rust code will build. And Yes I have manually tested that when compiling with rust src files, rust code is being used and giving the same result.

IshanGrover2004 avatar Apr 19 '24 12:04 IshanGrover2004

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

Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 4/7
DVD 3/3
DVR-MS 2/2
General 24/27
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 85/86
Teletext 21/21
WTV 13/13
XDS 34/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:

  • ccextractor --autoprogram --out=srt --latin1 85271be4d2..., Last passed: Never
  • ccextractor --stdout --quiet --no-fontcolor 79a51f3500..., Last passed: Never
  • ccextractor --stdout --quiet --no-fontcolor 767b546f96..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 1974a299f0..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 132d7df7e9..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 99e5eaafdc..., Last passed: Never
  • ccextractor --capfile /repository/Dictionary/MattS_dictionary.txt c83f765c66..., Last passed: Never

Congratulations: Merging this PR would fix the following tests:


Check the result page for more info.

ccextractor-bot avatar May 16 '24 04:05 ccextractor-bot

@prateekmedia Any idea why Windows is failing here?

PunitLodha avatar May 22 '24 13:05 PunitLodha

@PunitLodha You can go on for merge if you find correct, btw this method of having same name is working, I have checked it

And another thing I have also Enable the default_feature of bindgen bcs this becomes useful while developing in Rust & also used in next PRs as well

IshanGrover2004 avatar May 26 '24 08:05 IshanGrover2004

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

Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 4/7
DVD 3/3
DVR-MS 2/2
General 24/27
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 84/86
Teletext 21/21
WTV 9/13
XDS 34/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 avatar May 26 '24 10:05 ccextractor-bot