-
Notifications
You must be signed in to change notification settings - Fork 515
[FEAT] Add bitstream module in lib_ccxr
#1649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEAT] Add bitstream module in lib_ccxr
#1649
Conversation
src/lib_ccx/cc_bitstream.h
Outdated
| unsigned char *_i_pos; | ||
| int _i_bpos; | ||
| #ifndef DISABLE_RUST | ||
| void *rust_bs; // Pointer to Rust's Bitstream struct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we construct / deconstruct the struct when passing to rust instead of storing a different pointer?
There are other files, where you can see the to_ctype()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yaa surely, this should be a better approach rather than having to maintain two structures. I will try to fix it in my next commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prateekmedia , I have made changes in the bitstream.rs file , can you please review it now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will check this.
Migration module changes
Minor Formatting Issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new bitstream module implemented in Rust and connects it to the existing C code via FFI.
- Introduces Rust-side bitstream functions and exposes them through the C wrapper
- Updates C bitstream implementation to delegate to Rust when
DISABLE_RUSTis not defined - Adjusts tests and build configuration to include the new module
Reviewed Changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/rust/wrapper.h | Includes the C header for bitstream to expose FFI |
| src/rust/src/libccxr_exports/mod.rs | Registers the bitstream export module |
| src/rust/lib_ccxr/src/util/bits.rs | Refactors parity tests to use assert! |
| src/rust/lib_ccxr/src/common/mod.rs | Adds bitstream to the common mod exports |
| src/rust/build.rs | Links the bitstream library into the build script |
| src/lib_ccx/cc_bitstream.c | Wraps the C implementation to call into Rust |
Comments suppressed due to low confidence (3)
src/lib_ccx/cc_bitstream.c:238
- The function signature returns
unsigned char *but the Rust FFIccxr_next_bytesexports aconst uint8_t *. Consider updating this toconst unsigned char *to preserve theconstqualifier and avoid discarding it.
bstr->bpos = 8;
src/rust/wrapper.h:14
- wrapper.h includes the C header for bitstream but does not declare the extern
ccxr_*functions provided by the Rust bitstream module, so consumers won’t see the Rust FFI symbols. Consider adding extern declarations for eachccxr_*function in this header.
#include "../lib_ccx/cc_bitstream.h"
src/rust/build.rs:30
- The
bitstreamentry may not match the actual C library name produced (likely something likeccx_bitstreamor part of the existingccxlibrary), which could cause linker errors. Verify that the linked library name matches the compiled archive.
"bitstream",
Bitstream: Removed redundant CType
…-flow bitstream: recommended changes for is_byte_aligned
| /// # Safety | ||
| /// This function is unsafe because it: | ||
| /// - Dereferences a raw pointer (`bitstream_ptr`) without null checking | ||
| /// - Assumes `bitstream_ptr` points to a valid, properly initialized and aligned `bitstream` struct | ||
| /// - Assumes the `bitstream` struct has sufficient memory allocated for all fields | ||
| /// - Assumes `rust_bitstream` is properly initialized with valid data | ||
| /// - Performs pointer arithmetic on `rust_bitstream.data` assuming it points to valid memory | ||
| /// - The caller must ensure the `bitstream` struct remains valid for the duration of the operation | ||
| /// - The data referenced by `rust_bitstream.data` must remain valid during the operation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep Safety comments concise to 1 or 2 lines at max.
…-removal bitstream: recommended changes for long comments
prateekmedia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
* feat: Add bitstream module * run code formatters * Run cargo clippy --fix * Run cargo fmt --all * refactor: remove rust pointer from C struct * feat: Add bitstream module * run code formatters * Run cargo clippy --fix * Run cargo fmt --all * refactor: remove rust pointer from C struct * Added Bitstream to libccxr_exports * Minor Formatting Issue * Bitstream: Removed redundant CType * bitstream: recommended changes for is_byte_aligned * bitstream: recommended changes for long comments * bitstream: comment fix * bitstream: removed redundant comparism comments --------- Co-authored-by: Deepnarayan Sett <[email protected]> Co-authored-by: Deepnarayan Sett <[email protected]>
* feat: added demuxer module * Cargo Lock Update * Completed file_functions and demuxer * Completed file_functions and demuxer * written extern functions for demuxer * Removed libc completely, added tests for gxf and ported gxf to C * Hardsubx error fixed * Fixing format issues * clippy errors fixed * fixing format issues * fixing format issues * Windows failing tests * Windows failing tests * demuxer: added demuxer data transfer functions and removed some structs * made Demuxer and File Functions * Minor formatting changes * Minor Rebasing changes * demuxer: format rust and unit test rust checks * C formatting * Windows Failing test * Windows Failing test * Update CHANGES.TXT * Update CHANGES.TXT * Windows Failing Tests * Windows Failing Tests * Problem in Copy to Rust and some typos that copilot review suggested * Minor Formatting Error * Windows Failing Regressions * Windows Failing Regressions * Minor Comment Change * Data transfer module for DemuxerData added and more rustlike syntax to ctorust.rs * Minor Formatting Changes * demuxer: Rebase and a few tweaks to file_functions * demuxer: Minor Formatting Error * [FIX] 134 Codes in XDS and General Tests (#1708) * Made pointers valid in Unit Tests of Decoder * fix: test_do_cb * Copilot Suggestions * Suggestions about Redundancy * Suggestions about Redundancy * [FEAT] Add `bitstream` module in `lib_ccxr` (#1649) * feat: Add bitstream module * run code formatters * Run cargo clippy --fix * Run cargo fmt --all * refactor: remove rust pointer from C struct * feat: Add bitstream module * run code formatters * Run cargo clippy --fix * Run cargo fmt --all * refactor: remove rust pointer from C struct * Added Bitstream to libccxr_exports * Minor Formatting Issue * Bitstream: Removed redundant CType * bitstream: recommended changes for is_byte_aligned * bitstream: recommended changes for long comments * bitstream: comment fix * bitstream: removed redundant comparism comments --------- Co-authored-by: Deepnarayan Sett <[email protected]> Co-authored-by: Deepnarayan Sett <[email protected]> * demuxer: minor formatting changes * Demuxer: Changes to mistakes in CHANGES.txt * Demuxer: Removed extra newline in ccextractor.c * Demuxer: Changes to Encoding resolved * Demuxer: Moved CCX_NOPTS to common structs and some changes to Demuxer Data regd. MPEG_CLOCK_FREQ * some refactoring to CCX_NOPTS * Demuxer: Minor Mistake regarding CHANGES.txt * Demuxer: Unit test rust failing because of CCX_NOPTS * Demuxer: changed common_structs to common_types * Demuxer: Removed redundant libraries from Cargo.toml and moved tempfile to dev-dependencies * Demuxer: Removed to_vec function and renamed PSIBuffer/PMTEntry from_ctype functions * Demuxer: Renamed Stream_Type, improved Time complexity of the default() function and removed redundant comments * Demuxer: Removed two repeated code blocks and removed redundant comments * Demuxer: Removed two code blocks * Demuxer: Review Changes * Demuxer: Removed redundant tests * Update src/rust/src/demuxer/demux.rs Co-authored-by: Prateek Sunal <[email protected]> * Demuxer: Errors due to Rebase * Demuxer: Removed get_stream_mode * Demuxer: Errors due to rebasing and removing redundant CType Functions * Demuxer: Failing ES regressions * Demuxer: MythTV failing regression * Demuxer: Removed redundant comments * Demuxer: Unplugged ES for now * Demuxer: Replugged in ES * Demuxer: Formatting error * Demuxer: Windows failing CI * Demuxer: Windows failing CI * Demuxer: Windows failing Regressions * Demuxer: Formatting * Demuxer: Minor Cargo Clippy change * Demuxer: running regressions again * Demuxer: Cargo Lockfile Change * Demuxer: running regressions again * Demuxer: running regressions again --------- Co-authored-by: Swastik Patel <[email protected]> Co-authored-by: Prateek Sunal <[email protected]>
* feat: added demuxer module * Cargo Lock Update * Completed file_functions and demuxer * Completed file_functions and demuxer * written extern functions for demuxer * Removed libc completely, added tests for gxf and ported gxf to C * Hardsubx error fixed * Fixing format issues * clippy errors fixed * fixing format issues * fixing format issues * Windows failing tests * Windows failing tests * demuxer: added demuxer data transfer functions and removed some structs * made Demuxer and File Functions * Minor formatting changes * Minor Rebasing changes * demuxer: format rust and unit test rust checks * C formatting * Windows Failing test * Windows Failing test * Update CHANGES.TXT * Update CHANGES.TXT * Windows Failing Tests * Windows Failing Tests * Problem in Copy to Rust and some typos that copilot review suggested * Minor Formatting Error * Windows Failing Regressions * Windows Failing Regressions * Minor Comment Change * Data transfer module for DemuxerData added and more rustlike syntax to ctorust.rs * Minor Formatting Changes * demuxer: Rebase and a few tweaks to file_functions * demuxer: Minor Formatting Error * [FIX] 134 Codes in XDS and General Tests (CCExtractor#1708) * Made pointers valid in Unit Tests of Decoder * fix: test_do_cb * Copilot Suggestions * Suggestions about Redundancy * Suggestions about Redundancy * [FEAT] Add `bitstream` module in `lib_ccxr` (CCExtractor#1649) * feat: Add bitstream module * run code formatters * Run cargo clippy --fix * Run cargo fmt --all * refactor: remove rust pointer from C struct * feat: Add bitstream module * run code formatters * Run cargo clippy --fix * Run cargo fmt --all * refactor: remove rust pointer from C struct * Added Bitstream to libccxr_exports * Minor Formatting Issue * Bitstream: Removed redundant CType * bitstream: recommended changes for is_byte_aligned * bitstream: recommended changes for long comments * bitstream: comment fix * bitstream: removed redundant comparism comments --------- Co-authored-by: Deepnarayan Sett <[email protected]> Co-authored-by: Deepnarayan Sett <[email protected]> * demuxer: minor formatting changes * Demuxer: Changes to mistakes in CHANGES.txt * Demuxer: Removed extra newline in ccextractor.c * Demuxer: Changes to Encoding resolved * Demuxer: Moved CCX_NOPTS to common structs and some changes to Demuxer Data regd. MPEG_CLOCK_FREQ * some refactoring to CCX_NOPTS * Demuxer: Minor Mistake regarding CHANGES.txt * Demuxer: Unit test rust failing because of CCX_NOPTS * Demuxer: changed common_structs to common_types * Demuxer: Removed redundant libraries from Cargo.toml and moved tempfile to dev-dependencies * Demuxer: Removed to_vec function and renamed PSIBuffer/PMTEntry from_ctype functions * Demuxer: Renamed Stream_Type, improved Time complexity of the default() function and removed redundant comments * Demuxer: Removed two repeated code blocks and removed redundant comments * Demuxer: Removed two code blocks * Demuxer: Review Changes * Demuxer: Removed redundant tests * Update src/rust/src/demuxer/demux.rs Co-authored-by: Prateek Sunal <[email protected]> * Demuxer: Errors due to Rebase * Demuxer: Removed get_stream_mode * Demuxer: Errors due to rebasing and removing redundant CType Functions * Demuxer: Failing ES regressions * Demuxer: MythTV failing regression * Demuxer: Removed redundant comments * Demuxer: Unplugged ES for now * Demuxer: Replugged in ES * Demuxer: Formatting error * Demuxer: Windows failing CI * Demuxer: Windows failing CI * Demuxer: Windows failing Regressions * Demuxer: Formatting * Demuxer: Minor Cargo Clippy change * Demuxer: running regressions again * Demuxer: Cargo Lockfile Change * Demuxer: running regressions again * Demuxer: running regressions again --------- Co-authored-by: Swastik Patel <[email protected]> Co-authored-by: Prateek Sunal <[email protected]>
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
This PR migrates the bitstream functionality from C to Rust while maintaining backward compatibility by creating a new Rust implementation in
lib_ccxr/src/common/bitstream.rsand connecting it to the existing C code through FFI bindings.