diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe60b86d..005b3d995 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- **ParseOptions**: `ParseOptions::read_tags` to skip the parsing of tags ([issue](https://github.com/Serial-ATA/lofty-rs/issues/251)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/406)) + ## [0.20.1] - 2024-07-02 ### Fixed diff --git a/lofty/src/aac/read.rs b/lofty/src/aac/read.rs index 43c04b75f..6ad54ed85 100644 --- a/lofty/src/aac/read.rs +++ b/lofty/src/aac/read.rs @@ -48,18 +48,20 @@ where stream_len -= u64::from(header.size); - let id3v2 = parse_id3v2(reader, header, parse_mode)?; - if let Some(existing_tag) = &mut file.id3v2_tag { - log::warn!("Duplicate ID3v2 tag found, appending frames to previous tag"); - - // https://github.com/Serial-ATA/lofty-rs/issues/87 - // Duplicate tags should have their frames appended to the previous - for frame in id3v2.frames { - existing_tag.insert(frame); + if parse_options.read_tags { + let id3v2 = parse_id3v2(reader, header, parse_mode)?; + if let Some(existing_tag) = &mut file.id3v2_tag { + log::warn!("Duplicate ID3v2 tag found, appending frames to previous tag"); + + // https://github.com/Serial-ATA/lofty-rs/issues/87 + // Duplicate tags should have their frames appended to the previous + for frame in id3v2.frames { + existing_tag.insert(frame); + } + continue; } - continue; + file.id3v2_tag = Some(id3v2); } - file.id3v2_tag = Some(id3v2); // Skip over the footer if skip_footer { @@ -94,7 +96,7 @@ where } #[allow(unused_variables)] - let ID3FindResults(header, id3v1) = find_id3v1(reader, true)?; + let ID3FindResults(header, id3v1) = find_id3v1(reader, parse_options.read_tags)?; if header.is_some() { stream_len -= 128; diff --git a/lofty/src/ape/read.rs b/lofty/src/ape/read.rs index 4b01e996b..7c1b2ef6c 100644 --- a/lofty/src/ape/read.rs +++ b/lofty/src/ape/read.rs @@ -8,7 +8,7 @@ use crate::id3::v1::tag::Id3v1Tag; use crate::id3::v2::read::parse_id3v2; use crate::id3::v2::tag::Id3v2Tag; use crate::id3::{find_id3v1, find_id3v2, find_lyrics3v2, FindId3v2Config, ID3FindResults}; -use crate::macros::decode_err; +use crate::macros::{decode_err, err}; use std::io::{Read, Seek, SeekFrom}; @@ -27,10 +27,14 @@ where let mut id3v1_tag: Option = None; let mut ape_tag: Option = None; + let find_id3v2_config = if parse_options.read_tags { + FindId3v2Config::READ_TAG + } else { + FindId3v2Config::NO_READ_TAG + }; + // ID3v2 tags are unsupported in APE files, but still possible - if let ID3FindResults(Some(header), Some(content)) = - find_id3v2(data, FindId3v2Config::READ_TAG)? - { + if let ID3FindResults(Some(header), content) = find_id3v2(data, find_id3v2_config)? { log::warn!("Encountered an ID3v2 tag. This tag cannot be rewritten to the APE file!"); stream_len -= u64::from(header.size); @@ -40,10 +44,12 @@ where stream_len -= 10; } - let reader = &mut &*content; + if let Some(content) = content { + let reader = &mut &*content; - let id3v2 = parse_id3v2(reader, header, parse_options.parsing_mode)?; - id3v2_tag = Some(id3v2); + let id3v2 = parse_id3v2(reader, header, parse_options.parsing_mode)?; + id3v2_tag = Some(id3v2); + } } let mut found_mac = false; @@ -76,14 +82,16 @@ where })?; if &remaining[..4] != b"AGEX" { - decode_err!(@BAIL Ape, "Found incomplete APE tag"); + err!(FakeTag) } let ape_header = read_ape_header(data, false)?; stream_len -= u64::from(ape_header.size); - let ape = read_ape_tag_with_header(data, ape_header)?; - ape_tag = Some(ape); + if parse_options.read_tags { + let ape = read_ape_tag_with_header(data, ape_header)?; + ape_tag = Some(ape); + } }, _ => { decode_err!(@BAIL Ape, "Invalid data found while reading header, expected any of [\"MAC \", \"APETAGEX\", \"ID3\"]") @@ -96,7 +104,7 @@ where // Starts with ['T', 'A', 'G'] // Exactly 128 bytes long (including the identifier) #[allow(unused_variables)] - let ID3FindResults(id3v1_header, id3v1) = find_id3v1(data, true)?; + let ID3FindResults(id3v1_header, id3v1) = find_id3v1(data, parse_options.read_tags)?; if id3v1_header.is_some() { stream_len -= 128; @@ -117,9 +125,9 @@ where // Strongly recommended to be at the end of the file data.seek(SeekFrom::Current(-32))?; - if let Some((tag, header)) = read_ape_tag(data, true)? { + if let (tag, Some(header)) = read_ape_tag(data, true, parse_options.read_tags)? { stream_len -= u64::from(header.size); - ape_tag = Some(tag); + ape_tag = tag; } let file_length = data.stream_position()?; diff --git a/lofty/src/ape/tag/mod.rs b/lofty/src/ape/tag/mod.rs index d25cab76f..949c44df9 100644 --- a/lofty/src/ape/tag/mod.rs +++ b/lofty/src/ape/tag/mod.rs @@ -622,9 +622,11 @@ mod tests { let tag = crate::tag::utils::test_utils::read_path("tests/tags/assets/test.apev2"); let mut reader = Cursor::new(tag); - let (parsed_tag, _) = crate::ape::tag::read::read_ape_tag(&mut reader, false) - .unwrap() - .unwrap(); + let (Some(parsed_tag), _) = + crate::ape::tag::read::read_ape_tag(&mut reader, false, true).unwrap() + else { + unreachable!(); + }; assert_eq!(expected_tag.len(), parsed_tag.len()); @@ -638,9 +640,11 @@ mod tests { let tag_bytes = crate::tag::utils::test_utils::read_path("tests/tags/assets/test.apev2"); let mut reader = Cursor::new(tag_bytes); - let (parsed_tag, _) = crate::ape::tag::read::read_ape_tag(&mut reader, false) - .unwrap() - .unwrap(); + let (Some(parsed_tag), _) = + crate::ape::tag::read::read_ape_tag(&mut reader, false, true).unwrap() + else { + unreachable!(); + }; let mut writer = Vec::new(); parsed_tag @@ -649,9 +653,11 @@ mod tests { let mut temp_reader = Cursor::new(writer); - let (temp_parsed_tag, _) = crate::ape::tag::read::read_ape_tag(&mut temp_reader, false) - .unwrap() - .unwrap(); + let (Some(temp_parsed_tag), _) = + crate::ape::tag::read::read_ape_tag(&mut temp_reader, false, true).unwrap() + else { + unreachable!() + }; assert_eq!(parsed_tag, temp_parsed_tag); } @@ -661,9 +667,10 @@ mod tests { let tag_bytes = crate::tag::utils::test_utils::read_path("tests/tags/assets/test.apev2"); let mut reader = Cursor::new(tag_bytes); - let (ape, _) = crate::ape::tag::read::read_ape_tag(&mut reader, false) - .unwrap() - .unwrap(); + let (Some(ape), _) = crate::ape::tag::read::read_ape_tag(&mut reader, false, true).unwrap() + else { + unreachable!() + }; let tag: Tag = ape.into(); diff --git a/lofty/src/ape/tag/read.rs b/lofty/src/ape/tag/read.rs index 6096de00b..802a7888c 100644 --- a/lofty/src/ape/tag/read.rs +++ b/lofty/src/ape/tag/read.rs @@ -86,16 +86,20 @@ where pub(crate) fn read_ape_tag( reader: &mut R, footer: bool, -) -> Result> { + read_tag: bool, +) -> Result<(Option, Option)> { let mut ape_preamble = [0; 8]; reader.read_exact(&mut ape_preamble)?; + let mut ape_tag = None; if &ape_preamble == APE_PREAMBLE { let ape_header = header::read_ape_header(reader, footer)?; + if read_tag { + ape_tag = Some(read_ape_tag_with_header(reader, ape_header)?); + } - let ape = read_ape_tag_with_header(reader, ape_header)?; - return Ok(Some((ape, ape_header))); + return Ok((ape_tag, Some(ape_header))); } - Ok(None) + Ok((None, None)) } diff --git a/lofty/src/ape/tag/write.rs b/lofty/src/ape/tag/write.rs index 336fe9237..77e76848e 100644 --- a/lofty/src/ape/tag/write.rs +++ b/lofty/src/ape/tag/write.rs @@ -48,8 +48,8 @@ where let mut header_ape_tag = (false, (0, 0)); let start = file.stream_position()?; - match read::read_ape_tag(file, false)? { - Some((mut existing_tag, header)) => { + match read::read_ape_tag(file, false, true)? { + (Some(mut existing_tag), Some(header)) => { if write_options.respect_read_only { // Only keep metadata around that's marked read only existing_tag.items.retain(|i| i.read_only); @@ -61,7 +61,7 @@ where header_ape_tag = (true, (start, start + u64::from(header.size))) }, - None => { + _ => { file.seek(SeekFrom::Current(-8))?; }, } @@ -80,7 +80,7 @@ where // Also check this tag for any read only items let start = file.stream_position()? as usize + 32; - if let Some((mut existing_tag, header)) = read::read_ape_tag(file, true)? { + if let (Some(mut existing_tag), Some(header)) = read::read_ape_tag(file, true, true)? { if write_options.respect_read_only { existing_tag.items.retain(|i| i.read_only); diff --git a/lofty/src/config/parse_options.rs b/lofty/src/config/parse_options.rs index c11d6cdbd..e23ffb35b 100644 --- a/lofty/src/config/parse_options.rs +++ b/lofty/src/config/parse_options.rs @@ -3,6 +3,7 @@ #[non_exhaustive] pub struct ParseOptions { pub(crate) read_properties: bool, + pub(crate) read_tags: bool, pub(crate) parsing_mode: ParsingMode, pub(crate) max_junk_bytes: usize, } @@ -15,6 +16,7 @@ impl Default for ParseOptions { /// ```rust,ignore /// ParseOptions { /// read_properties: true, + /// read_tags: true, /// parsing_mode: ParsingMode::BestAttempt, /// max_junk_bytes: 1024 /// } @@ -46,6 +48,7 @@ impl ParseOptions { pub const fn new() -> Self { Self { read_properties: true, + read_tags: true, parsing_mode: Self::DEFAULT_PARSING_MODE, max_junk_bytes: Self::DEFAULT_MAX_JUNK_BYTES, } @@ -66,6 +69,21 @@ impl ParseOptions { *self } + /// Whether or not to read the tags + /// + /// # Examples + /// + /// ```rust + /// use lofty::config::ParseOptions; + /// + /// // By default, `read_tags` is enabled. Here, we don't want to read them. + /// let parsing_options = ParseOptions::new().read_tags(false); + /// ``` + pub fn read_tags(&mut self, read_tags: bool) -> Self { + self.read_tags = read_tags; + *self + } + /// The parsing mode to use, see [`ParsingMode`] for details /// /// # Examples diff --git a/lofty/src/flac/read.rs b/lofty/src/flac/read.rs index db961d3a9..ec0660336 100644 --- a/lofty/src/flac/read.rs +++ b/lofty/src/flac/read.rs @@ -47,10 +47,14 @@ where properties: FlacProperties::default(), }; + let find_id3v2_config = if parse_options.read_tags { + FindId3v2Config::READ_TAG + } else { + FindId3v2Config::NO_READ_TAG + }; + // It is possible for a FLAC file to contain an ID3v2 tag - if let ID3FindResults(Some(header), Some(content)) = - find_id3v2(data, FindId3v2Config::READ_TAG)? - { + if let ID3FindResults(Some(header), Some(content)) = find_id3v2(data, find_id3v2_config)? { log::warn!("Encountered an ID3v2 tag. This tag cannot be rewritten to the FLAC file!"); let reader = &mut &*content; @@ -78,7 +82,7 @@ where decode_err!(@BAIL Flac, "Encountered a zero-sized metadata block"); } - if block.ty == BLOCK_ID_VORBIS_COMMENTS { + if block.ty == BLOCK_ID_VORBIS_COMMENTS && parse_options.read_tags { log::debug!("Encountered a Vorbis Comments block, parsing"); // NOTE: According to the spec @@ -106,7 +110,7 @@ where continue; } - if block.ty == BLOCK_ID_PICTURE { + if block.ty == BLOCK_ID_PICTURE && parse_options.read_tags { log::debug!("Encountered a FLAC picture block, parsing"); match Picture::from_flac_bytes(&block.content, false, parse_options.parsing_mode) { diff --git a/lofty/src/iff/aiff/read.rs b/lofty/src/iff/aiff/read.rs index 69c099f72..d8a1998b7 100644 --- a/lofty/src/iff/aiff/read.rs +++ b/lofty/src/iff/aiff/read.rs @@ -69,7 +69,7 @@ where while chunks.next(data).is_ok() { match &chunks.fourcc { - b"ID3 " | b"id3 " => { + b"ID3 " | b"id3 " if parse_options.read_tags => { let tag = chunks.id3_chunk(data, parse_options.parsing_mode)?; if let Some(existing_tag) = id3v2_tag.as_mut() { log::warn!("Duplicate ID3v2 tag found, appending frames to previous tag"); @@ -95,12 +95,12 @@ where stream_len = chunks.size; chunks.skip(data)?; }, - b"ANNO" => { + b"ANNO" if parse_options.read_tags => { annotations.push(chunks.read_pstring(data, None)?); }, // These four chunks are expected to appear at most once per file, // so there's no need to replace anything we already read - b"COMT" if comments.is_empty() => { + b"COMT" if comments.is_empty() && parse_options.read_tags => { if chunks.size < 2 { continue; } @@ -123,13 +123,13 @@ where chunks.correct_position(data)?; }, - b"NAME" if text_chunks.name.is_none() => { + b"NAME" if text_chunks.name.is_none() && parse_options.read_tags => { text_chunks.name = Some(chunks.read_pstring(data, None)?); }, - b"AUTH" if text_chunks.author.is_none() => { + b"AUTH" if text_chunks.author.is_none() && parse_options.read_tags => { text_chunks.author = Some(chunks.read_pstring(data, None)?); }, - b"(c) " if text_chunks.copyright.is_none() => { + b"(c) " if text_chunks.copyright.is_none() && parse_options.read_tags => { text_chunks.copyright = Some(chunks.read_pstring(data, None)?); }, _ => chunks.skip(data)?, diff --git a/lofty/src/iff/wav/read.rs b/lofty/src/iff/wav/read.rs index d2c2d8cc2..f382475b1 100644 --- a/lofty/src/iff/wav/read.rs +++ b/lofty/src/iff/wav/read.rs @@ -78,7 +78,7 @@ where data.read_exact(&mut list_type)?; match &list_type { - b"INFO" => { + b"INFO" if parse_options.read_tags => { let end = data.stream_position()? + u64::from(chunks.size - 4); super::tag::read::parse_riff_info(data, &mut chunks, end, &mut riff_info)?; }, @@ -88,7 +88,7 @@ where }, } }, - b"ID3 " | b"id3 " => { + b"ID3 " | b"id3 " if parse_options.read_tags => { let tag = chunks.id3_chunk(data, parse_options.parsing_mode)?; if let Some(existing_tag) = id3v2_tag.as_mut() { log::warn!("Duplicate ID3v2 tag found, appending frames to previous tag"); diff --git a/lofty/src/mp4/moov.rs b/lofty/src/mp4/moov.rs index c2eed241e..c12686246 100644 --- a/lofty/src/mp4/moov.rs +++ b/lofty/src/mp4/moov.rs @@ -2,7 +2,7 @@ use super::atom_info::{AtomIdent, AtomInfo}; use super::ilst::read::parse_ilst; use super::ilst::Ilst; use super::read::{meta_is_full, nested_atom, skip_unneeded, AtomReader}; -use crate::config::ParsingMode; +use crate::config::{ParseOptions, ParsingMode}; use crate::error::Result; use crate::macros::decode_err; @@ -34,11 +34,7 @@ impl Moov { moov.ok_or_else(|| decode_err!(Mp4, "No \"moov\" atom found")) } - pub(super) fn parse( - reader: &mut AtomReader, - parse_mode: ParsingMode, - read_properties: bool, - ) -> Result + pub(super) fn parse(reader: &mut AtomReader, parse_options: ParseOptions) -> Result where R: Read + Seek, { @@ -48,15 +44,18 @@ impl Moov { while let Ok(Some(atom)) = reader.next() { if let AtomIdent::Fourcc(fourcc) = atom.ident { match &fourcc { - b"trak" if read_properties => { + b"trak" if parse_options.read_properties => { // All we need from here is trak.mdia - if let Some(mdia) = nested_atom(reader, atom.len, b"mdia", parse_mode)? { + if let Some(mdia) = + nested_atom(reader, atom.len, b"mdia", parse_options.parsing_mode)? + { skip_unneeded(reader, mdia.extended, mdia.len)?; traks.push(mdia); } }, - b"udta" => { - let ilst_parsed = ilst_from_udta(reader, parse_mode, atom.len - 8)?; + b"udta" if parse_options.read_tags => { + let ilst_parsed = + ilst_from_udta(reader, parse_options.parsing_mode, atom.len - 8)?; if let Some(ilst_parsed) = ilst_parsed { let Some(mut existing_ilst) = ilst else { ilst = Some(ilst_parsed); diff --git a/lofty/src/mp4/read.rs b/lofty/src/mp4/read.rs index 12c1b121d..08961327a 100644 --- a/lofty/src/mp4/read.rs +++ b/lofty/src/mp4/read.rs @@ -192,11 +192,7 @@ where let moov_info = Moov::find(&mut reader)?; reader.reset_bounds(moov_info.start + 8, moov_info.len - 8); - let moov = Moov::parse( - &mut reader, - parse_options.parsing_mode, - parse_options.read_properties, - )?; + let moov = Moov::parse(&mut reader, parse_options)?; Ok(Mp4File { ftyp, diff --git a/lofty/src/mpeg/read.rs b/lofty/src/mpeg/read.rs index 6f07de099..a6627b73b 100644 --- a/lofty/src/mpeg/read.rs +++ b/lofty/src/mpeg/read.rs @@ -43,16 +43,20 @@ where let header = Id3v2Header::parse(reader)?; let skip_footer = header.flags.footer; - let id3v2 = parse_id3v2(reader, header, parse_options.parsing_mode)?; - if let Some(existing_tag) = &mut file.id3v2_tag { - // https://github.com/Serial-ATA/lofty-rs/issues/87 - // Duplicate tags should have their frames appended to the previous - for frame in id3v2.frames { - existing_tag.insert(frame); + if parse_options.read_tags { + let id3v2 = parse_id3v2(reader, header, parse_options.parsing_mode)?; + if let Some(existing_tag) = &mut file.id3v2_tag { + // https://github.com/Serial-ATA/lofty-rs/issues/87 + // Duplicate tags should have their frames appended to the previous + for frame in id3v2.frames { + existing_tag.insert(frame); + } + continue; } - continue; + file.id3v2_tag = Some(id3v2); + } else { + reader.seek(SeekFrom::Current(i64::from(header.size)))?; } - file.id3v2_tag = Some(id3v2); // Skip over the footer if skip_footer { @@ -74,9 +78,13 @@ where if &header_remaining == b"AGEX" { let ape_header = read_ape_header(reader, false)?; - file.ape_tag = Some(crate::ape::tag::read::read_ape_tag_with_header( - reader, ape_header, - )?); + if parse_options.read_tags { + file.ape_tag = Some(crate::ape::tag::read::read_ape_tag_with_header( + reader, ape_header, + )?); + } else { + reader.seek(SeekFrom::Current(i64::from(ape_header.size)))?; + } continue; } @@ -105,7 +113,7 @@ where std::cmp::min(_first_frame_offset, parse_options.max_junk_bytes as u64); let config = FindId3v2Config { - read: true, + read: parse_options.read_tags, allowed_junk_window: Some(search_window_size), }; @@ -137,7 +145,7 @@ where } #[allow(unused_variables)] - let ID3FindResults(header, id3v1) = find_id3v1(reader, true)?; + let ID3FindResults(header, id3v1) = find_id3v1(reader, parse_options.read_tags)?; if header.is_some() { file.id3v1_tag = id3v1; @@ -147,15 +155,15 @@ where reader.seek(SeekFrom::Current(-32))?; - match crate::ape::tag::read::read_ape_tag(reader, true)? { - Some((tag, header)) => { - file.ape_tag = Some(tag); + match crate::ape::tag::read::read_ape_tag(reader, true, parse_options.read_tags)? { + (tag, Some(header)) => { + file.ape_tag = tag; // Seek back to the start of the tag let pos = reader.stream_position()?; reader.seek(SeekFrom::Start(pos - u64::from(header.size)))?; }, - None => { + _ => { // Correct the position (APE header - Preamble) reader.seek(SeekFrom::Current(24))?; }, diff --git a/lofty/src/musepack/read.rs b/lofty/src/musepack/read.rs index c82535f83..c75a1dd80 100644 --- a/lofty/src/musepack/read.rs +++ b/lofty/src/musepack/read.rs @@ -22,11 +22,15 @@ where let mut stream_length = reader.stream_len_hack()?; + let find_id3v2_config = if parse_options.read_tags { + FindId3v2Config::READ_TAG + } else { + FindId3v2Config::NO_READ_TAG + }; + // ID3v2 tags are unsupported in MPC files, but still possible #[allow(unused_variables)] - if let ID3FindResults(Some(header), Some(content)) = - find_id3v2(reader, FindId3v2Config::READ_TAG)? - { + if let ID3FindResults(Some(header), Some(content)) = find_id3v2(reader, find_id3v2_config)? { let reader = &mut &*content; let id3v2 = parse_id3v2(reader, header, parse_options.parsing_mode)?; @@ -39,7 +43,7 @@ where let pos_past_id3v2 = reader.stream_position()?; #[allow(unused_variables)] - let ID3FindResults(header, id3v1) = find_id3v1(reader, true)?; + let ID3FindResults(header, id3v1) = find_id3v1(reader, parse_options.read_tags)?; if header.is_some() { file.id3v1_tag = id3v1; @@ -51,8 +55,10 @@ where reader.seek(SeekFrom::Current(-32))?; - if let Some((tag, header)) = crate::ape::tag::read::read_ape_tag(reader, true)? { - file.ape_tag = Some(tag); + if let (tag, Some(header)) = + crate::ape::tag::read::read_ape_tag(reader, true, parse_options.read_tags)? + { + file.ape_tag = tag; // Seek back to the start of the tag let pos = reader.stream_position()?; diff --git a/lofty/src/ogg/opus/mod.rs b/lofty/src/ogg/opus/mod.rs index 4aa883346..032f15fad 100644 --- a/lofty/src/ogg/opus/mod.rs +++ b/lofty/src/ogg/opus/mod.rs @@ -30,7 +30,7 @@ impl OpusFile { R: Read + Seek, { let file_information = - super::read::read_from(reader, OPUSHEAD, OPUSTAGS, 2, parse_options.parsing_mode)?; + super::read::read_from(reader, OPUSHEAD, OPUSTAGS, 2, parse_options)?; Ok(Self { properties: if parse_options.read_properties { @@ -38,8 +38,8 @@ impl OpusFile { } else { OpusProperties::default() }, - // Safe to unwrap, a metadata packet is mandatory in Opus - vorbis_comments_tag: file_information.0.unwrap(), + // A metadata packet is mandatory in Opus + vorbis_comments_tag: file_information.0.unwrap_or_default(), }) } } diff --git a/lofty/src/ogg/opus/properties.rs b/lofty/src/ogg/opus/properties.rs index 63ad36dc1..4475c761c 100644 --- a/lofty/src/ogg/opus/properties.rs +++ b/lofty/src/ogg/opus/properties.rs @@ -32,7 +32,11 @@ impl From for FileProperties { sample_rate: Some(input.input_sample_rate), bit_depth: None, channels: Some(input.channels), - channel_mask: Some(input.channel_mask), + channel_mask: if input.channel_mask == ChannelMask(0) { + None + } else { + Some(input.channel_mask) + }, } } } diff --git a/lofty/src/ogg/read.rs b/lofty/src/ogg/read.rs index a7709c85a..26cd0b63c 100644 --- a/lofty/src/ogg/read.rs +++ b/lofty/src/ogg/read.rs @@ -1,6 +1,6 @@ use super::tag::VorbisComments; use super::verify_signature; -use crate::config::ParsingMode; +use crate::config::{ParseOptions, ParsingMode}; use crate::error::{ErrorKind, LoftyError, Result}; use crate::macros::{decode_err, err, parse_mode_choice}; use crate::picture::{MimeType, Picture, PictureInformation, PictureType}; @@ -189,7 +189,7 @@ pub(crate) fn read_from( header_sig: &[u8], comment_sig: &[u8], packets_to_read: isize, - parse_mode: ParsingMode, + parse_options: ParseOptions, ) -> Result where T: Read + Seek, @@ -210,6 +210,10 @@ where .ok_or_else(|| decode_err!("OGG: Expected identification packet"))?; verify_signature(identification_packet, header_sig)?; + if !parse_options.read_tags { + return Ok((None, first_page_header, packets)); + } + let mut metadata_packet = packets .get(1) .ok_or_else(|| decode_err!("OGG: Expected comment packet"))?; @@ -219,7 +223,7 @@ where metadata_packet = &metadata_packet[comment_sig.len()..]; let reader = &mut metadata_packet; - let tag = read_comments(reader, reader.len() as u64, parse_mode)?; + let tag = read_comments(reader, reader.len() as u64, parse_options.parsing_mode)?; Ok((Some(tag), first_page_header, packets)) } diff --git a/lofty/src/ogg/speex/mod.rs b/lofty/src/ogg/speex/mod.rs index 4af2b333d..85c0d12a4 100644 --- a/lofty/src/ogg/speex/mod.rs +++ b/lofty/src/ogg/speex/mod.rs @@ -28,8 +28,7 @@ impl SpeexFile { where R: Read + Seek, { - let file_information = - super::read::read_from(reader, SPEEXHEADER, &[], 2, parse_options.parsing_mode)?; + let file_information = super::read::read_from(reader, SPEEXHEADER, &[], 2, parse_options)?; Ok(Self { properties: if parse_options.read_properties { @@ -37,8 +36,8 @@ impl SpeexFile { } else { SpeexProperties::default() }, - // Safe to unwrap, a metadata packet is mandatory in Speex - vorbis_comments_tag: file_information.0.unwrap(), + // A metadata packet is mandatory in Speex + vorbis_comments_tag: file_information.0.unwrap_or_default(), }) } } diff --git a/lofty/src/ogg/vorbis/mod.rs b/lofty/src/ogg/vorbis/mod.rs index 6f401be8e..6cefcf358 100644 --- a/lofty/src/ogg/vorbis/mod.rs +++ b/lofty/src/ogg/vorbis/mod.rs @@ -34,7 +34,7 @@ impl VorbisFile { VORBIS_IDENT_HEAD, VORBIS_COMMENT_HEAD, 3, - parse_options.parsing_mode, + parse_options, )?; Ok(Self { @@ -43,8 +43,8 @@ impl VorbisFile { } else { VorbisProperties::default() }, - // Safe to unwrap, a metadata packet is mandatory in OGG Vorbis - vorbis_comments_tag: file_information.0.unwrap(), + // A metadata packet is mandatory in OGG Vorbis + vorbis_comments_tag: file_information.0.unwrap_or_default(), }) } } diff --git a/lofty/src/probe.rs b/lofty/src/probe.rs index 0d66d5c83..3017ced8c 100644 --- a/lofty/src/probe.rs +++ b/lofty/src/probe.rs @@ -459,6 +459,10 @@ impl Probe { let reader = &mut self.inner; let options = self.options.unwrap_or_default(); + if !options.read_tags && !options.read_properties { + log::warn!("Skipping both tag and property reading, file will be empty"); + } + match self.f_ty { Some(f_type) => Ok(match f_type { FileType::Aac => AacFile::read_from(reader, options)?.into(), diff --git a/lofty/src/properties/file_properties.rs b/lofty/src/properties/file_properties.rs index d60d14479..9d34d8e4c 100644 --- a/lofty/src/properties/file_properties.rs +++ b/lofty/src/properties/file_properties.rs @@ -85,4 +85,21 @@ impl FileProperties { pub fn channel_mask(&self) -> Option { self.channel_mask } + + /// Used for tests + #[doc(hidden)] + pub fn is_empty(&self) -> bool { + matches!( + self, + Self { + duration: Duration::ZERO, + overall_bitrate: None | Some(0), + audio_bitrate: None | Some(0), + sample_rate: None | Some(0), + bit_depth: None | Some(0), + channels: None | Some(0), + channel_mask: None, + } + ) + } } diff --git a/lofty/src/wavpack/properties.rs b/lofty/src/wavpack/properties.rs index 5eb5792cd..b2b4857bf 100644 --- a/lofty/src/wavpack/properties.rs +++ b/lofty/src/wavpack/properties.rs @@ -32,7 +32,11 @@ impl From for FileProperties { sample_rate: Some(input.sample_rate), bit_depth: Some(input.bit_depth), channels: Some(input.channels as u8), - channel_mask: Some(input.channel_mask), + channel_mask: if input.channel_mask == ChannelMask(0) { + None + } else { + Some(input.channel_mask) + }, } } } diff --git a/lofty/src/wavpack/read.rs b/lofty/src/wavpack/read.rs index b2ab8817f..14a689a5d 100644 --- a/lofty/src/wavpack/read.rs +++ b/lofty/src/wavpack/read.rs @@ -17,7 +17,7 @@ where let mut id3v1_tag = None; let mut ape_tag = None; - let ID3FindResults(id3v1_header, id3v1) = find_id3v1(reader, true)?; + let ID3FindResults(id3v1_header, id3v1) = find_id3v1(reader, parse_options.read_tags)?; if id3v1_header.is_some() { stream_length -= 128; @@ -38,9 +38,11 @@ where // Strongly recommended to be at the end of the file reader.seek(SeekFrom::Current(-32))?; - if let Some((tag, header)) = crate::ape::tag::read::read_ape_tag(reader, true)? { + if let (tag, Some(header)) = + crate::ape::tag::read::read_ape_tag(reader, true, parse_options.read_tags)? + { stream_length -= u64::from(header.size); - ape_tag = Some(tag); + ape_tag = tag; } Ok(WavPackFile { diff --git a/lofty/tests/files/aac.rs b/lofty/tests/files/aac.rs index 7d42b0bc3..e514c7e46 100644 --- a/lofty/tests/files/aac.rs +++ b/lofty/tests/files/aac.rs @@ -5,7 +5,7 @@ use lofty::prelude::*; use lofty::probe::Probe; use lofty::tag::TagType; -use std::io::{Seek, Write}; +use std::io::Seek; #[test] fn read() { @@ -96,3 +96,13 @@ fn remove_id3v2() { fn remove_id3v1() { crate::remove_tag!("tests/files/assets/minimal/full_test.aac", TagType::Id3v1); } + +#[test] +fn read_no_properties() { + crate::no_properties_test!("tests/files/assets/minimal/full_test.aac"); +} + +#[test] +fn read_no_tags() { + crate::no_tag_test!("tests/files/assets/minimal/full_test.aac"); +} diff --git a/lofty/tests/files/aiff.rs b/lofty/tests/files/aiff.rs index de7e9686b..72e323792 100644 --- a/lofty/tests/files/aiff.rs +++ b/lofty/tests/files/aiff.rs @@ -5,7 +5,7 @@ use lofty::prelude::*; use lofty::probe::Probe; use lofty::tag::TagType; -use std::io::{Seek, Write}; +use std::io::Seek; #[test] fn read() { @@ -70,3 +70,13 @@ fn remove_text_chunks() { fn remove_id3v2() { crate::remove_tag!("tests/files/assets/minimal/full_test.aiff", TagType::Id3v2); } + +#[test] +fn read_no_properties() { + crate::no_properties_test!("tests/files/assets/minimal/full_test.aiff"); +} + +#[test] +fn read_no_tags() { + crate::no_tag_test!("tests/files/assets/minimal/full_test.aiff"); +} diff --git a/lofty/tests/files/ape.rs b/lofty/tests/files/ape.rs index 3af288013..8ec458d51 100644 --- a/lofty/tests/files/ape.rs +++ b/lofty/tests/files/ape.rs @@ -5,7 +5,7 @@ use lofty::prelude::*; use lofty::probe::Probe; use lofty::tag::TagType; -use std::io::{Seek, Write}; +use std::io::Seek; #[test] fn read() { @@ -76,3 +76,13 @@ fn remove_id3v1() { fn remove_id3v2() { crate::remove_tag!("tests/files/assets/minimal/full_test.ape", TagType::Id3v2); } + +#[test] +fn read_no_properties() { + crate::no_properties_test!("tests/files/assets/minimal/full_test.ape"); +} + +#[test] +fn read_no_tags() { + crate::no_tag_test!("tests/files/assets/minimal/full_test.ape"); +} diff --git a/lofty/tests/files/flac.rs b/lofty/tests/files/flac.rs index a9401280d..dd4f78aa3 100644 --- a/lofty/tests/files/flac.rs +++ b/lofty/tests/files/flac.rs @@ -30,3 +30,13 @@ fn multiple_vorbis_comments() { Some("Artist 2") ); } + +#[test] +fn read_no_properties() { + crate::no_properties_test!("tests/files/assets/minimal/full_test.flac"); +} + +#[test] +fn read_no_tags() { + crate::no_tag_test!("tests/files/assets/minimal/full_test.flac"); +} diff --git a/lofty/tests/files/mp4.rs b/lofty/tests/files/mp4.rs index 0020f675c..cb6bbb1f7 100644 --- a/lofty/tests/files/mp4.rs +++ b/lofty/tests/files/mp4.rs @@ -5,7 +5,7 @@ use lofty::prelude::*; use lofty::probe::Probe; use lofty::tag::TagType; -use std::io::{Seek, Write}; +use std::io::Seek; #[test] fn read() { @@ -58,3 +58,13 @@ fn remove() { TagType::Mp4Ilst ); } + +#[test] +fn read_no_properties() { + crate::no_properties_test!("tests/files/assets/minimal/m4a_codec_aac.m4a"); +} + +#[test] +fn read_no_tags() { + crate::no_tag_test!("tests/files/assets/minimal/m4a_codec_aac.m4a"); +} diff --git a/lofty/tests/files/mpc.rs b/lofty/tests/files/mpc.rs index c9ab4466e..9d29f6dc9 100644 --- a/lofty/tests/files/mpc.rs +++ b/lofty/tests/files/mpc.rs @@ -6,7 +6,7 @@ use lofty::prelude::*; use lofty::probe::Probe; use lofty::tag::TagType; -use std::io::{Seek, Write}; +use std::io::Seek; // Marker test so IntelliJ Rust recognizes this as a test module #[test] @@ -79,6 +79,16 @@ macro_rules! generate_tests { fn []() { crate::remove_tag!($path, TagType::Ape); } + + #[test] + fn []() { + crate::no_properties_test!($path); + } + + #[test] + fn []() { + crate::no_tag_test!($path); + } } }; } diff --git a/lofty/tests/files/mpeg.rs b/lofty/tests/files/mpeg.rs index ceda308bf..dd1fc4975 100644 --- a/lofty/tests/files/mpeg.rs +++ b/lofty/tests/files/mpeg.rs @@ -8,7 +8,7 @@ use lofty::probe::Probe; use lofty::tag::{Tag, TagType}; use std::borrow::Cow; -use std::io::{Seek, Write}; +use std::io::Seek; #[test] fn read() { @@ -369,3 +369,26 @@ fn read_and_write_tpil_frame() { assert_eq!(key_value_pairs, content.key_value_pairs); } + +#[test] +fn read_no_properties() { + let mut file = crate::temp_file!("tests/files/assets/minimal/full_test.mp3"); + let tagged_file = Probe::new(&mut file) + .options(ParseOptions::new().read_properties(false)) + .guess_file_type() + .unwrap() + .read() + .unwrap(); + let properties = tagged_file.properties(); + assert!(properties.duration().is_zero()); + assert_eq!(properties.overall_bitrate(), Some(0)); + assert_eq!(properties.audio_bitrate(), Some(0)); + assert_eq!(properties.sample_rate(), Some(0)); + assert!(properties.bit_depth().is_none()); + assert_eq!(properties.channels(), Some(0)); +} + +#[test] +fn read_no_tags() { + crate::no_tag_test!("tests/files/assets/minimal/full_test.mp3"); +} diff --git a/lofty/tests/files/ogg.rs b/lofty/tests/files/ogg.rs index 1f002f09c..1d2903450 100644 --- a/lofty/tests/files/ogg.rs +++ b/lofty/tests/files/ogg.rs @@ -5,9 +5,9 @@ use lofty::prelude::*; use lofty::probe::Probe; use lofty::tag::TagType; -use std::io::{Seek, Write}; +use std::io::Seek; -// The tests for OGG Opus/Vorbis are nearly identical +// The tests for OGG Opus/Vorbis/Speex are nearly identical // We have the vendor string and a title stored in the tag #[test] @@ -186,3 +186,33 @@ fn flac_try_write_non_empty_id3v2() { ) .is_err()); } + +#[test] +fn read_no_properties_opus() { + crate::no_properties_test!("tests/files/assets/minimal/full_test.opus"); +} + +#[test] +fn read_no_tags_opus() { + crate::no_tag_test!(@MANDATORY_TAG "tests/files/assets/minimal/full_test.opus", expected_len: 1); +} + +#[test] +fn read_no_properties_vorbis() { + crate::no_properties_test!("tests/files/assets/minimal/full_test.ogg"); +} + +#[test] +fn read_no_tags_vorbis() { + crate::no_tag_test!(@MANDATORY_TAG "tests/files/assets/minimal/full_test.ogg", expected_len: 1); +} + +#[test] +fn read_no_properties_speex() { + crate::no_properties_test!("tests/files/assets/minimal/full_test.spx"); +} + +#[test] +fn read_no_tags_speex() { + crate::no_tag_test!(@MANDATORY_TAG "tests/files/assets/minimal/full_test.spx", expected_len: 1); +} diff --git a/lofty/tests/files/util/mod.rs b/lofty/tests/files/util/mod.rs index 72afe7694..8fd12fb4a 100644 --- a/lofty/tests/files/util/mod.rs +++ b/lofty/tests/files/util/mod.rs @@ -1,6 +1,8 @@ #[macro_export] macro_rules! temp_file { ($path:tt) => {{ + use std::io::Write as _; + let mut file = tempfile::tempfile().unwrap(); file.write_all(&std::fs::read($path).unwrap()).unwrap(); @@ -10,6 +12,48 @@ macro_rules! temp_file { }}; } +#[macro_export] +macro_rules! no_tag_test { + ($path:literal) => {{ + let mut file = crate::temp_file!($path); + let tagged_file = lofty::probe::Probe::new(&mut file) + .options(lofty::config::ParseOptions::new().read_tags(false)) + .guess_file_type() + .unwrap() + .read() + .unwrap(); + assert!(!tagged_file.contains_tag()); + }}; + (@MANDATORY_TAG $path:literal, expected_len: $expected_len:literal) => {{ + use lofty::tag::TagExt as _; + + let mut file = crate::temp_file!($path); + let tagged_file = lofty::probe::Probe::new(&mut file) + .options(lofty::config::ParseOptions::new().read_tags(false)) + .guess_file_type() + .unwrap() + .read() + .unwrap(); + for tag in tagged_file.tags() { + assert_eq!(tag.len(), $expected_len); + } + }}; +} + +#[macro_export] +macro_rules! no_properties_test { + ($path:literal) => {{ + let mut file = crate::temp_file!($path); + let tagged_file = lofty::probe::Probe::new(&mut file) + .options(lofty::config::ParseOptions::new().read_properties(false)) + .guess_file_type() + .unwrap() + .read() + .unwrap(); + assert!(tagged_file.properties().is_empty()); + }}; +} + #[macro_export] macro_rules! verify_artist { ($file:ident, $method:ident, $expected_value:literal, $item_count:expr) => {{ diff --git a/lofty/tests/files/wav.rs b/lofty/tests/files/wav.rs index 2f7b1eebd..12e64a6e9 100644 --- a/lofty/tests/files/wav.rs +++ b/lofty/tests/files/wav.rs @@ -5,7 +5,7 @@ use lofty::prelude::*; use lofty::probe::Probe; use lofty::tag::TagType; -use std::io::{Seek, Write}; +use std::io::Seek; #[test] fn read() { @@ -85,3 +85,13 @@ fn issue_174_divide_by_zero() { assert_eq!(file.file_type(), FileType::Wav); } + +#[test] +fn read_no_properties() { + crate::no_properties_test!("tests/files/assets/minimal/wav_format_pcm.wav"); +} + +#[test] +fn read_no_tags() { + crate::no_tag_test!("tests/files/assets/minimal/wav_format_pcm.wav"); +} diff --git a/lofty/tests/files/wavpack.rs b/lofty/tests/files/wavpack.rs index 8c6d81e72..a9bfc4f8a 100644 --- a/lofty/tests/files/wavpack.rs +++ b/lofty/tests/files/wavpack.rs @@ -5,7 +5,7 @@ use lofty::prelude::*; use lofty::probe::Probe; use lofty::tag::TagType; -use std::io::{Seek, Write}; +use std::io::Seek; #[test] fn read() { @@ -67,3 +67,13 @@ fn remove_id3v1() { fn remove_ape() { crate::remove_tag!("tests/files/assets/minimal/full_test.wv", TagType::Ape); } + +#[test] +fn read_no_properties() { + crate::no_properties_test!("tests/files/assets/minimal/full_test.wv"); +} + +#[test] +fn read_no_tags() { + crate::no_tag_test!("tests/files/assets/minimal/full_test.wv"); +}