-
Notifications
You must be signed in to change notification settings - Fork 505
fix(windows): Fix c_long ABI mismatch causing Windows CI failures #1873
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The extern declaration for ccxr_add_current_pts used c_long, but the actual implementation in time.rs uses i64. This caused an ABI mismatch on Windows where: - c_long = i32 (32-bit) - i64 = 64-bit On Linux both are 64-bit so it worked, but on Windows the type mismatch could cause incorrect parameter passing. Changes: - Change extern fn declaration from c_long to i64 - Remove unnecessary cast (FRAME_DURATION_TICKS is already i64) - Remove unused c_long import 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The C type 'long' has different sizes on different platforms: - Linux: 64-bit - Windows: 32-bit This causes ABI mismatches when interfacing with Rust, since Rust's c_long matches the platform's long size, but we were treating these values as 64-bit throughout. Changed the following fields from 'long' to 'int64_t': - asf_constants.h: parsebufsize - avc_functions.h: cc_databufsize, num_nal_unit_type_7, num_vcl_hrd, num_nal_hrd, num_jump_in_frames, num_unexpected_sei_length - ccx_decoders_608.h: bytes_processed_608 - ccx_demuxer.h: capbufsize, capbuflen - lib_ccx.h: ts_readstream() return type, FILEBUFFERSIZE - file_functions.c: FILEBUFFERSIZE definition - ts_functions.c: ts_readstream() implementation Also updated Rust code in common.rs to remove c_long casts, since bindgen will now generate i64 for these fields. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
For clarity and consistency, use explicit i64 instead of c_longlong. While c_longlong is 64-bit on all platforms, i64 is clearer and follows the same pattern as the previous commit that removed c_long. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Collaborator
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 25d68b7...:
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes Windows-specific CI test failures caused by platform-dependent type issues in the C/Rust FFI layer.
Problem
The C type
longhas different sizes on different platforms:This caused ABI mismatches when interfacing with Rust, since Rust's
c_longmatches the platform'slongsize.Changes
1. Fix ABI mismatch in demuxer.rs (Commit 1)
The extern declaration for
ccxr_add_current_ptsusedc_long, but the actual implementation usesi64:2. Replace all
longwithint64_tin C code (Commit 2)Changed the following fields from
longtoint64_t:asf_constants.hparsebufsizeavc_functions.hcc_databufsize,num_nal_unit_type_7,num_vcl_hrd,num_nal_hrd,num_jump_in_frames,num_unexpected_sei_lengthccx_decoders_608.hbytes_processed_608ccx_demuxer.hcapbufsize,capbuflenlib_ccx.hts_readstream()return type,FILEBUFFERSIZEAlso updated Rust code in
common.rsto removec_longcasts (bindgen will now generatei64for these fields).Expected Impact
Should fix Windows-specific failures in:
Test Plan
🤖 Generated with Claude Code