Skip to content

Commit 2fa366e

Browse files
committed
refactor(rust): fix Provider lifecycle, improve cross-platform scripts, and address PR #44 review feedback
Rust bindings improvements: - Provider::open_device: clean up callbacks before destroying handle (prevents UAF/leaks) - set_resolution: add rollback logic to avoid partial state updates if height set fails - Global error callback: clarify process-global semantics with alias APIs - Add set_global_error_callback() and clear_global_error_callback() aliases - Always clear C-side callback in clear_error_callback() Build and test script improvements: - build_and_test.sh: cross-platform CPU core detection (nproc → sysctl/getconf fallback) - build_and_test.sh: robust CLI binary discovery (handle .exe on Windows) - build_and_test.sh: remove duplicate echo output - test_remote_crate.sh: fix cargo metadata version detection (remove --no-deps) - test_remote_crate.sh: improve cargo update error handling message Documentation fixes: - Wrap bare URLs in angle brackets (MD034) - Fix list indentation and table formatting (MD007/MD060) - Add blank lines around list features (MD032) Verified with: - cargo fmt (format all code) - cargo test (Rust bindings pass) - markdownlint (no errors remaining)
1 parent 6b2c5a7 commit 2fa366e

5 files changed

Lines changed: 88 additions & 34 deletions

File tree

README.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ int main() {
212212
213213
Rust bindings are available as a crate on crates.io:
214214
215-
- Crate: https://crates.io/crates/ccap-rs
216-
- Docs: https://docs.rs/ccap-rs
215+
- Crate: <https://crates.io/crates/ccap-rs>
216+
- Docs: <https://docs.rs/ccap-rs>
217217
- Source: `bindings/rust/`
218218
219219
Quick install:
@@ -256,6 +256,7 @@ cmake --build .
256256
```
257257
258258
**Key Features:**
259+
259260
- 📷 List and select camera devices
260261
- 🎯 Capture single or multiple images
261262
- 👁️ Real-time preview window (with GLFW)
@@ -265,12 +266,13 @@ cmake --build .
265266
- ⏱️ Duration-based or count-based capture modes
266267
- 🔁 Video looping and playback speed control
267268
269+
268270
For complete CLI documentation, see [CLI Tool Guide](./docs/content/cli.md).
269271
270272
## System Requirements
271273
272274
| Platform | Compiler | System Requirements |
273-
|----------|----------|---------------------|
275+
| -------- | -------- | ------------------- |
274276
| **Windows** | MSVC 2019+ (including 2026) / MinGW-w64 | DirectShow |
275277
| **macOS** | Xcode 11+ | macOS 10.13+ |
276278
| **iOS** | Xcode 11+ | iOS 13.0+ |
@@ -290,7 +292,7 @@ For complete CLI documentation, see [CLI Tool Guide](./docs/content/cli.md).
290292
## Examples
291293
292294
| Example | Description | Language | Platform |
293-
|---------|-------------|----------|----------|
295+
| ------- | ----------- | -------- | -------- |
294296
| [0-print_camera](./examples/desktop/0-print_camera.cpp) / [0-print_camera_c](./examples/desktop/0-print_camera_c.c) | List available cameras | C++ / C | Desktop |
295297
| [1-minimal_example](./examples/desktop/1-minimal_example.cpp) / [1-minimal_example_c](./examples/desktop/1-minimal_example_c.c) | Basic frame capture | C++ / C | Desktop |
296298
| [2-capture_grab](./examples/desktop/2-capture_grab.cpp) / [2-capture_grab_c](./examples/desktop/2-capture_grab_c.c) | Continuous capture | C++ / C | Desktop |

bindings/rust/README.md

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ If you want the crate name in code to be `ccap` (recommended), set it explicitly
3131
ccap = { package = "ccap-rs", version = "<latest>" }
3232
```
3333

34-
> Tip: Replace `<latest>` with the latest version shown on https://crates.io/crates/ccap-rs
34+
> Tip: Replace `<latest>` with the latest version shown on <https://crates.io/crates/ccap-rs>
3535
3636
### Basic Usage
3737

@@ -94,12 +94,8 @@ cargo run --example capture_callback
9494

9595
This crate supports two build modes:
9696

97-
- **Distribution mode (default):** `build-source`
98-
- Builds the native C/C++ implementation via the `cc` crate.
99-
- Intended for crates.io users.
100-
- **Development mode:** `static-link`
101-
- Links against a pre-built native library from a CameraCapture checkout (e.g. `build/Debug/libccap.a`).
102-
- Intended for developing this repository.
97+
- **Distribution mode (default):** `build-source` — Builds the native C/C++ implementation via the `cc` crate (intended for crates.io users).
98+
- **Development mode:** `static-link` — Links against a pre-built native library from a CameraCapture checkout (e.g. `build/Debug/libccap.a`) (intended for developing this repository).
10399

104100
### Prerequisites
105101

@@ -142,21 +138,16 @@ An ASan-instrumented `libccap.a` requires the ASan runtime at link/run time.
142138

143139
- When using `static-link`, `build.rs` will **only** link the ASan runtime **if it detects** ASan symbols inside the prebuilt `libccap.a`.
144140
- This does **not** affect the default crates.io build (`build-source`).
145-
- You can disable the auto-link behavior by setting:
146-
- `CCAP_RUST_NO_ASAN_LINK=1`
141+
- You can disable the auto-link behavior by setting `CCAP_RUST_NO_ASAN_LINK=1`.
147142

148143
## Feature flags
149144

150145
- `build-source` (default): build the C/C++ ccap sources during `cargo build` (best for crates.io usage).
151-
- `static-link`: link against a pre-built static library from a CameraCapture checkout (best for development).
152-
- If you use this mode, make sure you have built the C/C++ project first, and set `CCAP_SOURCE_DIR` when needed.
146+
- `static-link`: link against a pre-built static library from a CameraCapture checkout (best for development). If you use this mode, make sure you have built the C/C++ project first, and set `CCAP_SOURCE_DIR` when needed.
153147

154148
## Platform notes
155149

156-
- Camera capture:
157-
- Windows: DirectShow
158-
- macOS/iOS: AVFoundation
159-
- Linux: V4L2
150+
- Camera capture: Windows (DirectShow), macOS/iOS (AVFoundation), Linux (V4L2)
160151
- Video file playback support depends on the underlying C/C++ library backend (currently Windows/macOS only).
161152

162153
## API Documentation
@@ -193,12 +184,12 @@ match provider.grab_frame(3000) { // 3 second timeout
193184
194185
## Platform Support
195186

196-
| Platform | Backend | Status |
197-
|----------|---------|---------|
198-
| Windows | DirectShow | ✅ Supported |
187+
| Platform | Backend | Status |
188+
| -------- | ------------ | ------------ |
189+
| Windows | DirectShow | ✅ Supported |
199190
| macOS | AVFoundation | ✅ Supported |
200191
| iOS | AVFoundation | ✅ Supported |
201-
| Linux | V4L2 | ✅ Supported |
192+
| Linux | V4L2 | ✅ Supported |
202193

203194
## System Requirements
204195

bindings/rust/build_and_test.sh

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,25 @@
44

55
set -e
66

7+
jobs_count() {
8+
# Cross-platform parallelism detection.
9+
if command -v nproc >/dev/null 2>&1; then
10+
nproc
11+
return
12+
fi
13+
14+
if command -v sysctl >/dev/null 2>&1; then
15+
# macOS / BSD
16+
sysctl -n hw.ncpu 2>/dev/null && return
17+
fi
18+
19+
if command -v getconf >/dev/null 2>&1; then
20+
getconf _NPROCESSORS_ONLN 2>/dev/null && return
21+
fi
22+
23+
echo 4
24+
}
25+
726
detect_cli_devices() {
827
local cli_bin=""
928
local original_dir
@@ -27,12 +46,18 @@ detect_cli_devices() {
2746

2847
# Reconfigure with CLI enabled (idempotent)
2948
cmake ../.. -DCMAKE_BUILD_TYPE=Debug -DCCAP_BUILD_CLI=ON
30-
cmake --build . --config Debug --target ccap-cli -- -j"$(nproc 2>/dev/null || echo 4)"
49+
cmake --build . --config Debug --target ccap-cli -- -j"$(jobs_count)"
3150

3251
popd >/dev/null
3352
popd >/dev/null
3453

35-
cli_bin="$PROJECT_ROOT/build/Debug/ccap"
54+
if [ -x "$PROJECT_ROOT/build/Debug/ccap" ]; then
55+
cli_bin="$PROJECT_ROOT/build/Debug/ccap"
56+
elif [ -x "$PROJECT_ROOT/build/Debug/ccap.exe" ]; then
57+
cli_bin="$PROJECT_ROOT/build/Debug/ccap.exe"
58+
else
59+
cli_bin="$PROJECT_ROOT/build/Debug/ccap"
60+
fi
3661
fi
3762

3863
cd "$original_dir"
@@ -94,7 +119,7 @@ if [ ! -f "$PROJECT_ROOT/build/Debug/libccap.a" ] && [ ! -f "$PROJECT_ROOT/build
94119
cmake ../.. -DCMAKE_BUILD_TYPE=Debug
95120
fi
96121

97-
cmake --build . --config Debug -- -j"$(nproc 2>/dev/null || echo 4)"
122+
cmake --build . --config Debug -- -j"$(jobs_count)"
98123

99124
cd "$RUST_DIR"
100125
else
@@ -113,7 +138,6 @@ cargo clean
113138
echo "Building with default features..."
114139
cargo build
115140

116-
117141
# Run tests
118142
echo ""
119143
echo "Step 3: Running tests..."
@@ -203,4 +227,3 @@ echo " cargo run --example capture_callback"
203227
echo ""
204228
echo "To use in your project, add to Cargo.toml:"
205229
echo ' ccap = { package = "ccap-rs", path = "'$RUST_DIR'" }'
206-
echo ' ccap = { package = "ccap-rs", path = "'$RUST_DIR'" }'

bindings/rust/src/provider.rs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,11 @@ impl Provider {
212212
if let Some(name) = device_name {
213213
// Recreate provider with specific device
214214
if !self.handle.is_null() {
215+
// If the previous provider was running, stop it and detach callbacks
216+
// before destroying the underlying handle.
217+
let _ = self.stop_capture();
218+
let _ = self.remove_new_frame_callback();
219+
self.cleanup_callback();
215220
unsafe {
216221
sys::ccap_provider_destroy(self.handle);
217222
}
@@ -284,8 +289,18 @@ impl Provider {
284289

285290
/// Set camera resolution
286291
pub fn set_resolution(&mut self, width: u32, height: u32) -> Result<()> {
292+
// Avoid leaving the device in a partially-updated state if only one property update
293+
// succeeds (e.g. width succeeds but height fails).
294+
let (old_w, old_h) = self.resolution()?;
295+
287296
self.set_property(PropertyName::Width, width as f64)?;
288-
self.set_property(PropertyName::Height, height as f64)?;
297+
if let Err(e) = self.set_property(PropertyName::Height, height as f64) {
298+
// Best-effort rollback.
299+
let _ = self.set_property(PropertyName::Width, old_w as f64);
300+
let _ = self.set_property(PropertyName::Height, old_h as f64);
301+
return Err(e);
302+
}
303+
289304
Ok(())
290305
}
291306

@@ -383,6 +398,9 @@ impl Provider {
383398
/// This is a **global** callback that persists until replaced or cleared.
384399
/// Calling this function multiple times will properly clean up the previous callback.
385400
///
401+
/// **Important**: this callback is process-global (shared by all `Provider` instances).
402+
/// The last one set wins.
403+
///
386404
/// # Thread Safety
387405
///
388406
/// The callback will be invoked from the camera capture thread. Ensure your
@@ -443,6 +461,16 @@ impl Provider {
443461
}
444462
}
445463

464+
/// Set the **global** error callback.
465+
///
466+
/// This is an alias for [`Provider::set_error_callback`] to make the global scope explicit.
467+
pub fn set_global_error_callback<F>(callback: F)
468+
where
469+
F: Fn(i32, &str) + Send + Sync + 'static,
470+
{
471+
Self::set_error_callback(callback)
472+
}
473+
446474
/// Clear the global error callback
447475
///
448476
/// This removes the error callback and frees associated memory.
@@ -451,15 +479,25 @@ impl Provider {
451479

452480
// Use module-level GLOBAL_ERROR_CALLBACK (same as set_error_callback)
453481
if let Ok(mut guard) = GLOBAL_ERROR_CALLBACK.lock() {
482+
// Always clear the C-side callback even if we don't have a stored Rust callback.
483+
unsafe {
484+
sys::ccap_set_error_callback(None, ptr::null_mut());
485+
}
454486
if let Some(SendSyncPtr(old_ptr)) = guard.take() {
455487
unsafe {
456488
let _ = Box::from_raw(old_ptr as *mut ErrorCallbackBox);
457-
sys::ccap_set_error_callback(None, ptr::null_mut());
458489
}
459490
}
460491
}
461492
}
462493

494+
/// Clear the **global** error callback.
495+
///
496+
/// This is an alias for [`Provider::clear_error_callback`] to make the global scope explicit.
497+
pub fn clear_global_error_callback() {
498+
Self::clear_error_callback()
499+
}
500+
463501
/// Open device with index and auto start
464502
pub fn open_with_index(&mut self, device_index: i32, auto_start: bool) -> Result<()> {
465503
// Destroy old handle if exists

scripts/test_remote_crate.sh

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,13 @@ PY
117117

118118
# Get the actual version being used
119119
log "Running cargo update..."
120-
if ! cargo update --quiet 2>&1; then
121-
warn "cargo update had warnings/errors, continuing..."
120+
if ! cargo update --quiet; then
121+
warn "cargo update failed (possibly offline or registry issues). Continuing..."
122122
fi
123123

124124
local actual_version
125-
actual_version=$(cargo metadata --format-version=1 --no-deps 2>/dev/null |
126-
python3 -c "import sys, json; data = json.load(sys.stdin); pkg = next((p for p in data.get('packages', []) if p.get('name') == 'ccap-rs'), None); print(pkg['version'] if pkg else 'unknown')" 2>/dev/null || echo "unknown")
125+
actual_version=$(cargo metadata --format-version=1 2>/dev/null |
126+
python3 -c "import sys, json; data = json.load(sys.stdin); pkgs = data.get('packages', []); pkg = next((p for p in pkgs if p.get('name') == 'ccap-rs'), None); print(pkg.get('version') if pkg else 'unknown')" 2>/dev/null || echo "unknown")
127127

128128
log "Testing ccap-rs version: ${actual_version}"
129129

0 commit comments

Comments
 (0)