Simplify the library API calls using the windows-link macro.#2
Merged
Simplify the library API calls using the windows-link macro.#2
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR replaces dynamic library loading with static linking via the windows-link macro, updates FFI signatures to use *mut c_void and i32, and removes redundant loading macros and the libloading crate.
- Transition all FFI bindings to
windows-link::link!and droplibloading - Change pointer and return types in FFI signatures and update corresponding method signatures
- Remove custom
load_symbol!/release_ocr_resource!macros and adjust resource cleanup
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/ocr_word.rs | Updated OcrWord::new to use *mut c_void, removed libloading and symbol-loading macros |
| src/ocr_result.rs | Simplified OcrResult to hold *mut c_void, removed lifetimes and release macro |
| src/ocr_line.rs | Updated OcrLine to use *mut c_void, removed dynamic symbol loading |
| src/ocr_engine.rs | Replaced dynamic loading with direct calls, changed option pointers to *mut c_void and return types to i32 |
| src/lib.rs | Removed load_symbol!/release_ocr_resource! macro definitions |
| src/ffi.rs | Added link! definitions for all FFI functions, updated signatures |
| src/errors.rs | Changed OcrApiError result field to i32 |
| Cargo.toml | Dropped libloading, added windows-link dependency |
Comments suppressed due to low confidence (3)
src/errors.rs:16
- [nitpick] The error message is confusing and repeats 'result'. Consider rephrasing to something like
Failed to run OCR API (code: {result}): {message}for clarity.
#[error("Failed to run ocr API {result}, result: {message}")]
src/ocr_engine.rs:85
- [nitpick] Consider adding unit tests for
get_max_recognition_line_count(and other newly updated methods) to verify the static FFI bindings and return type conversions behave as expected.
pub fn get_max_recognition_line_count(&self) -> Result<i32, OneOcrError> {
src/ffi.rs:5
- The FFI signature for
OcrInitOptionsSetUseModelDelayLoadno longer accepts the model delay-load flag (c_char), causing a mismatch with the underlying C API and potential undefined behavior. Restore the second parameter for the flag and update calls accordingly.
link!("oneocr.dll" "system" fn OcrInitOptionsSetUseModelDelayLoad(init_option: *mut c_void) -> i32);
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
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.
libloadingcratelink!macro from thewindows-linkcrate to simplify the API calls greatlyThis pull request introduces significant changes to the OneOCR codebase, focusing on replacing the dynamic library loading approach with a static linking mechanism using the
windows-linkcrate. This simplifies the codebase, improves type safety, and enhances maintainability. Key changes include the removal of macros for symbol loading, updates to FFI function signatures, and adjustments to error handling and type definitions.Transition to Static Linking
libloading) with static linking using thewindows-linkcrate for all FFI functions insrc/ffi.rs, improving type safety and reducing runtime errors.Updates to FFI Function Signatures and Types
*mut c_voidinstead ofi64for pointers, aligning with the new static linking approach. This change affects multiple methods insrc/ffi.rsandsrc/ocr_engine.rs. [1] [2] [3] [4]OcrApiErrortype insrc/errors.rsto usei32instead ofi64for theresultfield, reflecting the updated FFI function return types.Removal of Redundant Code
load_symbolandrelease_ocr_resource) used for dynamic symbol loading insrc/lib.rs, as they are no longer needed with static linking.libloadingimports and references in files such assrc/ocr_engine.rsandsrc/ocr_line.rs. [1] [2]Adjustments to
OcrEngineOcrEnginestruct by removing theLibraryfield, as dynamic library management is no longer required. Updated related methods to directly call statically linked functions. [1] [2]get_max_recognition_line_count,set_resize_resolution, andrun_pipelineto reflect the new FFI function signatures and type changes. [1] [2] [3]Adjustments to
OcrLineOcrLinestruct to use*mut c_voidfor theline_handlefield and modified related methods to use the new static linking approach. [1] [2]get_line_stylemethod by removing dynamic symbol loading and directly calling the statically linked function.