From d6d6467e2889eedddc67904c5e346d4da921f5a5 Mon Sep 17 00:00:00 2001 From: bakgio <76126058+bakgio@users.noreply.github.com> Date: Sat, 11 Apr 2026 15:28:38 +0300 Subject: [PATCH 1/2] feat(mediainfo): add MediaInfoContext for reusable library loading # Why - Every v0.1.0 parse call paid a full dlopen, 10-symbol resolve, handle creation, version probe, and dlclose cycle. For batch workloads (asset scanners, directory walks, import pipelines) the load overhead was pure waste and dominated the work. - A reusable context lets callers amortize the one-time library load cost across many parses while keeping every existing public API signature and the global parse lock intact. # What - Add `MediaInfoContext` public type that loads the MediaInfo shared library once and reuses it across all parse entry points (path, reader, URL, pre-built `MediaInfoInput`, structured + raw-text output). `Clone + Send + Sync`; wrap in `Arc` to share across worker pools. - Constructors: `new`, `with_library_file`, `with_library_search_dir`. Accessors: `library_version`, `library_version_string`, `library_file`, `can_parse`. - Refactor the parse pipeline: introduce private `LoadedLibrary<'a>` and split `load_library` into `load_library_full` so the `*_internal_unlocked` functions accept an already-loaded library instead of re-loading on every call. - Route free `MediaInfo::parse*` functions through a lazily initialized process-wide default context (`OnceLock>>>`) whenever no `library_file`/`library_search_dir` override is set. Callers that pin an explicit path still go through the per-call load path, preserving v0.1.0 behavior verbatim. - Add `MediaInfo::reset_default_context()` as an escape hatch to drop the cached context and force a fresh load on the next parse. - Add `MediaInfoError::LibraryMismatch { context, requested }` + `library_mismatch` constructor, returned when a context parse call is given a conflicting `library_file`/`library_search_dir` override. - Ship 20 new tests: 14 in `tests/context_tests.rs` covering reuse, thread safety, mismatch guard, reader/JSON/custom-options/reset paths; 2 in `tests/end_to_end_tests.rs` (URL via context, 100-thread shared-context stress); 2 in `tests/error_unit_tests.rs`; 2 new doctests. Total test count: 203 -> 223. - Add `benches/parse_overhead.rs` with criterion benchmarks: free function, context, and a forced-uncached variant that pins `library_file` to reproduce the v0.1.0 fresh-load path without needing a git worktree. - Add `examples/batch_parse.rs` mirroring the existing example style. - Wire a new `semver` job (`cargo semver-checks -p rsmediainfo`) into the CI pipeline and the release gate so future version bumps are gated on API compatibility. - Bump crate version 0.1.0 -> 0.2.0 in `Cargo.toml`, `README.md` install snippet, and the bug-report issue template placeholder. Update `CHANGELOG.md` with the 0.2.0 entry. # Notes - Additive only. No existing public API signature changed; every existing test passes unmodified. - Observable behavior change: the library now stays mapped into the process after the first successful parse instead of being dlclosed at the end of every call. This matches other language wrappers and is the whole reason the reuse path is faster. The OS reclaims the mapping on process exit. - Default-context error handling: load failures are not cached, so a retry after fixing the environment will work without needing `reset_default_context()`. This sidesteps the `std::io::Error` not-Clone problem entirely. - Windows benchmark numbers are within noise because the OS loader keeps `MediaInfo.dll` mapped across `LoadLibrary`/`FreeLibrary` pairs; on Linux, where `dlclose` is eager, the context path is expected to dominate the uncached path significantly. - `cargo-semver-checks` classifies 0.1.0 -> 0.2.0 as a major change (0.x semver) and skips all 252 lint checks, which is the expected "no further update required" outcome. --- .github/ISSUE_TEMPLATE/bug_report.yml | 2 +- .github/workflows/ci.yml | 17 +- CHANGELOG.md | 26 ++ Cargo.toml | 7 +- README.md | 7 +- benches/parse_overhead.rs | 81 ++++ examples/batch_parse.rs | 44 ++ src/error.rs | 45 ++ src/lib.rs | 7 +- src/mediainfo.rs | 637 +++++++++++++++++++++++--- tests/context_tests.rs | 398 ++++++++++++++++ tests/end_to_end_tests.rs | 84 +++- tests/error_unit_tests.rs | 21 + 13 files changed, 1309 insertions(+), 67 deletions(-) create mode 100644 benches/parse_overhead.rs create mode 100644 examples/batch_parse.rs create mode 100644 tests/context_tests.rs diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 97c8073..24fe61c 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -17,7 +17,7 @@ body: ``` cargo tree -p rsmediainfo ``` - placeholder: "0.1.0" + placeholder: "0.2.0" validations: required: true diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d920c91..f431743 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,6 +94,21 @@ jobs: env: RUSTDOCFLAGS: -Dwarnings + semver: + name: Semver Check + runs-on: ubuntu-latest + env: + RS_MEDIAINFO_SKIP_DOWNLOAD: "1" + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - uses: dtolnay/rust-toolchain@efa25f7f19611383d5b0ccf2d1c8914531636bf9 # stable + with: + toolchain: stable + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 + - name: Install cargo-semver-checks + run: cargo install cargo-semver-checks --locked + - run: cargo semver-checks -p rsmediainfo + audit: name: Security Audit runs-on: ubuntu-latest @@ -134,7 +149,7 @@ jobs: release: name: GitHub Release runs-on: ubuntu-latest - needs: [fmt, clippy, test, msrv, docs, audit, check-version] + needs: [fmt, clippy, test, msrv, docs, semver, audit, check-version] if: github.event_name == 'push' && needs.check-version.outputs.new_release == 'true' permissions: contents: write diff --git a/CHANGELOG.md b/CHANGELOG.md index be592f6..53b23d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,29 @@ +# 0.2.0 (April 11, 2026) + +### Added +- `MediaInfoContext` — reusable parse context that loads the MediaInfo shared + library once and reuses it across every parse call. Wrap it in `Arc` and + share it across a worker pool for batch workloads; parse calls are still + serialized internally to keep the library's global option store + deterministic. +- `MediaInfo::reset_default_context()` — rare-path escape hatch that drops + the cached process-wide default context so the next free-function parse + rebuilds it from scratch. Useful when recovering from a load failure or + picking up a newly installed library mid-process. +- `MediaInfoError::LibraryMismatch` — returned when a context parse call is + given a `library_file` or `library_search_dir` override that would require + a different shared library than the one the context already loaded. + +### Changed +- Free `MediaInfo::parse*` functions now route through a lazily initialized + process-wide default context when no `library_file`/`library_search_dir` + override is set. Callers that pin an explicit library path still go + through the per-call load path, preserving v0.1.0 behavior verbatim. +- The library stays mapped into the process after the first successful + parse (instead of being dlclosed at the end of every call). This matches + the behavior other language wrappers rely on and is the whole reason the + reuse path is faster. + # 0.1.0 (April 11, 2026) - Initial crate release diff --git a/Cargo.toml b/Cargo.toml index da931e7..bd835ca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rsmediainfo" -version = "0.1.0" +version = "0.2.0" edition = "2024" rust-version = "1.88" description = "Rust wrapper for MediaInfo library" @@ -30,6 +30,11 @@ libc = "0.2" tempfile = "3.8" tiny_http = "0.12" reqwest = { version = "0.12", features = ["blocking", "rustls-tls"] } +criterion = "0.5" + +[[bench]] +name = "parse_overhead" +harness = false [features] default = ["bundled"] diff --git a/README.md b/README.md index e264408..9064147 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ - **Multiple output formats** — text, JSON, XML, or `%`-delimited templates straight from the library - **Pre-parsed XML** — build a `MediaInfo` from an existing XML payload with no shared library required - **Bundled library** — optional download of the MediaInfo shared library at build time +- **Reusable context** — `MediaInfoContext` loads the shared library once and reuses it across many parse calls for batch workloads - **Configurable parsing** — parse speed, completion level, cover art, custom library options, encoding error policy - **Typed errors** — every failure mode of the underlying library surfaced through `MediaInfoError` - **Thread-safe** — internal serialization keeps the library's global option store deterministic across threads @@ -30,10 +31,10 @@ ```toml [dependencies] -rsmediainfo = "0.1.0" +rsmediainfo = "0.2.0" # Use a system-installed MediaInfo library instead of the bundled one: -# rsmediainfo = { version = "0.1.0", default-features = false } +# rsmediainfo = { version = "0.2.0", default-features = false } ``` ## Feature Flags @@ -42,7 +43,7 @@ rsmediainfo = "0.1.0" |:---|:---| | `bundled` | Download the MediaInfo shared library for the target platform during the build (enabled by default) | -> See the [`examples/`](examples/) directory for parsing from paths, readers, URLs, raw output, and pre-generated XML. +> See the [`examples/`](examples/) directory for parsing from paths, readers, URLs, raw output, pre-generated XML, and batch parsing through a reusable `MediaInfoContext`. ## License diff --git a/benches/parse_overhead.rs b/benches/parse_overhead.rs new file mode 100644 index 0000000..abed951 --- /dev/null +++ b/benches/parse_overhead.rs @@ -0,0 +1,81 @@ +//! Micro-benchmark comparing the reusable [`MediaInfoContext`] path +//! with the free function path. +//! +//! After the default-context routing landed, the free function path +//! also benefits from library caching — so the two numbers will be +//! very close once the process has warmed up. To measure the original +//! v0.1.0 "fresh dlopen per call" cost, a third benchmark forces a +//! per-call library load by pinning `library_file` in the +//! [`ParseOptions`]. That override bypasses the default-context fast +//! path through the free function path, which reproduces the old +//! behavior without having to check out an older commit. + +use criterion::{Criterion, criterion_group, criterion_main}; +use rsmediainfo::{MediaInfo, MediaInfoContext, ParseOptions}; +use std::path::PathBuf; + +fn sample_path() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/data/sample.mp4") +} + +fn bundled_library_path() -> Option { + let dir = option_env!("RS_MEDIAINFO_BUNDLED_DIR")?; + let dir = PathBuf::from(dir); + for name in ["MediaInfo.dll", "libmediainfo.so.0", "libmediainfo.0.dylib"] { + let candidate = dir.join(name); + if candidate.is_file() { + return Some(candidate); + } + } + None +} + +/// Exercises the free function path. After the default-context +/// routing landed this is effectively a reuse benchmark, since the +/// first iteration initializes the cached context and every later +/// iteration goes through it. +fn bench_free_function(c: &mut Criterion) { + let path = sample_path(); + c.bench_function("free_fn/parse_media_info_path", |b| { + b.iter(|| MediaInfo::parse_media_info_path(&path).expect("free function parse failed")); + }); +} + +/// Exercises the context path directly. +fn bench_context(c: &mut Criterion) { + let ctx = MediaInfoContext::new().expect("failed to construct context"); + let path = sample_path(); + c.bench_function("context/parse_media_info_path", |b| { + b.iter(|| { + ctx.parse_media_info_path(&path) + .expect("context parse failed") + }); + }); +} + +/// Forces a fresh library load on every iteration by pinning +/// `library_file`, which bypasses the cached default context. This +/// reproduces the v0.1.0 "real dlopen per call" behavior so the +/// speedup from the reuse path is measurable. +fn bench_free_function_uncached(c: &mut Criterion) { + let Some(library_file) = bundled_library_path() else { + return; + }; + let path = sample_path(); + let options = ParseOptions::new().library_file(library_file); + + c.bench_function("free_fn/parse_media_info_path_uncached", |b| { + b.iter(|| { + MediaInfo::parse_media_info_path_with_options(&path, &options) + .expect("uncached free function parse failed") + }); + }); +} + +criterion_group!( + benches, + bench_free_function, + bench_context, + bench_free_function_uncached +); +criterion_main!(benches); diff --git a/examples/batch_parse.rs b/examples/batch_parse.rs new file mode 100644 index 0000000..1116e30 --- /dev/null +++ b/examples/batch_parse.rs @@ -0,0 +1,44 @@ +//! Example that builds a [`MediaInfoContext`] once and parses every +//! media file listed on the command line through the same context. +//! +//! This is the recommended shape for batch workloads — asset scanners, +//! import pipelines, directory walks — because the MediaInfo shared +//! library is loaded exactly once instead of on every call. +//! +//! Run with: +//! +//! ```text +//! cargo run --example batch_parse -- tests/data/sample.mp4 tests/data/sample.mkv +//! ``` + +use rsmediainfo::MediaInfoContext; +use std::env; + +fn main() -> Result<(), Box> { + let paths: Vec = env::args().skip(1).collect(); + if paths.is_empty() { + eprintln!("usage: batch_parse [ ...]"); + std::process::exit(2); + } + + // One library load for the whole run — every subsequent parse + // reuses the same loaded copy. + let ctx = MediaInfoContext::new()?; + println!("loaded library version {}", ctx.library_version_string()); + + for path in &paths { + match ctx.parse_media_info_path(path) { + Ok(info) => { + println!("{}: {} tracks", path, info.tracks().len()); + for track in info.tracks() { + println!(" - {}", track.track_type()); + } + } + Err(err) => { + eprintln!("{}: {}", path, err); + } + } + } + + Ok(()) +} diff --git a/src/error.rs b/src/error.rs index 71e37f3..1aa78e2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -124,6 +124,26 @@ pub enum MediaInfoError { /// in the buffer-protocol path. Wraps the underlying [`std::io::Error`]. #[error("I/O error: {0}")] IoError(#[from] std::io::Error), + + /// A parse call on a [`MediaInfoContext`](crate::MediaInfoContext) + /// was given [`ParseOptions`](crate::ParseOptions) whose + /// `library_file` or `library_search_dir` would require a different + /// shared library than the one the context already loaded. + /// + /// A context is bound to a single library for its lifetime — if the + /// caller needs a different library, they should build a separate + /// context via + /// [`MediaInfoContext::with_library_file`](crate::MediaInfoContext::with_library_file) + /// or + /// [`MediaInfoContext::with_library_search_dir`](crate::MediaInfoContext::with_library_search_dir). + #[error("requested library '{requested}' does not match context library '{context}'")] + LibraryMismatch { + /// The library path the context was constructed with, or an + /// empty path when the context used the default search order. + context: PathBuf, + /// The conflicting library path the parse call requested. + requested: PathBuf, + }, } impl MediaInfoError { @@ -182,6 +202,31 @@ impl MediaInfoError { pub fn xml_parse_error>(message: S) -> Self { MediaInfoError::XmlParseError(message.into()) } + + /// Builds a [`MediaInfoError::LibraryMismatch`] from the context + /// library path and the conflicting requested library path. + /// + /// # Example + /// + /// ``` + /// use rsmediainfo::MediaInfoError; + /// + /// let err = MediaInfoError::library_mismatch( + /// "/usr/lib/libmediainfo.so", + /// "/opt/custom/libmediainfo.so", + /// ); + /// assert!(err.to_string().contains("/opt/custom/libmediainfo.so")); + /// ``` + pub fn library_mismatch(context: C, requested: R) -> Self + where + C: Into, + R: Into, + { + MediaInfoError::LibraryMismatch { + context: context.into(), + requested: requested.into(), + } + } } /// Routes XML parser errors into [`MediaInfoError::XmlParseError`] so the diff --git a/src/lib.rs b/src/lib.rs index c2332f2..e18084e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,6 +15,8 @@ //! - structured track models with typed and dynamic attribute access //! - raw text, JSON, XML, and `%`-delimited template output formats //! - configurable parsing through [`ParseOptions`] +//! - reusable [`MediaInfoContext`] for batch workloads that parse many +//! files in a row without repeating the shared library load cost //! - a typed error model ([`MediaInfoError`]) that surfaces every failure //! mode of the underlying library through [`Result`] //! - cross-platform support (Windows, macOS, Linux) for both x86_64 and @@ -59,6 +61,8 @@ //! value //! - `parse_xml` — construct a [`MediaInfo`] directly from a pre-generated //! XML string with no shared library required at runtime +//! - `batch_parse` — build a [`MediaInfoContext`] once and parse a list of +//! files through it without reloading the shared library //! //! Run any of them with `cargo run --example `. //! @@ -79,7 +83,8 @@ mod xml; pub use error::{EncodingErrorMode, MediaInfoError, Result}; pub use mediainfo::{ - MediaInfo, MediaInfoInput, MediaInfoSource, ParseOptions, ParseOutput, ReadSeek, + MediaInfo, MediaInfoContext, MediaInfoInput, MediaInfoSource, ParseOptions, ParseOutput, + ReadSeek, }; pub use track::{AttributeValue, Track, TrackId}; diff --git a/src/mediainfo.rs b/src/mediainfo.rs index 24b13e7..27ca76b 100644 --- a/src/mediainfo.rs +++ b/src/mediainfo.rs @@ -17,7 +17,7 @@ use std::collections::HashMap; use std::io::{Read, Seek, SeekFrom, Write}; use std::net::TcpStream; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::{Arc, Mutex, OnceLock, RwLock}; /// Library version information. /// @@ -168,6 +168,18 @@ pub struct ParseOptions { /// library version 19.9 and newer so the next call starts clean; on /// older builds it cannot, and a warning is logged through the /// `log` crate. Defaults to `None`. + /// + /// # Thread safety + /// + /// The `Reset` call used to restore the option store after a + /// custom-options parse mutates process-global state. The parse + /// pipeline serializes every call via an internal mutex, so + /// concurrent parses with custom options are safe in the sense + /// that they will not race against each other — but callers + /// should still be aware that custom options temporarily take + /// over the library's global option store for the duration of + /// each parse. Unrelated FFI callers in the same process that + /// bypass this crate's lock would observe the transient state. pub mediainfo_options: Option>, /// When `Some`, parse calls return a raw text payload in the @@ -435,6 +447,105 @@ fn parse_lock() -> std::sync::MutexGuard<'static, ()> { .expect("parse lock poisoned") } +/// Already-loaded MediaInfo library plus cached version metadata. +/// +/// Passed into the `*_internal_unlocked` parse functions so they do not +/// have to reload the shared library on every call. The lifetime borrows +/// from either a [`MediaInfoContext`] or a transient load in the free +/// function fallback path; the struct itself is a cheap stack value. +struct LoadedLibrary<'a> { + lib: &'a Arc, + version: LibVersion, + version_number: &'a str, +} + +/// Process-wide lazy default [`MediaInfoContext`]. +/// +/// `OnceLock` gives the single-initialization guarantee, the inner +/// `RwLock>` lets [`MediaInfo::reset_default_context`] swap +/// the stored value back to `None` so the next parse call rebuilds it. +/// Reads on the happy path take a shared `RwLock::read` guard, which is +/// effectively free on an uncontended lock. +static DEFAULT_CONTEXT: OnceLock>>> = OnceLock::new(); + +/// Returns the process-wide default [`MediaInfoContext`], constructing +/// it on first call. +/// +/// A load failure is returned to the caller directly rather than +/// cached: subsequent calls will retry, which keeps the error path +/// transparent for users who fix their environment mid-process. +/// [`MediaInfo::reset_default_context`] can be used to force a retry +/// after a successful context has been installed. +fn default_context() -> Result> { + let lock = DEFAULT_CONTEXT.get_or_init(|| RwLock::new(None)); + + { + let guard = lock.read().expect("default context lock poisoned"); + if let Some(ctx) = guard.as_ref() { + return Ok(Arc::clone(ctx)); + } + } + + let mut guard = lock.write().expect("default context lock poisoned"); + if let Some(ctx) = guard.as_ref() { + return Ok(Arc::clone(ctx)); + } + + let ctx = Arc::new(MediaInfoContext::new()?); + *guard = Some(Arc::clone(&ctx)); + Ok(ctx) +} + +/// Drops the cached default [`MediaInfoContext`] so the next free +/// function parse call rebuilds it from scratch. +/// +/// This is the only way to recover from a library-load failure that +/// was observed earlier in the process without restarting, or to pick +/// up a new library copy that was installed after the first parse. +pub fn reset_default_context() { + if let Some(lock) = DEFAULT_CONTEXT.get() { + let mut guard = lock.write().expect("default context lock poisoned"); + *guard = None; + } +} + +/// Loads the MediaInfo shared library and probes its version. +/// +/// This is the one-time expensive work that used to run on every parse +/// call: resolving the candidate library paths, calling `dlopen` (or the +/// platform equivalent), binding every FFI symbol, creating a temporary +/// handle to query `Info_Version`, and parsing the resulting version +/// string into a [`LibVersion`]. The returned tuple lets the caller +/// build a [`LoadedLibrary`] once and reuse it across many parses. +fn load_library_full( + library_file: Option<&Path>, + library_search_dir: Option<&Path>, +) -> Result<(Arc, String, LibVersion)> { + let search_dir = library_search_dir + .map(PathBuf::from) + .or_else(MediaInfo::default_library_search_dir); + + let paths = if let Some(path) = library_file { + vec![path.to_path_buf()] + } else { + platform::get_library_paths(search_dir.as_deref()) + }; + + let lib = Arc::new(MediaInfoLib::load_from_paths(&paths)?); + + // Probe the library version through a throwaway handle so the + // returned `LibVersion` can feed downstream feature gating without + // any caller ever having to re-query it. + let probe_handle = MediaInfoHandle::new(lib.clone()); + let version_str = probe_handle.option("Info_Version", ""); + + let version_number = MediaInfo::extract_version_number(&version_str)?; + let version = + LibVersion::parse(&version_number).ok_or(MediaInfoError::VersionDetectionFailed)?; + + Ok((lib, version_number, version)) +} + /// Return value of the parse entrypoints that accept arbitrary /// [`ParseOptions::output`] modes. /// @@ -604,7 +715,27 @@ impl MediaInfo { /// } /// ``` pub fn can_parse(library_file: Option<&Path>) -> bool { - Self::load_library(library_file, None).is_ok() + load_library_full(library_file, None).is_ok() + } + + /// Drops the cached process-wide default [`MediaInfoContext`]. + /// + /// Free parse functions on [`MediaInfo`] route through a lazily + /// initialized default context that stays alive for the process + /// lifetime after the first successful parse. Calling this function + /// releases the cached context so the next parse call rebuilds it + /// from scratch. Useful when: + /// + /// - The shared library was missing at the first call but has + /// since been installed. + /// - The process intentionally installed a new library copy and + /// wants new parses to pick it up. + /// - A test wants to force a clean default-context state between + /// runs. + /// + /// This is a rare-path escape hatch. Most programs never need it. + pub fn reset_default_context() { + reset_default_context(); } /// Loads the underlying library and returns its raw version string @@ -636,7 +767,7 @@ impl MediaInfo { /// } /// ``` pub fn library_version(library_file: Option<&Path>) -> Result<(String, LibVersion)> { - let (_handle, version_number, version) = Self::load_library(library_file, None)?; + let (_lib, version_number, version) = load_library_full(library_file, None)?; Ok((version_number, version)) } @@ -819,10 +950,30 @@ impl MediaInfo { options: &ParseOptions, ) -> Result { let _parse_guard = parse_lock(); - Self::parse_to_string_internal_unlocked(filename, options) + + // When the caller pins the library path or search directory, + // honor the v0.1.0 contract by loading a fresh library for this + // call instead of reusing the cached default context. + if options.library_file.is_some() || options.library_search_dir.is_some() { + let (lib, version_number, version) = load_library_full( + options.library_file.as_deref(), + options.library_search_dir.as_deref(), + )?; + let loaded = LoadedLibrary { + lib: &lib, + version, + version_number: &version_number, + }; + return Self::parse_to_string_internal_unlocked(&loaded, filename, options); + } + + let ctx = default_context()?; + let loaded = ctx.loaded(); + Self::parse_to_string_internal_unlocked(&loaded, filename, options) } fn parse_to_string_internal_unlocked>( + loaded: &LoadedLibrary<'_>, filename: P, options: &ParseOptions, ) -> Result { @@ -832,16 +983,13 @@ impl MediaInfo { // Strings that look like URLs are routed straight to the URL // entrypoint rather than being interpreted as filesystem paths. if path_str.contains("://") { - return Self::parse_to_string_from_url_unlocked(&path_str, options); + return Self::parse_to_string_from_url_unlocked(loaded, &path_str, options); } - let (handle, version_number, version) = Self::load_library( - options.library_file.as_deref(), - options.library_search_dir.as_deref(), - )?; + let handle = MediaInfoHandle::new(loaded.lib.clone()); - Self::configure_parse_options(&handle, &version, &version_number, options); - Self::configure_output_options(&handle, &version, options); + Self::configure_parse_options(&handle, &loaded.version, loaded.version_number, options); + Self::configure_output_options(&handle, &loaded.version, options); // Probe the filesystem first so missing-file errors surface as // FileNotFound rather than the more generic library parse error. @@ -857,7 +1005,7 @@ impl MediaInfo { // Reset the global option store on library builds that support // it, so the next parse call starts from a clean slate. - if options.mediainfo_options.is_some() && version.supports_reset() { + if options.mediainfo_options.is_some() && loaded.version.supports_reset() { handle.option("Reset", ""); } @@ -866,20 +1014,37 @@ impl MediaInfo { fn parse_to_string_from_url(url: &str, options: &ParseOptions) -> Result { let _parse_guard = parse_lock(); - Self::parse_to_string_from_url_unlocked(url, options) + + if options.library_file.is_some() || options.library_search_dir.is_some() { + let (lib, version_number, version) = load_library_full( + options.library_file.as_deref(), + options.library_search_dir.as_deref(), + )?; + let loaded = LoadedLibrary { + lib: &lib, + version, + version_number: &version_number, + }; + return Self::parse_to_string_from_url_unlocked(&loaded, url, options); + } + + let ctx = default_context()?; + let loaded = ctx.loaded(); + Self::parse_to_string_from_url_unlocked(&loaded, url, options) } - fn parse_to_string_from_url_unlocked(url: &str, options: &ParseOptions) -> Result { - let (handle, version_number, version) = Self::load_library( - options.library_file.as_deref(), - options.library_search_dir.as_deref(), - )?; + fn parse_to_string_from_url_unlocked( + loaded: &LoadedLibrary<'_>, + url: &str, + options: &ParseOptions, + ) -> Result { + let handle = MediaInfoHandle::new(loaded.lib.clone()); - Self::configure_parse_options(&handle, &version, &version_number, options); - Self::configure_output_options(&handle, &version, options); + Self::configure_parse_options(&handle, &loaded.version, loaded.version_number, options); + Self::configure_output_options(&handle, &loaded.version, options); if handle.open(url) == 0 { - if options.mediainfo_options.is_some() && version.supports_reset() { + if options.mediainfo_options.is_some() && loaded.version.supports_reset() { handle.option("Reset", ""); } @@ -891,7 +1056,7 @@ impl MediaInfo { "native URL opener could not handle {}; falling back to built-in HTTP fetch", url ); - return Self::parse_url_via_http(url, options); + return Self::parse_url_via_http(loaded, url, options); } return Err(MediaInfoError::parse_error(url)); @@ -899,7 +1064,7 @@ impl MediaInfo { let output = handle.inform(); - if options.mediainfo_options.is_some() && version.supports_reset() { + if options.mediainfo_options.is_some() && loaded.version.supports_reset() { handle.option("Reset", ""); } @@ -1026,10 +1191,27 @@ impl MediaInfo { options: &ParseOptions, ) -> Result { let _parse_guard = parse_lock(); - Self::parse_reader_to_string_internal_unlocked(reader, options) + + if options.library_file.is_some() || options.library_search_dir.is_some() { + let (lib, version_number, version) = load_library_full( + options.library_file.as_deref(), + options.library_search_dir.as_deref(), + )?; + let loaded = LoadedLibrary { + lib: &lib, + version, + version_number: &version_number, + }; + return Self::parse_reader_to_string_internal_unlocked(&loaded, reader, options); + } + + let ctx = default_context()?; + let loaded = ctx.loaded(); + Self::parse_reader_to_string_internal_unlocked(&loaded, reader, options) } fn parse_reader_to_string_internal_unlocked( + loaded: &LoadedLibrary<'_>, reader: &mut R, options: &ParseOptions, ) -> Result { @@ -1039,13 +1221,10 @@ impl MediaInfo { let file_size = reader.seek(SeekFrom::End(0))?; reader.seek(SeekFrom::Start(0))?; - let (handle, version_number, version) = Self::load_library( - options.library_file.as_deref(), - options.library_search_dir.as_deref(), - )?; + let handle = MediaInfoHandle::new(loaded.lib.clone()); - Self::configure_parse_options(&handle, &version, &version_number, options); - Self::configure_output_options(&handle, &version, options); + Self::configure_parse_options(&handle, &loaded.version, loaded.version_number, options); + Self::configure_output_options(&handle, &loaded.version, options); handle.open_buffer_init(file_size, 0); @@ -1114,17 +1293,21 @@ impl MediaInfo { // Reset the global option store on library builds that support // it, so the next parse call starts from a clean slate. - if options.mediainfo_options.is_some() && version.supports_reset() { + if options.mediainfo_options.is_some() && loaded.version.supports_reset() { handle.option("Reset", ""); } Ok(output) } - fn parse_url_via_http(url: &str, options: &ParseOptions) -> Result { + fn parse_url_via_http( + loaded: &LoadedLibrary<'_>, + url: &str, + options: &ParseOptions, + ) -> Result { let bytes = Self::fetch_http_bytes(url).map_err(|_| MediaInfoError::parse_error(url))?; let mut cursor = std::io::Cursor::new(bytes); - Self::parse_reader_to_string_internal_unlocked(&mut cursor, options) + Self::parse_reader_to_string_internal_unlocked(loaded, &mut cursor, options) } fn fetch_http_bytes(url: &str) -> Result> { @@ -1303,32 +1486,6 @@ impl MediaInfo { .map_err(|e| MediaInfoError::invalid_input(e.to_string())) } - fn load_library( - library_file: Option<&Path>, - library_search_dir: Option<&Path>, - ) -> Result<(MediaInfoHandle, String, LibVersion)> { - let search_dir = library_search_dir - .map(PathBuf::from) - .or_else(Self::default_library_search_dir); - - let paths = if let Some(path) = library_file { - vec![path.to_path_buf()] - } else { - platform::get_library_paths(search_dir.as_deref()) - }; - - let lib = Arc::new(MediaInfoLib::load_from_paths(&paths)?); - - let handle = MediaInfoHandle::new(lib); - let version_str = handle.option("Info_Version", ""); - - let version_number = Self::extract_version_number(&version_str)?; - let version = - LibVersion::parse(&version_number).ok_or(MediaInfoError::VersionDetectionFailed)?; - - Ok((handle, version_number, version)) - } - fn default_library_search_dir() -> Option { // Highest precedence: an explicit runtime override. if let Ok(dir) = std::env::var("RS_MEDIAINFO_LIBRARY_DIR") @@ -1455,6 +1612,368 @@ impl MediaInfo { } } +/// Reusable parse context that loads the MediaInfo shared library once +/// and reuses it across many parse calls. +/// +/// Constructing a [`MediaInfoContext`] pays the one-time library load +/// cost up front: candidate path resolution, `dlopen` (or the platform +/// equivalent), FFI symbol binding, and version probing. Every parse +/// call made through the context skips that work and only creates a +/// fresh per-call handle, which is dramatically cheaper for batch +/// workloads such as scanning a directory of media files. +/// +/// A context is `Send + Sync` and `Clone` (the clone is cheap — the +/// underlying library is shared via [`Arc`]). One context per process is +/// the expected usage pattern; wrap it in an [`Arc`] and share it across +/// a worker pool. Parse calls are still serialized internally by the +/// global parse lock so the underlying library's process-wide option +/// store stays deterministic even under heavy concurrency. +/// +/// # Example +/// +/// ```no_run +/// use rsmediainfo::MediaInfoContext; +/// +/// let ctx = MediaInfoContext::new()?; +/// for path in &["a.mp4", "b.mkv", "c.mov"] { +/// let info = ctx.parse_media_info_path(path)?; +/// println!("{}: {} tracks", path, info.tracks().len()); +/// } +/// # Ok::<(), rsmediainfo::MediaInfoError>(()) +/// ``` +#[derive(Clone)] +pub struct MediaInfoContext { + lib: Arc, + version_number: String, + version: LibVersion, + library_file: Option, +} + +impl MediaInfoContext { + /// Constructs a new context using the default library search order. + /// + /// The resolver walks the following locations in order: the + /// `RS_MEDIAINFO_LIBRARY_DIR` environment variable, the + /// compile-time `RS_MEDIAINFO_BUNDLED_DIR` directory, the directory + /// containing the running executable, and finally the current + /// working directory. The first location that yields a loadable + /// library is used. + /// + /// # Errors + /// + /// Returns [`MediaInfoError::LibraryNotFound`] when no candidate + /// path can be opened, or [`MediaInfoError::VersionDetectionFailed`] + /// when the loaded library reports a version string the crate + /// cannot interpret. + pub fn new() -> Result { + Self::build(None, None) + } + + /// Constructs a new context that loads the library from an explicit + /// path, bypassing the default search order entirely. + /// + /// # Errors + /// + /// Same as [`MediaInfoContext::new`]. + pub fn with_library_file>(path: P) -> Result { + let path = path.into(); + Self::build(Some(path.as_path()), None) + } + + /// Constructs a new context that searches a caller-supplied + /// directory for a bundled copy of the library before falling back + /// to the platform library search path. + /// + /// # Errors + /// + /// Same as [`MediaInfoContext::new`]. + pub fn with_library_search_dir>(dir: P) -> Result { + let dir = dir.into(); + Self::build(None, Some(dir.as_path())) + } + + fn build(library_file: Option<&Path>, library_search_dir: Option<&Path>) -> Result { + let (lib, version_number, version) = load_library_full(library_file, library_search_dir)?; + Ok(Self { + lib, + version_number, + version, + library_file: library_file.map(PathBuf::from), + }) + } + + /// Returns the parsed [`LibVersion`] of the loaded library. + pub fn library_version(&self) -> LibVersion { + self.version + } + + /// Returns the raw version string reported by the loaded library + /// (typically something like `"25.10"`). + pub fn library_version_string(&self) -> &str { + &self.version_number + } + + /// Returns the explicit library path the context was built with, or + /// `None` when the library was resolved through the default search + /// order. + pub fn library_file(&self) -> Option<&Path> { + self.library_file.as_deref() + } + + /// Returns `true` because a context can only be constructed once + /// the library has successfully loaded. + pub fn can_parse(&self) -> bool { + true + } + + fn loaded(&self) -> LoadedLibrary<'_> { + LoadedLibrary { + lib: &self.lib, + version: self.version, + version_number: &self.version_number, + } + } + + /// Parses any supported source (path, URL, or reader) using the + /// default [`ParseOptions`]. + /// + /// See [`MediaInfo::parse`] for the full behavior; this is the + /// context-bound equivalent. + pub fn parse<'a, S>(&self, source: S) -> Result + where + S: MediaInfoSource<'a>, + { + self.parse_with_options(source, &ParseOptions::default()) + } + + /// Parses any supported source with caller-supplied + /// [`ParseOptions`]. See [`MediaInfo::parse_with_options`] for the + /// full behavior. + pub fn parse_with_options<'a, S>( + &self, + source: S, + options: &ParseOptions, + ) -> Result + where + S: MediaInfoSource<'a>, + { + self.check_library_overrides(options)?; + let input = source.into_input(); + self.parse_input_with_options(input, options) + } + + /// Parses any supported source and returns the result as a + /// structured [`MediaInfo`] directly. + pub fn parse_media_info<'a, S>(&self, source: S) -> Result + where + S: MediaInfoSource<'a>, + { + self.parse_media_info_with_options(source, &ParseOptions::default()) + } + + /// Parses any supported source with caller-supplied + /// [`ParseOptions`] and returns the result as a structured + /// [`MediaInfo`] directly. + pub fn parse_media_info_with_options<'a, S>( + &self, + source: S, + options: &ParseOptions, + ) -> Result + where + S: MediaInfoSource<'a>, + { + if options.output.is_some() { + return Err(MediaInfoError::invalid_input( + "output is only supported by parse or parse_with_options", + )); + } + + match self.parse_with_options(source, options)? { + ParseOutput::MediaInfo(mi) => Ok(mi), + ParseOutput::Output(_) => Err(MediaInfoError::invalid_input( + "output is only supported by parse or parse_with_options", + )), + } + } + + /// Path-typed convenience wrapper around [`MediaInfoContext::parse`]. + pub fn parse_path>(&self, path: P) -> Result { + self.parse(path.as_ref()) + } + + /// Path-typed convenience wrapper around + /// [`MediaInfoContext::parse_with_options`]. + pub fn parse_path_with_options>( + &self, + path: P, + options: &ParseOptions, + ) -> Result { + self.parse_with_options(path.as_ref(), options) + } + + /// Path-typed convenience wrapper around + /// [`MediaInfoContext::parse_media_info`]. + pub fn parse_media_info_path>(&self, path: P) -> Result { + self.parse_media_info(path.as_ref()) + } + + /// Path-typed convenience wrapper around + /// [`MediaInfoContext::parse_media_info_with_options`]. + pub fn parse_media_info_path_with_options>( + &self, + path: P, + options: &ParseOptions, + ) -> Result { + self.parse_media_info_with_options(path.as_ref(), options) + } + + /// Parses any `Read + Seek` source using the default [`ParseOptions`]. + pub fn parse_from_reader(&self, reader: &mut R) -> Result { + self.parse_from_reader_with_options(reader, &ParseOptions::default()) + } + + /// Parses any `Read + Seek` source with caller-supplied + /// [`ParseOptions`]. + pub fn parse_from_reader_with_options( + &self, + reader: &mut R, + options: &ParseOptions, + ) -> Result { + self.check_library_overrides(options)?; + let _parse_guard = parse_lock(); + let loaded = self.loaded(); + + if options.output.is_some() { + let output = + MediaInfo::parse_reader_to_string_internal_unlocked(&loaded, reader, options)?; + return Ok(ParseOutput::Output(output)); + } + + let output = MediaInfo::parse_reader_to_string_internal_unlocked(&loaded, reader, options)?; + let mi = + MediaInfo::from_xml_bytes_with_encoding(output.as_bytes(), options.encoding_errors)?; + Ok(ParseOutput::MediaInfo(mi)) + } + + /// Parses a path and returns the raw text output produced by the + /// underlying library. + pub fn parse_to_string>(&self, path: P, output_format: &str) -> Result { + let options = ParseOptions::new().output(output_format.to_string()); + self.parse_to_string_with_options(path, &options) + } + + /// Parses a path with caller-supplied [`ParseOptions`] and returns + /// the raw text output produced by the underlying library. + pub fn parse_to_string_with_options>( + &self, + path: P, + options: &ParseOptions, + ) -> Result { + self.check_library_overrides(options)?; + let _parse_guard = parse_lock(); + let loaded = self.loaded(); + MediaInfo::parse_to_string_internal_unlocked(&loaded, path.as_ref(), options) + } + + /// Parses a reader and returns the raw text output produced by the + /// underlying library. + pub fn parse_reader_to_string( + &self, + reader: &mut R, + output_format: &str, + ) -> Result { + let options = ParseOptions::new().output(output_format.to_string()); + self.parse_reader_to_string_with_options(reader, &options) + } + + /// Parses a reader with caller-supplied [`ParseOptions`] and + /// returns the raw text output produced by the underlying library. + pub fn parse_reader_to_string_with_options( + &self, + reader: &mut R, + options: &ParseOptions, + ) -> Result { + self.check_library_overrides(options)?; + let _parse_guard = parse_lock(); + let loaded = self.loaded(); + MediaInfo::parse_reader_to_string_internal_unlocked(&loaded, reader, options) + } + + /// Parses a pre-built [`MediaInfoInput`] using the default + /// [`ParseOptions`]. + pub fn parse_input(&self, input: MediaInfoInput<'_>) -> Result { + self.parse_input_with_options(input, &ParseOptions::default()) + } + + /// Parses a pre-built [`MediaInfoInput`] with caller-supplied + /// [`ParseOptions`]. + pub fn parse_input_with_options( + &self, + input: MediaInfoInput<'_>, + options: &ParseOptions, + ) -> Result { + self.check_library_overrides(options)?; + + if options.output.is_some() { + let output = self.parse_input_to_string_with_options(input, options)?; + return Ok(ParseOutput::Output(output)); + } + + let output = self.parse_input_to_string_with_options(input, options)?; + let mi = + MediaInfo::from_xml_bytes_with_encoding(output.as_bytes(), options.encoding_errors)?; + Ok(ParseOutput::MediaInfo(mi)) + } + + fn parse_input_to_string_with_options( + &self, + input: MediaInfoInput<'_>, + options: &ParseOptions, + ) -> Result { + let _parse_guard = parse_lock(); + let loaded = self.loaded(); + match input { + MediaInfoInput::Path(path) => { + MediaInfo::parse_to_string_internal_unlocked(&loaded, path, options) + } + MediaInfoInput::Url(url) => { + MediaInfo::parse_to_string_from_url_unlocked(&loaded, url, options) + } + MediaInfoInput::Reader(reader) => { + MediaInfo::parse_reader_to_string_internal_unlocked(&loaded, reader, options) + } + } + } + + /// Rejects parse calls that would require a different shared + /// library than the one the context already loaded. + /// + /// The match is path-literal and does not canonicalise (the two + /// sides may differ by symlink resolution or relative-vs-absolute + /// spelling). Callers that need a different library should build a + /// separate context rather than trying to override through + /// [`ParseOptions`]. + fn check_library_overrides(&self, options: &ParseOptions) -> Result<()> { + if let Some(requested) = options.library_file.as_deref() + && self.library_file.as_deref() != Some(requested) + { + return Err(MediaInfoError::library_mismatch( + self.library_file.clone().unwrap_or_default(), + requested, + )); + } + + if let Some(requested_dir) = options.library_search_dir.as_deref() { + return Err(MediaInfoError::library_mismatch( + self.library_file.clone().unwrap_or_default(), + requested_dir, + )); + } + + Ok(()) + } +} + impl std::str::FromStr for MediaInfo { type Err = MediaInfoError; diff --git a/tests/context_tests.rs b/tests/context_tests.rs new file mode 100644 index 0000000..40508e2 --- /dev/null +++ b/tests/context_tests.rs @@ -0,0 +1,398 @@ +//! Tests for the reusable [`MediaInfoContext`] parse entrypoint. +//! +//! These tests cover both the functional behavior of the context +//! (parse result equivalence, cross-thread reuse, option isolation) +//! and the new [`MediaInfoError::LibraryMismatch`] guard that prevents +//! a context-bound call from silently switching libraries under the +//! caller. + +use rsmediainfo::{ + MediaInfo, MediaInfoContext, MediaInfoError, MediaInfoInput, ParseOptions, ParseOutput, +}; +use std::collections::HashMap; +use std::fs::File; +use std::io::BufReader; +use std::path::PathBuf; +use std::sync::Arc; +use std::thread; + +/// Returns the path to the test data directory. +fn test_data_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/data") +} + +/// Returns the path to a specific test file. +fn test_file(name: &str) -> PathBuf { + test_data_dir().join(name) +} + +// --------------------------------------------------------------------------- +// Basic reuse +// --------------------------------------------------------------------------- + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_basic_reuse() { + // A single context should be able to parse the same file many times + // in a row and return the exact same structured result. This pins + // down the "per-call configure" contract: if any state leaked + // between calls, later iterations would diverge from the first. + let ctx = MediaInfoContext::new().expect("failed to construct context"); + let path = test_file("sample.mp4"); + + let baseline = ctx + .parse_media_info_path(&path) + .expect("first parse failed"); + + for i in 0..10 { + let next = ctx + .parse_media_info_path(&path) + .expect("reused-context parse failed"); + assert_eq!( + baseline, next, + "parse #{i} disagreed with the baseline result" + ); + } +} + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_matches_free_function_output() { + // The context path and the free function path must agree on every + // byte of the parsed structure for an identical input and option + // set. Any divergence would indicate a bug in either the context + // plumbing or the `LoadedLibrary` refactor. + let path = test_file("sample.mp4"); + let ctx = MediaInfoContext::new().expect("failed to construct context"); + + let via_ctx = ctx + .parse_media_info_path(&path) + .expect("context parse failed"); + let via_free = MediaInfo::parse_media_info_path(&path).expect("free function parse failed"); + + assert_eq!(via_ctx, via_free); +} + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_version_cache() { + // Both library version accessors should report a non-empty, well + // formed value that agrees with itself: the parsed `LibVersion` + // should round-trip the raw string the library reported at load. + let ctx = MediaInfoContext::new().expect("failed to construct context"); + + let version_string = ctx.library_version_string(); + assert!( + !version_string.is_empty(), + "library version string should never be empty" + ); + + let version = ctx.library_version(); + assert!( + version.major > 0, + "library major version should be greater than zero" + ); +} + +// --------------------------------------------------------------------------- +// Concurrency +// --------------------------------------------------------------------------- + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_shared_across_threads() { + // A single shared context driven by many worker threads must + // produce deterministic output for every parse. The global parse + // lock serializes FFI access, but this test still exercises the + // reuse path end-to-end under real contention. + let ctx = Arc::new(MediaInfoContext::new().expect("failed to construct context")); + let path = test_file("sample.mp4"); + + let baseline = ctx + .parse_media_info_path(&path) + .expect("baseline parse failed") + .to_data(); + + let mut handles = Vec::new(); + for _ in 0..16 { + let ctx_clone = Arc::clone(&ctx); + let path_clone = path.clone(); + + handles.push(thread::spawn(move || { + let mut results = Vec::new(); + for _ in 0..5 { + let mi = ctx_clone + .parse_media_info_path(&path_clone) + .expect("worker parse failed"); + results.push(mi.to_data()); + } + results + })); + } + + for handle in handles { + let results = handle.join().expect("worker thread panicked"); + for result in results { + assert_eq!(result, baseline, "worker result diverged from the baseline"); + } + } +} + +// --------------------------------------------------------------------------- +// LibraryMismatch guard +// --------------------------------------------------------------------------- + +/// Returns the bundled library path recorded by `build.rs` through the +/// `RS_MEDIAINFO_BUNDLED_DIR` environment variable at compile time. +fn bundled_library_path() -> Option { + let dir = option_env!("RS_MEDIAINFO_BUNDLED_DIR")?; + let dir = PathBuf::from(dir); + for name in ["MediaInfo.dll", "libmediainfo.so.0", "libmediainfo.0.dylib"] { + let candidate = dir.join(name); + if candidate.is_file() { + return Some(candidate); + } + } + None +} + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_library_mismatch_file() { + // Constructing a context with an explicit library file pins it to + // that library. A subsequent parse call that tries to override + // `library_file` must return a `LibraryMismatch` error instead of + // quietly ignoring the override. + let Some(real_library) = bundled_library_path() else { + return; + }; + + let ctx = MediaInfoContext::with_library_file(&real_library) + .expect("failed to construct context from bundled library"); + let opts = ParseOptions::new().library_file("/nonexistent/other-libmediainfo.so"); + + let result = ctx.parse_media_info_path_with_options(test_file("sample.mp4"), &opts); + assert!( + matches!(result, Err(MediaInfoError::LibraryMismatch { .. })), + "expected LibraryMismatch, got {result:?}" + ); +} + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_library_mismatch_search_dir() { + // Setting `library_search_dir` on a context parse call is always a + // mismatch, regardless of whether the directory would resolve to + // the same library file. A caller asking to re-resolve is asking + // for a different load than the context performed. + let Some(real_library) = bundled_library_path() else { + return; + }; + + let ctx = MediaInfoContext::with_library_file(&real_library) + .expect("failed to construct context from bundled library"); + let opts = ParseOptions::new().library_search_dir("/nonexistent/dir"); + + let result = ctx.parse_media_info_path_with_options(test_file("sample.mp4"), &opts); + assert!( + matches!(result, Err(MediaInfoError::LibraryMismatch { .. })), + "expected LibraryMismatch, got {result:?}" + ); +} + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_matching_library_file_allowed() { + // When the caller passes the exact same `library_file` the context + // was constructed with, the parse must succeed — the guard only + // rejects genuine mismatches, not tautologies. + let Some(real_library) = bundled_library_path() else { + return; + }; + + let ctx = MediaInfoContext::with_library_file(&real_library) + .expect("failed to construct context from bundled library"); + let opts = ParseOptions::new().library_file(&real_library); + + let mi = ctx + .parse_media_info_path_with_options(test_file("sample.mp4"), &opts) + .expect("matching library_file should be allowed"); + assert!(!mi.tracks().is_empty(), "expected at least one track"); +} + +// --------------------------------------------------------------------------- +// Reader input +// --------------------------------------------------------------------------- + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_reader_input() { + // Parsing the same file through a `BufReader` many times on a + // shared context should produce identical results, confirming that + // the buffer-protocol path also benefits from the reuse. + let ctx = MediaInfoContext::new().expect("failed to construct context"); + let path = test_file("sample.mp4"); + + let mut first_reader = BufReader::new(File::open(&path).expect("open failed")); + let baseline = ctx + .parse_media_info(&mut first_reader) + .expect("first reader parse failed"); + + for i in 0..5 { + let mut reader = BufReader::new(File::open(&path).expect("open failed")); + let next = ctx + .parse_media_info(&mut reader) + .expect("reused-context reader parse failed"); + assert_eq!( + baseline, next, + "reader parse #{i} disagreed with the baseline" + ); + } +} + +// --------------------------------------------------------------------------- +// Raw output mode +// --------------------------------------------------------------------------- + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_raw_output_mode() { + // Asking for JSON output through `ParseOptions` should route the + // call through the raw-text code path and yield a `ParseOutput::Output` + // value carrying a JSON document. + let ctx = MediaInfoContext::new().expect("failed to construct context"); + let opts = ParseOptions::new().output("JSON"); + let result = ctx + .parse_path_with_options(test_file("sample.mp4"), &opts) + .expect("raw output parse failed"); + + match result { + ParseOutput::Output(text) => { + assert!( + text.trim_start().starts_with('{'), + "expected a JSON document, got: {}", + &text[..text.len().min(40)] + ); + } + ParseOutput::MediaInfo(_) => panic!("expected raw JSON output"), + } +} + +// --------------------------------------------------------------------------- +// Option isolation between calls +// --------------------------------------------------------------------------- + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_custom_options() { + // Setting custom `mediainfo_options` on one call must not leak + // into the next call made through the same context. The second + // parse should match a fresh default-options parse byte for byte. + let ctx = MediaInfoContext::new().expect("failed to construct context"); + let path = test_file("sample.mp4"); + + let mut custom = HashMap::new(); + custom.insert("Language_Select".to_string(), "raw".to_string()); + let custom_opts = ParseOptions::new().mediainfo_options(custom); + + let _ = ctx + .parse_media_info_path_with_options(&path, &custom_opts) + .expect("custom-options parse failed"); + + let plain_after = ctx + .parse_media_info_path(&path) + .expect("default-options parse after custom failed"); + let plain_control = ctx + .parse_media_info_path(&path) + .expect("second default-options parse failed"); + + assert_eq!( + plain_after, plain_control, + "custom options leaked across context parse calls" + ); +} + +// --------------------------------------------------------------------------- +// Default context reuse and reset +// --------------------------------------------------------------------------- + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_default_context_is_reused() { + // Two consecutive free-function parses should now route through + // the shared default context and produce identical results. + let path = test_file("sample.mp4"); + + let first = MediaInfo::parse_media_info_path(&path).expect("first free-function parse failed"); + let second = + MediaInfo::parse_media_info_path(&path).expect("second free-function parse failed"); + + assert_eq!(first, second); +} + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_reset_default_context() { + // The default context reset is a rare-path escape hatch. After + // resetting, the next free-function parse must still succeed + // without the user having to do anything else. + let path = test_file("sample.mp4"); + let _ = MediaInfo::parse_media_info_path(&path).expect("seed parse failed"); + + MediaInfo::reset_default_context(); + + let mi = MediaInfo::parse_media_info_path(&path).expect("parse after reset failed"); + assert!( + !mi.tracks().is_empty(), + "expected parsed tracks after reset" + ); +} + +// --------------------------------------------------------------------------- +// Free function with an explicit library override still works +// --------------------------------------------------------------------------- + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_free_function_with_library_override_bypasses_default() { + // When the caller pins `library_file` on the free-function path, + // the default-context routing should step aside and the explicit + // per-call load should still work. We cannot directly observe the + // fallback path from the outside, so we just assert the call + // succeeds and the result agrees with an unoverridden parse. + let Some(real_library) = bundled_library_path() else { + return; + }; + + let path = test_file("sample.mp4"); + let baseline = MediaInfo::parse_media_info_path(&path).expect("baseline parse failed"); + + let opts = ParseOptions::new().library_file(&real_library); + let overridden = + MediaInfo::parse_media_info_path_with_options(&path, &opts).expect("override parse failed"); + + assert_eq!(baseline, overridden); +} + +// --------------------------------------------------------------------------- +// Context accepts pre-built MediaInfoInput values +// --------------------------------------------------------------------------- + +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_parse_input_dispatch() { + // Feeding a pre-built `MediaInfoInput::Path` to the context + // entrypoint should behave the same as the path-typed helper. + let ctx = MediaInfoContext::new().expect("failed to construct context"); + let path = test_file("sample.mp4"); + + let via_path = ctx.parse_media_info_path(&path).expect("path parse failed"); + + let input = MediaInfoInput::Path(path.as_path()); + let via_input = match ctx.parse_input(input).expect("input parse failed") { + ParseOutput::MediaInfo(mi) => mi, + ParseOutput::Output(_) => panic!("expected structured result"), + }; + + assert_eq!(via_path, via_input); +} diff --git a/tests/end_to_end_tests.rs b/tests/end_to_end_tests.rs index d6f9556..58ad70b 100644 --- a/tests/end_to_end_tests.rs +++ b/tests/end_to_end_tests.rs @@ -1,6 +1,6 @@ //! End-to-end behavioral tests covering the public parse pipeline. -use rsmediainfo::{MediaInfo, MediaInfoError, ParseOptions, ParseOutput}; +use rsmediainfo::{MediaInfo, MediaInfoContext, MediaInfoError, ParseOptions, ParseOutput}; use std::fs::File; use std::io::BufReader; use std::path::PathBuf; @@ -1037,3 +1037,85 @@ fn test_parse_url() { let mi = MediaInfo::parse_media_info(&url).expect("Failed to parse URL"); assert_eq!(mi.tracks().len(), 3, "Expected 3 tracks from URL"); } + +/// Verifies URL parsing works through a reusable [`MediaInfoContext`]. +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_context_url_parse() { + // Mirror of `test_parse_url` but driven through a shared context + // so the URL parse path also exercises the reuse-friendly code. + let data = std::fs::read(test_file("sample.mkv")).expect("Failed to read sample.mkv"); + + let server = Server::http("127.0.0.1:0").expect("Failed to start HTTP server"); + let addr = server.server_addr().to_string(); + + std::thread::spawn(move || { + if let Ok(request) = server.recv() { + let response = Response::from_data(data); + let _ = request.respond(response); + } + }); + + let url = format!("http://{}/sample.mkv", addr); + let ctx = MediaInfoContext::new().expect("Failed to construct context"); + let mi = ctx + .parse_media_info(url.as_str()) + .expect("Failed to parse URL through context"); + assert_eq!( + mi.tracks().len(), + 3, + "Expected 3 tracks from URL via context" + ); +} + +/// Verifies parsing from many threads through a shared context produces consistent results. +#[test] +#[ignore = "Requires libmediainfo installation"] +fn test_thread_safety_context() { + use std::sync::{Arc, Mutex}; + use std::thread; + + let path = test_file("sample.mp4"); + let ctx = Arc::new(MediaInfoContext::new().expect("Failed to construct context")); + + let expected = ctx + .parse_media_info_path(&path) + .expect("Failed to parse expected result"); + let expected_data = expected.to_data(); + + let results = Arc::new(Mutex::new(Vec::new())); + let mut handles = vec![]; + + for _ in 0..100 { + let path_clone = path.clone(); + let ctx_clone = Arc::clone(&ctx); + let results_clone = Arc::clone(&results); + + let handle = thread::spawn(move || { + if let Ok(mi) = ctx_clone.parse_media_info_path(&path_clone) { + let data = mi.to_data(); + results_clone.lock().unwrap().push(data); + } + }); + handles.push(handle); + } + + for handle in handles { + handle.join().expect("Thread panicked"); + } + + let results_vec = results.lock().unwrap(); + assert_eq!( + results_vec.len(), + 100, + "All 100 threads should produce results through the shared context" + ); + + for (i, result) in results_vec.iter().enumerate() { + assert_eq!( + result, &expected_data, + "Thread {} produced different result through the shared context", + i + ); + } +} diff --git a/tests/error_unit_tests.rs b/tests/error_unit_tests.rs index d34d414..3fb628e 100644 --- a/tests/error_unit_tests.rs +++ b/tests/error_unit_tests.rs @@ -102,3 +102,24 @@ fn test_library_not_found_format_includes_every_attempted_name() { ); } } + +#[test] +fn test_library_mismatch_error_format() { + // The Display impl must surface both the context library and the + // conflicting requested library so the caller can tell exactly + // which override was rejected. + let err = + MediaInfoError::library_mismatch("/usr/lib/libmediainfo.so", "/opt/custom/libmediainfo.so"); + let msg = err.to_string(); + assert!(msg.contains("/usr/lib/libmediainfo.so")); + assert!(msg.contains("/opt/custom/libmediainfo.so")); + assert!(msg.contains("does not match context library")); +} + +#[test] +fn test_library_mismatch_matches_variant() { + // Callers should be able to pattern-match the variant directly to + // surface a structured error at the API boundary. + let err = MediaInfoError::library_mismatch("a.so", "b.so"); + assert!(matches!(err, MediaInfoError::LibraryMismatch { .. })); +} From 0aa20a0a2bd6a486a03cf87167afa0db1dcd94b8 Mon Sep 17 00:00:00 2001 From: bakgio <76126058+bakgio@users.noreply.github.com> Date: Sat, 11 Apr 2026 16:10:35 +0300 Subject: [PATCH 2/2] fix(mediainfo): serialize version probe with parse lock Why - MediaInfo_New / MediaInfo_Delete are not thread-safe on every libmediainfo build, so the throwaway probe handle used by load_library_full could race against an in-flight parse on another thread. What - Take the global parse lock inside load_library_full for the duration of the version probe so both handle creation and destruction happen in the locked region. - Reorder parse_to_string / parse_to_string_from_url / parse_reader_to_string so the library is resolved before the parse lock is acquired, preventing the inner probe from re-entering and deadlocking the same lock. - Document the new locking contract on load_library_full. Notes - No API changes. Pure concurrency fix. --- src/mediainfo.rs | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/mediainfo.rs b/src/mediainfo.rs index 27ca76b..faaf53e 100644 --- a/src/mediainfo.rs +++ b/src/mediainfo.rs @@ -517,6 +517,14 @@ pub fn reset_default_context() { /// handle to query `Info_Version`, and parsing the resulting version /// string into a [`LibVersion`]. The returned tuple lets the caller /// build a [`LoadedLibrary`] once and reuse it across many parses. +/// +/// The throwaway probe handle used to read `Info_Version` issues +/// `MediaInfo_New` and `MediaInfo_Delete` calls, which are not +/// thread-safe on every libmediainfo build. To avoid racing against an +/// in-flight parse on another thread, this function takes the global +/// parse lock for the duration of the probe. Callers **must not** +/// already hold the parse lock when calling this function, or the +/// second acquisition will deadlock. fn load_library_full( library_file: Option<&Path>, library_search_dir: Option<&Path>, @@ -533,9 +541,10 @@ fn load_library_full( let lib = Arc::new(MediaInfoLib::load_from_paths(&paths)?); - // Probe the library version through a throwaway handle so the - // returned `LibVersion` can feed downstream feature gating without - // any caller ever having to re-query it. + // Serialize the probe with any concurrent parse. The probe handle + // drops before the guard is released, so both `MediaInfo_New` and + // `MediaInfo_Delete` happen inside the locked region. + let _parse_guard = parse_lock(); let probe_handle = MediaInfoHandle::new(lib.clone()); let version_str = probe_handle.option("Info_Version", ""); @@ -949,16 +958,20 @@ impl MediaInfo { filename: P, options: &ParseOptions, ) -> Result { - let _parse_guard = parse_lock(); - - // When the caller pins the library path or search directory, - // honor the v0.1.0 contract by loading a fresh library for this - // call instead of reusing the cached default context. + // The library must be resolved BEFORE we acquire the parse + // lock: `load_library_full` and `default_context` both take + // the parse lock internally to serialize their probe handles, + // so entering them while already holding it would deadlock. if options.library_file.is_some() || options.library_search_dir.is_some() { + // Honor the v0.1.0 contract: when the caller pins the + // library path or search directory, load a fresh library + // for this call instead of reusing the cached default + // context. let (lib, version_number, version) = load_library_full( options.library_file.as_deref(), options.library_search_dir.as_deref(), )?; + let _parse_guard = parse_lock(); let loaded = LoadedLibrary { lib: &lib, version, @@ -968,6 +981,7 @@ impl MediaInfo { } let ctx = default_context()?; + let _parse_guard = parse_lock(); let loaded = ctx.loaded(); Self::parse_to_string_internal_unlocked(&loaded, filename, options) } @@ -1013,13 +1027,15 @@ impl MediaInfo { } fn parse_to_string_from_url(url: &str, options: &ParseOptions) -> Result { - let _parse_guard = parse_lock(); - + // Resolve the library before taking the parse lock so the + // inner probe-handle serialization in `load_library_full` / + // `default_context` does not re-enter the lock. if options.library_file.is_some() || options.library_search_dir.is_some() { let (lib, version_number, version) = load_library_full( options.library_file.as_deref(), options.library_search_dir.as_deref(), )?; + let _parse_guard = parse_lock(); let loaded = LoadedLibrary { lib: &lib, version, @@ -1029,6 +1045,7 @@ impl MediaInfo { } let ctx = default_context()?; + let _parse_guard = parse_lock(); let loaded = ctx.loaded(); Self::parse_to_string_from_url_unlocked(&loaded, url, options) } @@ -1190,13 +1207,15 @@ impl MediaInfo { reader: &mut R, options: &ParseOptions, ) -> Result { - let _parse_guard = parse_lock(); - + // Resolve the library before taking the parse lock so the + // inner probe-handle serialization in `load_library_full` / + // `default_context` does not re-enter the lock. if options.library_file.is_some() || options.library_search_dir.is_some() { let (lib, version_number, version) = load_library_full( options.library_file.as_deref(), options.library_search_dir.as_deref(), )?; + let _parse_guard = parse_lock(); let loaded = LoadedLibrary { lib: &lib, version, @@ -1206,6 +1225,7 @@ impl MediaInfo { } let ctx = default_context()?; + let _parse_guard = parse_lock(); let loaded = ctx.loaded(); Self::parse_reader_to_string_internal_unlocked(&loaded, reader, options) }