-
Notifications
You must be signed in to change notification settings - Fork 505
Smptett encoder #1814
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
base: master
Are you sure you want to change the base?
Smptett encoder #1814
Conversation
…borrow fix, unimplemented stubs
- Implemented write_cc_buffer_as_smptett with full styling support * Handles italics, bold, underline, and font color styling * Calculates proper row/column positioning for SMPTE-TT * Converts HTML-style tags to SMPTE-TT style attributes - Implemented write_cc_subtitle_as_smptett * Processes linked list of subtitle structures * Properly manages memory and frees C-allocated data * Handles CC_TEXT subtitle types - Implemented write_cc_bitmap_as_smptett * Feature-gated for OCR support (hardsubx_ocr) * Proper cleanup of bitmap data - Added helper functions: * escape_xml() for XML entity escaping * write_stringz_as_smptett() for core formatting * process_line_styling() for style tag conversion * write_wrapped() for safe file I/O - Added comprehensive test coverage: * XML escaping tests * Styling conversion tests (italic, bold, no styling) * All tests passing (268 total) Matches C implementation behavior while using safe Rust idioms. Fixes CCExtractor#1789
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit 134cd75...:
Your PR breaks these cases:
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:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Check the result page for more info. |
- Exported write_stringz_as_smptett with #[no_mangle] for C calls - All three main functions now properly exported - Renamed C implementation to .bak (kept as reference) - Successfully builds and links with C code - All 268 tests passing
bd8a8ab to
2264917
Compare
The SMPTE-TT encoder has been ported to Rust. Exclude the C file from CMake builds to avoid duplicate symbols. The C file is still needed for autoconf builds.
This reverts commit 8ee175a.
- Remove PKG_CONFIG_FOUND condition for Rust build - Conditionally exclude C file only when Rust library is built - Fix CMake syntax errors (removed duplicate target_link_libraries) - Rust SMPTE-TT encoder successfully linked and working - All 4 SMPTE-TT functions exported from Rust with #[no_mangle] - Builds successfully on macOS
- Remove conditional logic, always exclude ccx_encoders_smptett.c - Align with Rust-required configuration - Prevent duplicate symbol conflicts
- Fix CMake to properly detect and use cargo instead of rustc - Add macOS-specific frameworks (Security, CoreFoundation) for Rust linking - Build Rust library during CMake configuration with execute_process - Link Rust static library with proper system dependencies - Exclude C SMPTE-TT implementation when Rust is available Build tested successfully on macOS with Apple Silicon. Fixes macOS build failures.
The build.command script was compiling both the C and Rust implementations of SMPTE-TT encoder, causing duplicate symbol errors during linking. Modified SRC_CCX collection to exclude ccx_encoders_smptett.c using grep -v, matching the exclusion already done in CMakeLists.txt. This fixes the Mac build_shell CI failure.
Mac CMake CI jobs were failing because Rust was not installed. Added Rust installation step to both cmake and cmake_ocr_hardsubx jobs using rustup, matching the approach used in Linux workflows. This fixes the Mac CMake build failures.
Previously was using execute_process to manually build Rust and set library paths, which conflicted with Corrosion's automatic handling in src/rust/CMakeLists.txt. Changes: - Removed duplicate execute_process(cargo build) call - Removed manual RUST_LIB path setting - Now properly link to 'ccx_rust' target created by Corrosion - Keep platform-specific system libraries (Security/CoreFoundation for macOS) This should fix the CMake CI build failures on Linux and Mac.
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit a0593c6...:
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:
All tests passing on the master branch were passed completely. Check the result page for more info. |
The Rust installation adds cargo to GITHUB_PATH, but this only takes effect in subsequent steps. CMake runs in the same step and needs to source the cargo environment explicitly to find the cargo binary. Added 'source $HOME/.cargo/env' before cmake commands in both: - cmake job (basic build) - cmake_ocr_hardsubx job (build with OCR and hardsubx) This fixes the Linux CMake CI build failures.
All Linux CI jobs now install Rust before building: - build_shell - build_autoconf - cmake - cmake_ocr_hardsubx - build_rust Each job installs Rust using rustup and sources the cargo environment immediately, making cargo available for subsequent build steps. Removed redundant 'source $HOME/.cargo/env' from cmake run commands since it's already done in the Install Rust step. This fixes all Linux CMake CI build failures.
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
{pull request content here}
I am still working on it