From 3a9a2dea7f42e7a91fb34c3b2015063d09e1c8df Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Tue, 2 Jan 2024 16:09:29 -0500 Subject: [PATCH] Probe: Introduce `GlobalOptions` to handle allocations and resolvers This moves `ParseOptions::{use_custom_resolvers, allocation_limit}` to the new `GlobalOptions` struct. This makes it easier to check for certain options in places where `ParseOptions` may not be available. `GlobalOptions` are not truly global, they only apply to the current thread. --- CHANGELOG.md | 2 + examples/custom_resolver/src/main.rs | 8 +- src/file.rs | 17 ++-- src/global_options.rs | 121 +++++++++++++++++++++++++++ src/lib.rs | 3 + src/probe.rs | 98 ++++------------------ src/resolve.rs | 4 + src/util/alloc.rs | 20 +---- 8 files changed, 169 insertions(+), 104 deletions(-) create mode 100644 src/global_options.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 30ff06d6..6dd20b44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `Id3v2ErrorKind::EmptyFrame` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/299)) - Support converting some TIPL frame values into generic `TagItem`s ([PR](https://github.com/Serial-ATA/lofty-rs/pull/301)) - Supported TIPL keys are: "producer", "arranger", "engineer", "DJ-mix", "mix". +- **GlobalOptions**: Options local to the thread that persist between reads and writes ([PR](https://github.com/Serial-ATA/lofty-rs/pull/321)) + - See [the docs](https://docs.rs/lofty/latest/lofty/struct.GlobalOptions.html) for more information ### Changed - **ID3v1**: Renamed `GENRES[14]` to `"R&B"` (Previously `"Rhythm & Blues"`) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/296)) diff --git a/examples/custom_resolver/src/main.rs b/examples/custom_resolver/src/main.rs index bec12520..c76fcb9e 100644 --- a/examples/custom_resolver/src/main.rs +++ b/examples/custom_resolver/src/main.rs @@ -2,8 +2,9 @@ use lofty::ape::ApeTag; use lofty::error::Result as LoftyResult; use lofty::id3::v2::Id3v2Tag; use lofty::resolve::FileResolver; -use lofty::{FileProperties, FileType, ParseOptions, TagType}; +use lofty::{FileProperties, FileType, GlobalOptions, ParseOptions, TagType}; use lofty_attr::LoftyFile; + use std::fs::File; #[rustfmt::skip] @@ -90,6 +91,11 @@ fn main() { // The name should preferably match the name of the file struct to avoid confusion. lofty::resolve::register_custom_resolver::("MyFile"); + // By default, lofty will not check for custom files. + // We can enable this by updating our `GlobalOptions`. + let global_options = GlobalOptions::new().use_custom_resolvers(true); + lofty::apply_global_options(global_options); + // Now when using the following functions, your custom file will be checked let path = "examples/custom_resolver/test_asset.myfile"; diff --git a/src/file.rs b/src/file.rs index 66947fcd..b6be904a 100644 --- a/src/file.rs +++ b/src/file.rs @@ -1,4 +1,5 @@ use crate::error::Result; +use crate::global_options::global_options; use crate::probe::ParseOptions; use crate::properties::FileProperties; use crate::resolve::custom_resolvers; @@ -818,13 +819,15 @@ impl FileType { let ext = ext.as_ref().to_str()?.to_ascii_lowercase(); // Give custom resolvers priority - if let Some((ty, _)) = custom_resolvers() - .lock() - .ok()? - .iter() - .find(|(_, f)| f.extension() == Some(ext.as_str())) - { - return Some(Self::Custom(ty)); + if unsafe { global_options().use_custom_resolvers } { + if let Some((ty, _)) = custom_resolvers() + .lock() + .ok()? + .iter() + .find(|(_, f)| f.extension() == Some(ext.as_str())) + { + return Some(Self::Custom(ty)); + } } match ext.as_str() { diff --git a/src/global_options.rs b/src/global_options.rs new file mode 100644 index 00000000..28ff5ff0 --- /dev/null +++ b/src/global_options.rs @@ -0,0 +1,121 @@ +use std::cell::UnsafeCell; + +thread_local! { + static GLOBAL_OPTIONS: UnsafeCell = UnsafeCell::new(GlobalOptions::default()); +} + +pub(crate) unsafe fn global_options() -> &'static GlobalOptions { + GLOBAL_OPTIONS.with(|global_options| unsafe { &*global_options.get() }) +} + +/// Options that control all interactions with Lofty for the current thread +/// +/// # Examples +/// +/// ```rust +/// use lofty::GlobalOptions; +/// +/// // I have a custom resolver that I need checked +/// let global_options = GlobalOptions::new().use_custom_resolvers(true); +/// lofty::apply_global_options(global_options); +/// ``` +#[derive(Copy, Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] +#[non_exhaustive] +pub struct GlobalOptions { + pub(crate) use_custom_resolvers: bool, + pub(crate) allocation_limit: usize, +} + +impl GlobalOptions { + /// Default allocation limit for any single tag item + pub const DEFAULT_ALLOCATION_LIMIT: usize = 16 * 1024 * 1024; + + /// Creates a new `GlobalOptions`, alias for `Default` implementation + /// + /// See also: [`GlobalOptions::default`] + /// + /// # Examples + /// + /// ```rust + /// use lofty::GlobalOptions; + /// + /// let global_options = GlobalOptions::new(); + /// ``` + #[must_use] + pub const fn new() -> Self { + Self { + use_custom_resolvers: true, + allocation_limit: Self::DEFAULT_ALLOCATION_LIMIT, + } + } + + /// Whether or not to check registered custom resolvers + /// + /// See also: [`crate::resolve`] + /// + /// # Examples + /// + /// ```rust + /// use lofty::GlobalOptions; + /// + /// // By default, `use_custom_resolvers` is enabled. Here, we don't want to use them. + /// let global_options = GlobalOptions::new().use_custom_resolvers(false); + /// lofty::apply_global_options(global_options); + /// ``` + pub fn use_custom_resolvers(&mut self, use_custom_resolvers: bool) -> Self { + self.use_custom_resolvers = use_custom_resolvers; + *self + } + + /// The maximum number of bytes to allocate for any single tag item + /// + /// This is a safety measure to prevent allocating too much memory for a single tag item. If a tag item + /// exceeds this limit, the allocator will return [`crate::error::ErrorKind::TooMuchData`]. + /// + /// # Examples + /// + /// ```rust + /// use lofty::GlobalOptions; + /// + /// // I have files with gigantic images, I'll double the allocation limit! + /// let global_options = GlobalOptions::new().allocation_limit(32 * 1024 * 1024); + /// lofty::apply_global_options(global_options); + /// ``` + pub fn allocation_limit(&mut self, allocation_limit: usize) -> Self { + self.allocation_limit = allocation_limit; + *self + } +} + +impl Default for GlobalOptions { + /// The default implementation for `GlobalOptions` + /// + /// The defaults are as follows: + /// + /// ```rust,ignore + /// GlobalOptions { + /// use_custom_resolvers: true, + /// allocation_limit: Self::DEFAULT_ALLOCATION_LIMIT, + /// } + /// ``` + fn default() -> Self { + Self::new() + } +} + +/// Applies the given `GlobalOptions` to the current thread +/// +/// # Examples +/// +/// ```rust +/// use lofty::GlobalOptions; +/// +/// // I have a custom resolver that I need checked +/// let global_options = GlobalOptions::new().use_custom_resolvers(true); +/// lofty::apply_global_options(global_options); +/// ``` +pub fn apply_global_options(options: GlobalOptions) { + GLOBAL_OPTIONS.with(|global_options| unsafe { + *global_options.get() = options; + }); +} diff --git a/src/lib.rs b/src/lib.rs index d5ebae6e..94ea72ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -154,6 +154,7 @@ pub mod ape; pub mod error; pub(crate) mod file; pub mod flac; +pub(crate) mod global_options; pub mod id3; pub mod iff; pub(crate) mod macros; @@ -186,4 +187,6 @@ pub use crate::traits::{Accessor, MergeTag, SplitTag, TagExt}; pub use picture::PictureInformation; +pub use global_options::{apply_global_options, GlobalOptions}; + pub use lofty_attr::LoftyFile; diff --git a/src/probe.rs b/src/probe.rs index 68fc5296..175e7331 100644 --- a/src/probe.rs +++ b/src/probe.rs @@ -3,6 +3,7 @@ use crate::ape::ApeFile; use crate::error::Result; use crate::file::{AudioFile, FileType, FileTypeGuessResult, TaggedFile}; use crate::flac::FlacFile; +use crate::global_options::global_options; use crate::iff::aiff::AiffFile; use crate::iff::wav::WavFile; use crate::macros::err; @@ -25,10 +26,8 @@ use std::path::Path; #[non_exhaustive] pub struct ParseOptions { pub(crate) read_properties: bool, - pub(crate) use_custom_resolvers: bool, pub(crate) parsing_mode: ParsingMode, pub(crate) max_junk_bytes: usize, - pub(crate) allocation_limit: usize, } impl Default for ParseOptions { @@ -39,7 +38,6 @@ impl Default for ParseOptions { /// ```rust,ignore /// ParseOptions { /// read_properties: true, - /// use_custom_resolvers: true, /// parsing_mode: ParsingMode::BestAttempt, /// max_junk_bytes: 1024 /// } @@ -56,9 +54,6 @@ impl ParseOptions { /// Default number of junk bytes to read pub const DEFAULT_MAX_JUNK_BYTES: usize = 1024; - /// Default allocation limit for any single tag item - pub const DEFAULT_ALLOCATION_LIMIT: usize = 16 * 1024 * 1024; - /// Creates a new `ParseOptions`, alias for `Default` implementation /// /// See also: [`ParseOptions::default`] @@ -74,10 +69,8 @@ impl ParseOptions { pub const fn new() -> Self { Self { read_properties: true, - use_custom_resolvers: true, parsing_mode: Self::DEFAULT_PARSING_MODE, max_junk_bytes: Self::DEFAULT_MAX_JUNK_BYTES, - allocation_limit: Self::DEFAULT_ALLOCATION_LIMIT, } } @@ -96,23 +89,6 @@ impl ParseOptions { *self } - /// Whether or not to check registered custom resolvers - /// - /// See also: [`crate::resolve`] - /// - /// # Examples - /// - /// ```rust - /// use lofty::ParseOptions; - /// - /// // By default, `use_custom_resolvers` is enabled. Here, we don't want to use them. - /// let parsing_options = ParseOptions::new().use_custom_resolvers(false); - /// ``` - pub fn use_custom_resolvers(&mut self, use_custom_resolvers: bool) -> Self { - self.use_custom_resolvers = use_custom_resolvers; - *self - } - /// The parsing mode to use, see [`ParsingMode`] for details /// /// # Examples @@ -145,35 +121,6 @@ impl ParseOptions { self.max_junk_bytes = max_junk_bytes; *self } - - /// The maximum number of bytes to allocate for any single tag item - /// - /// This is a safety measure to prevent allocating too much memory for a single tag item. If a tag item - /// exceeds this limit, the allocator will return [`crate::error::ErrorKind::TooMuchData`]. - /// - /// NOTE: This only needs to be set once per thread. The limit will be used for all subsequent - /// reads, until a new one is set. - /// - /// # Examples - /// - /// ```rust - /// use lofty::ParseOptions; - /// - /// // I have files with gigantic images, I'll double the allocation limit! - /// let parsing_options = ParseOptions::new().allocation_limit(32 * 1024 * 1024); - /// ``` - pub fn allocation_limit(&mut self, allocation_limit: usize) -> Self { - self.allocation_limit = allocation_limit; - *self - } - - fn finalize(self) -> Self { - unsafe { - crate::util::alloc::update_allocation_limit(self.allocation_limit); - } - - self - } } /// The parsing strictness mode @@ -532,11 +479,13 @@ impl Probe { self.inner.seek(SeekFrom::Start(starting_position))?; // Give custom resolvers priority - if let Ok(lock) = custom_resolvers().lock() { - #[allow(clippy::significant_drop_in_scrutinee)] - for (_, resolve) in lock.iter() { - if let ret @ Some(_) = resolve.guess(&buf[..buf_len]) { - return Ok(ret); + if unsafe { global_options().use_custom_resolvers } { + if let Ok(lock) = custom_resolvers().lock() { + #[allow(clippy::significant_drop_in_scrutinee)] + for (_, resolve) in lock.iter() { + if let ret @ Some(_) = resolve.guess(&buf[..buf_len]) { + return Ok(ret); + } } } } @@ -593,18 +542,6 @@ impl Probe { ret }, - _ => { - if let Ok(lock) = custom_resolvers().lock() { - #[allow(clippy::significant_drop_in_scrutinee)] - for (_, resolve) in lock.iter() { - if let ret @ Some(_) = resolve.guess(&buf[..buf_len]) { - return Ok(ret); - } - } - } - - Ok(None) - }, } } @@ -664,9 +601,7 @@ impl Probe { /// ``` pub fn read(mut self) -> Result { let reader = &mut self.inner; - let options = self - .options - .map_or_else(ParseOptions::default, ParseOptions::finalize); + let options = self.options.unwrap_or_default(); match self.f_ty { Some(f_type) => Ok(match f_type { @@ -683,7 +618,7 @@ impl Probe { FileType::Speex => SpeexFile::read_from(reader, options)?.into(), FileType::WavPack => WavPackFile::read_from(reader, options)?.into(), FileType::Custom(c) => { - if !options.use_custom_resolvers { + if !unsafe { global_options().use_custom_resolvers } { err!(UnknownFormat) } @@ -752,7 +687,7 @@ where #[cfg(test)] mod tests { - use crate::{FileType, Probe}; + use crate::{FileType, GlobalOptions, Probe}; use lofty::ParseOptions; use std::fs::File; @@ -846,9 +781,10 @@ mod tests { .collect::>() } - let parse_options = ParseOptions::new() - .allocation_limit(50) - .read_properties(false); + let parse_options = ParseOptions::new().read_properties(false); + + let mut global_options = GlobalOptions::new().allocation_limit(50); + crate::global_options::apply_global_options(global_options); // An allocation with a size of 40 bytes should be ok let within_limits = create_fake_mp3(40); @@ -865,7 +801,9 @@ mod tests { assert!(probe.read().is_err()); // Now test the default allocation limit (16MB), which should of course be ok with 60 bytes - let parse_options = ParseOptions::new().read_properties(false); + global_options.allocation_limit = GlobalOptions::DEFAULT_ALLOCATION_LIMIT; + crate::global_options::apply_global_options(global_options); + let probe = Probe::new(std::io::Cursor::new(&too_big)) .set_file_type(FileType::Mpeg) .options(parse_options); diff --git a/src/resolve.rs b/src/resolve.rs index 5f291c6d..fb759e95 100644 --- a/src/resolve.rs +++ b/src/resolve.rs @@ -130,6 +130,7 @@ pub fn register_custom_resolver(name: &'static str) { #[cfg(test)] mod tests { use crate::file::{FileType, TaggedFileExt}; + use crate::global_options::GlobalOptions; use crate::id3::v2::Id3v2Tag; use crate::probe::ParseOptions; use crate::properties::FileProperties; @@ -194,6 +195,9 @@ mod tests { fn custom_resolver() { register_custom_resolver::("MyFile"); + let global_options = GlobalOptions::new().use_custom_resolvers(true); + crate::apply_global_options(global_options); + let path = "examples/custom_resolver/test_asset.myfile"; let read = crate::read_from_path(path).unwrap(); assert_eq!(read.file_type(), FileType::Custom("MyFile")); diff --git a/src/util/alloc.rs b/src/util/alloc.rs index a18e40d4..0fe53c91 100644 --- a/src/util/alloc.rs +++ b/src/util/alloc.rs @@ -1,16 +1,7 @@ use crate::error::Result; use crate::macros::err; -use crate::probe::ParseOptions; -use std::cell::UnsafeCell; - -thread_local! { - static ALLOCATION_LIMIT: UnsafeCell = const { UnsafeCell::new(ParseOptions::DEFAULT_ALLOCATION_LIMIT) }; -} - -pub(crate) unsafe fn update_allocation_limit(limit: usize) { - ALLOCATION_LIMIT.with(|l| *l.get() = limit); -} +use crate::global_options::global_options; /// Provides the `fallible_repeat` method on `Vec` /// @@ -30,12 +21,9 @@ impl VecFallibleRepeat for Vec { return Ok(self); } - ALLOCATION_LIMIT.with(|limit| { - if expected_size > unsafe { *limit.get() } { - err!(TooMuchData); - } - Ok(()) - })?; + if expected_size > unsafe { global_options().allocation_limit } { + err!(TooMuchData); + } self.try_reserve(expected_size)?;