Initialize data structures correctly for the rust CEA-708 decoder

[PunitLodha]

Currently, the entry point for the rust decoder is here where we create a new Dtvcc struct each time. This causes the data to be reset each time ccxr_process_cc_data() is called.

Instead, we want to initialize Dtvcc only once at the start of the program and then use it for all subsequent function calls.

Steps:-

• [ ] Change Dtvcc::new() to create Dtvcc using ccx_decoder_dtvcc_settings as done here, https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_dtvcc.c#L76-L124
• [ ] Create a new extern C fn ccxr_dtvcc_init() in lib.rs and use this at all places where dtvcc_init() is being called. (Probably add a new struct field to lib_cc_decode like dtvcc_rust and store Dtvcc there instead of using the dtvcc field)
• [ ] Similar for dtvcc_free()
• [ ] Now change ccxr_process_cc_data to use Dtvcc from dec_ctx instead of creating a new one

Additionally to fix mp4 code flow:-

• [ ] Call the corresponding rust function here and pass Dtvcc as done in the above steps

[ArchitBhonsle]

I was on the first task. Any idea how the encoder should be instantiated? The current rust code just copies it over from the context it is passed while the C code does not instantiate it at all. Is it null or am I reading it wrong?

[cfsmp3]

I was on the first task. Any idea how the encoder should be instantiated? The current rust code just copies it over from the context it is passed while the C code does not instantiate it at all. Is it null or am I reading it wrong?

It should start with NULL - please send a PR to correct that in the C code :-) Just a matter of hygiene. If you look at the actual initilization

https://github.com/CCExtractor/ccextractor/search?q=dtvcc-%3Eencoder

There a couple of places in which it happens. But it should start with NULL when the context is created.

[PunitLodha]

As @cfsmp3 mentioned, it should be initialized to NULL. You should also add a function to set the value for encoder field, which is called whenever the value for dtvcc->encoder is updated in the C code.

On a side note, If you are working on this issue, I recommend you to open a draft PR and keep pushing your changes there, so that I can review each step individually. This issue is quite a bit of work, and reviewing a huge PR would be difficult

[ArchitBhonsle]

If could push my progress but because of how I'm working, the commits themselves would fail to build. Is that okay?

[ArchitBhonsle]

Also how do I approach the compatibility issue. The encoder for example. It would be initialized to NULL but in Rust should I use the Rusty approach of using Option and write getters and setters to work with it in C or use std::ptr::null()? I would prefer the former but it might necessitate more changes on the C side.

[ArchitBhonsle]

And other issue is stuff like dtvcc_tv_screen where in C we just allocate memory for it and assign some of the values while the rest are left as garbage. While instantiating these structs in Rust I have to assign all the values, right? Which is fine for simple values but when there's nested structs (like windows in dtvcc_tv_screen) it can get cumbersome.

Edit: For now I have added dervive_default(true) to the bindgen builder which makes this a lot simpler. But do tell me if you guys have any better ideas.

Edit 2: This conflicts with the manually defined Default implementations for dtvcc_pen_attribs and dtvcc_pen_color. I'll rename these to new for now.

[ArchitBhonsle]

@PunitLodha here's the PR #1501. For now I've just rewritten Dtvcc::new().

[ArchitBhonsle]

It should start with NULL - please send a PR to correct that in the C code :-) Just a matter of hygiene. If you look at the actual initilization

https://github.com/CCExtractor/ccextractor/search?q=dtvcc-%3Eencoder

There a couple of places in which it happens. But it should start with NULL when the context is created.

Hey @cfsmp3. Of the 3 code results in the link you have sent only the first two, general_loop.c and mp4.c are initializations that should affect the Rust decoder right?

[cfsmp3]

Hey @cfsmp3. Of the 3 code results in the link you have sent only the first two, general_loop.c and mp4.c are initializations that should affect the Rust decoder right?

I haven't tracked it down, but in general any structure should have all its fields initialized with something when it's created :-)

[ArchitBhonsle]

@PunitLodha for the mp4 part, do I just expose another extern function from Rust to call this or somehow reuse this?

[PunitLodha]

You'll need to expose another extern function

[asher-gh]

Seems like #1501 is not active anymore. What's the best way to get ccx_decoder_dtvcc_settings when calling Dtvcc::new in ccxr_process_cc_data here?

[IshanGrover2004]

I have started working on this issue & I have raised a Draft PR #1599 as it is WIP. Till now I have done the first task which is Changing Dtvcc::new() to create Dtvcc using ccx_decoder_dtvcc_settings

Feedbacks are appreciated