Initialize data structures correctly for the rust CEA-708 decoder #748

New Issue

claunia · 2026-01-29T16:52:40Z

claunia commented

2026-01-29 16:52:40 +00:00

Originally created by @PunitLodha on GitHub (Mar 14, 2023).

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

Originally created by @PunitLodha on GitHub (Mar 14, 2023). Currently, the entry point for the rust decoder is [here](https://github.com/CCExtractor/ccextractor/blob/master/src/rust/src/lib.rs#L53-L75) 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](https://github.com/CCExtractor/ccextractor/blob/master/src/rust/src/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](https://github.com/CCExtractor/ccextractor/blob/master/src/rust/src/lib.rs#L53-L75) to use `Dtvcc` from `dec_ctx` instead of creating a new one Additionally to fix mp4 code flow:- - [ ] Call the corresponding rust function [here](https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/mp4.c#L398) and pass `Dtvcc` as done in the above steps

claunia added the CEA-708 label 2026-01-29 16:52:40 +00:00

claunia closed this issue

2026-01-29 16:52:40 +00:00

claunia commented

2026-01-29 16:52:40 +00:00

@ArchitBhonsle commented on GitHub (Mar 15, 2023):

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?

@ArchitBhonsle commented on GitHub (Mar 15, 2023): I was on the first task. Any idea how the [`encoder`](https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_decoders_708.h#L359) should be instantiated? The [current rust code](https://github.com/CCExtractor/ccextractor/blob/master/src/rust/src/decoder/mod.rs#L44) just copies it over from the context it is passed while the [C code](https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_dtvcc.c#L76-L124) does not instantiate it at all. Is it null or am I reading it wrong?

claunia commented

2026-01-29 16:52:41 +00:00

@cfsmp3 commented on GitHub (Mar 15, 2023):

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.

@cfsmp3 commented on GitHub (Mar 15, 2023): > I was on the first task. Any idea how the [`encoder`](https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_decoders_708.h#L359) should be instantiated? The [current rust code](https://github.com/CCExtractor/ccextractor/blob/master/src/rust/src/decoder/mod.rs#L44) just copies it over from the context it is passed while the [C code](https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_dtvcc.c#L76-L124) 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.

claunia commented

2026-01-29 16:52:41 +00:00

@PunitLodha commented on GitHub (Mar 15, 2023):

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

@PunitLodha commented on GitHub (Mar 15, 2023): 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

claunia commented

2026-01-29 16:52:41 +00:00

@ArchitBhonsle commented on GitHub (Mar 16, 2023):

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

@ArchitBhonsle commented on GitHub (Mar 16, 2023): If could push my progress but because of how I'm working, the commits themselves would fail to build. Is that okay?

claunia commented

2026-01-29 16:52:41 +00:00

@ArchitBhonsle commented on GitHub (Mar 16, 2023):

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 commented on GitHub (Mar 16, 2023): 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.

claunia commented

2026-01-29 16:52:41 +00:00

@ArchitBhonsle commented on GitHub (Mar 16, 2023):

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 commented on GitHub (Mar 16, 2023): And other issue is stuff like [`dtvcc_tv_screen`](https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/ccx_dtvcc.c#L111) 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.

claunia commented

2026-01-29 16:52:42 +00:00

@ArchitBhonsle commented on GitHub (Mar 16, 2023):

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

@ArchitBhonsle commented on GitHub (Mar 16, 2023): @PunitLodha here's the PR #1501. For now I've just rewritten `Dtvcc::new()`.

claunia commented

2026-01-29 16:52:42 +00:00

@ArchitBhonsle commented on GitHub (Mar 18, 2023):

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?

@ArchitBhonsle commented on GitHub (Mar 18, 2023): > 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?

claunia commented

2026-01-29 16:52:43 +00:00

@cfsmp3 commented on GitHub (Mar 18, 2023):

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 :-)

@cfsmp3 commented on GitHub (Mar 18, 2023): > > 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 :-)

claunia commented

2026-01-29 16:52:43 +00:00

@ArchitBhonsle commented on GitHub (Apr 11, 2023):

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

@ArchitBhonsle commented on GitHub (Apr 11, 2023): @PunitLodha for the mp4 part, do I just expose another extern function from Rust to call [this](https://github.com/ArchitBhonsle/ccextractor/blob/c9d9e2357d1223872e0aca7d47c73f61e13b86f6/src/rust/src/decoder/mod.rs#L127) or somehow reuse [this](https://github.com/ArchitBhonsle/ccextractor/blob/c9d9e2357d1223872e0aca7d47c73f61e13b86f6/src/rust/src/lib.rs#L85)?

claunia commented

2026-01-29 16:52:43 +00:00

@PunitLodha commented on GitHub (Apr 12, 2023):

You'll need to expose another extern function

@PunitLodha commented on GitHub (Apr 12, 2023): You'll need to expose another extern function

claunia commented

2026-01-29 16:52:43 +00:00

@asher-gh commented on GitHub (Jan 16, 2024):

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?

@asher-gh commented on GitHub (Jan 16, 2024): 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](https://github.com/CCExtractor/ccextractor/blob/c55072677875ee766c6cf388da70eff0a686ca62/src/rust/src/lib.rs#L64C1-L64C43)?

claunia commented

2026-01-29 16:52:43 +00:00

@IshanGrover2004 commented on GitHub (Feb 20, 2024):

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

@IshanGrover2004 commented on GitHub (Feb 20, 2024): 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

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/ccextractor#748