ccextractor icon indicating copy to clipboard operation
ccextractor copied to clipboard

Published 20 hours ago • verified publisher icon CCExtractor

[FEAT] Add argument parsing in ccextractor rust

Open prateekmedia opened this issue 1 year ago • 10 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.
  • [ ] 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.

Rewrote argument parsing with all the structs and enums too into rust.

Help arguments demo: image

prateekmedia avatar Jul 28 '23 11:07 prateekmedia

Now you need to create an extern function in lib.rs which parses the arguments as passed by the C code, and which can be called by the C code.

/// Parse parameters from argv and argc
#[no_mangle]
pub extern "C" fn ccxr_parse_parameters(options: *mut ccx_s_options, argc: c_int, argv: *mut *mut i8 (or something similar...)) -> c_int {
   // Convert argv to Vec<String> and pass it to parse_parameters
    let args: Vec<String> = ...
    let args = Args::try_parse_from(args); // Handle the error here
    let opt = CcxOptions::default();
    opt.parse_parameters(args); 
    // Convert the rust struct (CcxOptions) to C struct (ccx_s_options), so that it can be used by the C code 
    options = opt.to_options(); 
}

And then call it from C

// At the start
#ifndef DISABLE_RUST
extern int ccxr_parse_parameters(struct ccx_s_options *opt, int argc, char *argv[])
#endif


// Inside main()
#ifndef DISABLE_RUST
	int compile_ret = ccxr_parse_parameters(api_options, argc, argv);
#else
	int compile_ret = parse_parameters(api_options, argc, argv);
#endif

PunitLodha avatar Aug 21 '23 11:08 PunitLodha

@PunitLodha I am getting ccx_s_options not found in rust

prateekmedia avatar Aug 21 '23 14:08 prateekmedia

You'll have to generate bindings for it. Check out build.rs

PunitLodha avatar Aug 21 '23 17:08 PunitLodha

cc @cfsmp3 @canihavesomecoffee, The tests are all with 0's as they are ran on C code, which uses old params and not on rust code. Rust code is already unit tested but still after C parameter PR is merged this can also be rebased and merged so its all good from my side. Already reviewed by @PunitLodha

prateekmedia avatar Jan 14 '24 17:01 prateekmedia

It seems not to be working. This is the issue:

Invalid option to CCextractor Library

prateekmedia avatar Jan 15 '24 13:01 prateekmedia

This PR needs to implement these methods to let tests run: add_file_sequence append_file_to_queue

Or use these existing methods from c as extern.

prateekmedia avatar Apr 13 '24 15:04 prateekmedia

So now the Args are parsing correctly but the processing output is not visible.

prateekmedia avatar Apr 14 '24 15:04 prateekmedia

So now the Args are parsing correctly but the processing output is not visible.

@prateekmedia is this fixed?

PunitLodha avatar May 20 '24 12:05 PunitLodha

@prateekmedia could you split out the CI changes into another PR?

PunitLodha avatar May 28 '24 09:05 PunitLodha

@PunitLodha #1612

prateekmedia avatar May 28 '24 09:05 prateekmedia

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

Report Name Tests Passed
Broken 8/13
CEA-708 14/14
DVB 4/7
DVD 3/3
DVR-MS 2/2
General 24/27
Hauppage 3/3
MP4 1/3
NoCC 10/10
Options 73/86
Teletext 21/21
WTV 5/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 Jun 21 '24 06:06 ccextractor-bot

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

Report Name Tests Passed
Broken 0/13
CEA-708 1/14
DVB 0/7
DVD 0/3
DVR-MS 0/2
General 0/27
Hauppage 0/3
MP4 0/3
NoCC 0/10
Options 0/86
Teletext 0/21
WTV 0/13
XDS 0/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 Jun 25 '24 22:06 ccextractor-bot