[FEAT] added `demuxer` and `file_functions` module #1662

steel-bucket · 2025-02-11T09:29:30Z

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

I have read and understood the contributors guide.
I have checked that another pull request for this purpose does not exist.
I have considered, and confirmed that this submission will be valuable to others.
I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
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.
I am an active contributor to CCExtractor(2 previous contributions).

In this PR, I have attempted to port the large demuxer module to Rust, the primary logic of the heavily interconnected C libraries file_functions.c and, ccx_demuxer.c and their corresponding header files has aleady been implemented here.
This PR was inspired by the ones done for the 708 Decoder in CCextractor.
The part of the codebase that the demuxer part of this PR migrates to Rust is the part that Opens a File(ccx_demuxer_open), points the codebase towards that file, detects the stream type and some other parameters like myth, and then closes the file or gets the file size.
The file_functions part of this PR is tested locally, and in unit tests, but integrating it into C made the codebase really slow, due to the constant copying back and forth C and Rust, so it was left to be used in future Rust Libraries like MythTV, MXF, GXF, etc.
Any criticism or suggestion is wholeheartedly welcome.

It is built and ready to be reviewed now.

prateekmedia · 2025-03-23T13:50:32Z

@steel-bucket Is it still WIP?

steel-bucket · 2025-03-23T15:52:12Z

@steel-bucket Is it still WIP?

Yes, I'm done with the hard part though, file_functions module is fully tested and ready. And the demuxer module just needs a couple more tests. Then I just have the gxf one to do. It won't be long though. Sorry to be late with it, I had some exams which are cleared out now.

steel-bucket · 2025-03-31T07:13:36Z

Hi, @prateekmedia the builds and tests are all working(other than regression). Please review it if time permits. Also should I squash the commits together?
Thank you so much.

Copilot

Pull Request Overview

This PR ports the C demuxer and file_functions modules to Rust, adds corresponding FFI bindings for MXF/GXF, and updates build configuration.

Introduced new Rust modules (demuxer, file_functions) and added them to the crate root.
Extended the C wrapper (wrapper.h) and extern "C" block in lib.rs for MXF/GXF and demuxer functions.
Updated C source (ccx_gxf.*, ccx_demuxer_mxf.*, ccx_demuxer.c) to remove duplicate definitions and conditionally call Rust implementations.

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/rust/wrapper.h	Added headers for GXF and MXF demuxer
src/rust/src/parser.rs	Tightened cast to `usize` for input file capacity check
src/rust/src/libccxr_exports/mod.rs	Exported the new `demuxer` module
src/rust/src/lib.rs	Declared new Rust modules and FFI externs
src/rust/src/file_functions/mod.rs	Added `file_functions` module with documentation stub
src/rust/src/demuxer/common_structs.rs	New demuxer data structures and defaults
src/lib_ccx/ccx_gxf.h	Defined `ccx_gxf` struct for GXF support
src/lib_ccx/ccx_gxf.c	Removed duplicate struct; left stray comments
src/lib_ccx/ccx_demuxer_mxf.h	Added MXF context and type definitions
src/lib_ccx/ccx_demuxer_mxf.c	Cleaned up duplicate MXF definitions
src/lib_ccx/ccx_demuxer.c	Wrapped demuxer calls under `DISABLE_RUST`
docs/CHANGES.TXT	Updated changelog with new entries

Comments suppressed due to low confidence (5)

src/lib_ccx/ccx_demuxer_mxf.c:1

This file uses uint8_t and other fixed-width types but does not include <stdint.h>. Add the proper include to avoid compilation errors.

#define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))

docs/CHANGES.TXT:44

The leading -- appears duplicated in this changelog entry. Use a single - to match the existing formatting.

-- Fix: Unit Test Rust failing due to changes in Rust Version 1.86.0

src/rust/src/lib.rs:43

[nitpick] The imported types c_uchar, c_ulong, and c_void are not used elsewhere in this file; consider removing this import.

use std::os::raw::{c_uchar, c_ulong, c_void};

src/lib_ccx/ccx_gxf.h:9

Ensure this header has proper include guards or #pragma once to prevent duplicate definitions when included multiple times.

struct ccx_gxf

src/lib_ccx/ccx_demuxer_mxf.h:6

Add include guards or #pragma once to this header to prevent multiple inclusion, and verify that <stdint.h> is included for uint8_t.

typedef uint8_t UID[16];

Copilot · 2025-06-12T10:09:55Z

src/rust/src/file_functions/mod.rs

+ * @param ctx ccx_demuxer context properly initilaized ccx_demuxer with some input
+ *            Not to be NULL, since ctx is derefrenced inside this function
+ *
+ * @param buffer if buffer then it must be allocated to at;east bytes len as


Fix typos in the documentation: change "initilaized" to "initialized", "derefrenced" to "dereferenced", and "at;east" to "at least".

Suggested change

* @param ctx ccx_demuxer context properly initilaized ccx_demuxer with some input

* Not to be NULL, since ctx is derefrenced inside this function

*

* @param buffer if buffer then it must be allocated to at;east bytes len as

* @param ctx ccx_demuxer context properly initialized ccx_demuxer with some input

* Not to be NULL, since ctx is dereferenced inside this function

*

* @param buffer if buffer then it must be allocated to at least bytes len as

Copilot · 2025-06-12T10:09:56Z

src/rust/src/demuxer/common_structs.rs

+    #[allow(unused)] // used in tests
+    pub(crate) fn default() -> PSIBuffer {
+        PSIBuffer {
+            prev_ccounter: 0,
+            buffer: Box::into_raw(Box::new(0u8)),
+            buffer_length: 0,
+            ccounter: 0,
+        }
+    }


You have both an impl Default for PSIBuffer and an inherent default() method; consider removing the duplicate inherent method or merging them to avoid confusion.

Suggested change

#[allow(unused)] // used in tests

pub(crate) fn default() -> PSIBuffer {

PSIBuffer {

prev_ccounter: 0,

buffer: Box::into_raw(Box::new(0u8)),

buffer_length: 0,

ccounter: 0,

}

}

Copilot · 2025-06-12T10:09:56Z

src/lib_ccx/ccx_gxf.c

@@ -1000,6 +951,7 @@ static int parse_ad_vbi(struct ccx_demuxer *demux, int len, struct demuxer_data
 	if (result != len)
 		ret = CCX_EOF;
 	return ret;
+	// #endif


[nitpick] Remove stray commented // #endif lines inside function bodies to clean up leftover conditional compilation markers.

Suggested change

// #endif

prateekmedia · 2025-08-06T16:53:59Z

src/rust/Cargo.toml

@@ -27,7 +28,11 @@ cfg-if = "1.0.0"
 num-integer = "0.1.46"
 lib_ccxr = { path = "lib_ccxr" }
 url = "2.5.4"
-encoding_rs = "0.8.5"
+tempfile = "3.20.0"


Add this to dev-dependencies if it is only used for tests.

moved to dev-dependencies

prateekmedia · 2025-08-06T16:54:40Z

src/rust/Cargo.toml

+memoffset = "0.9.1"
+byteorder = "1.5.0"


I can't find them being used in the code.

prateekmedia · 2025-08-06T16:55:11Z

src/rust/Cargo.toml

@@ -13,6 +13,7 @@ crate-type = ["staticlib"]
 [dependencies]
 log = "0.4.26"
 env_logger = "0.8.4"
+iconv = "0.1.1"


Can't seem to find it in the rust code.

prateekmedia · 2025-08-06T16:58:47Z

src/rust/src/ctorust.rs

+/// This function is unsafe because it takes a raw pointer to a C struct.
+pub unsafe fn from_ctype_PSI_buffer(buffer_ptr: *mut PSI_buffer) -> Option<*mut PSIBuffer> {
+    if buffer_ptr.is_null() {
+        return None;
+    }
+
+    // Safety: We've checked that the pointer is not null
+    let buffer = unsafe { &*buffer_ptr };
+
+    // Create a new PSIBuffer
+    let psi_buffer = PSIBuffer {
+        prev_ccounter: buffer.prev_ccounter,
+        buffer: buffer.buffer,
+        buffer_length: buffer.buffer_length,
+        ccounter: buffer.ccounter,
+    };
+
+    // Box it and convert to raw pointer
+    Some(Box::into_raw(Box::new(psi_buffer)))
+}
+/// # Safety
+/// This function is unsafe because it takes a raw pointer to a C struct.
+pub unsafe fn from_ctype_PMT_entry(buffer_ptr: *mut PMT_entry) -> Option<*mut PMTEntry> {
+    if buffer_ptr.is_null() {
+        return None;
+    }
+
+    let buffer = unsafe { &*buffer_ptr };
+
+    let program_number = if buffer.program_number != 0 {
+        buffer.program_number
+    } else {
+        0
+    };
+
+    let elementary_pid = if buffer.elementary_PID != 0 {
+        buffer.elementary_PID
+    } else {
+        0
+    };
+
+    let stream_type = if buffer.stream_type != 0 {
+        StreamType::from_ctype(buffer.stream_type as u32).unwrap_or(StreamType::Unknownstream)
+    } else {
+        StreamType::Unknownstream
+    };
+
+    let printable_stream_type = if buffer.printable_stream_type != 0 {
+        buffer.printable_stream_type
+    } else {
+        0
+    };
+
+    let mut pmt_entry = PMTEntry {
+        program_number,
+        elementary_pid,
+        stream_type,
+        printable_stream_type,
+    };
+
+    Some(&mut pmt_entry as *mut PMTEntry)
+}
+pub fn from_ctype_DebugMessageMask(mask_on_normal: u32, mask_on_debug: u32) -> DebugMessageMask {
+    DebugMessageMask::new(
+        DebugMessageFlag::from_bits_truncate(mask_on_normal as u16),
+        DebugMessageFlag::from_bits_truncate(mask_on_debug as u16),
+    )
+}


These doesn't seem to be following the pattern of FromC, but they are declared as methods only. Any reason for this?

prateekmedia · 2025-08-06T16:59:33Z

src/rust/src/ctorust.rs

+// Helper function for array conversion
+fn c_array_to_vec(c_array: &[c_uint; 128usize]) -> Vec<u32> {
+    c_array.to_vec()
+}


Move to separate file as this seems different? If you feel this belong here only then move to top.

resolved(It was just being used once and was pretty redundant so I removed it)

prateekmedia · 2025-08-06T17:00:04Z

src/rust/src/ctorust.rs

+// Helper struct for DtvccServiceCharset conversion
+pub struct DtvccServiceCharsetArgs {
+    pub services_charsets: *mut *mut ::std::os::raw::c_char,
+    pub all_services_charset: *mut ::std::os::raw::c_char,
+}


This doesn't belong here.

Can you please tell me where could I place it? It's only used in this file so that we could pass 2 pointers into FromCType instead of just one.

prateekmedia · 2025-08-06T17:06:21Z

src/rust/src/demuxer/common_types.rs

+pub const CCX_NOPTS: i64 = 0x8000_0000_0000_0000u64 as i64;
+
+#[repr(u32)]
+pub enum Stream_Type {


This seem very weird casing, it should be CamelCase, like:
StreamType

Actually there are 2 stream types
I've named it this way.
ccx_stream_type => StreamType
STREAM_TYPE => Stream_Type

But it's wrong way to write it, you'll need to rename one whilst maintaining meaning and consistency.

Renamed to DemuxerStreamType

prateekmedia · 2025-08-06T17:08:00Z

src/rust/src/demuxer/common_types.rs

+    pub initialized_ocr: bool, // Avoid initializing the OCR more than once
+    pub analysed_pmt_once: u8, // 1-bit field
+    pub version: u8,
+    pub saved_section: [u8; 1021],


No magic numbers, we can take out 1021 as a constant, since it is not something that's general.

prateekmedia · 2025-08-06T17:14:52Z

src/rust/src/demuxer/common_types.rs

+            let mut pi = ProgramInfo {
+                has_all_min_pts: false,
+                ..Default::default()
+            };
+            for j in 0..(Stream_Type::Count as usize) {
+                pi.got_important_streams_min_pts[j] = u64::MAX;
+            }
+            pi.initialized_ocr = false;
+            pi.version = 0xFF; // “not initialized” marker
+                               // pid and program_number remain zero for now
+            pinfo_vec.push(pi);


Can we do something to not run it for MAX_PROGRAM times cause you are just creating a similar looking pi, maybe just create and assign it once and deep copy it to pinfo_vec for each value.

prateekmedia · 2025-08-06T18:13:09Z

src/rust/src/libccxr_exports/demuxerdata.rs

+
+    (*c_data).rollover_bits = rust_data.rollover_bits as c_uint;
+    (*c_data).pts = rust_data.pts as i64;
+    (*c_data).tb = rust_data.tb.to_ctype(); // Assuming Into trait is implemented
