From 2c3a04180780391dd87475874c8dca456cfce463 Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Tue, 24 Aug 2021 23:35:28 -0400 Subject: [PATCH] Clippy --- .../{read_from_signature.rs => read_file.rs} | 10 +-- benches/read_from_extension.rs | 32 --------- src/lib.rs | 4 +- src/logic/ape/mod.rs | 2 +- src/logic/ape/read.rs | 9 +++ src/logic/iff/aiff.rs | 2 +- src/logic/mpeg/mod.rs | 2 +- src/logic/ogg/flac.rs | 1 - src/logic/ogg/opus.rs | 1 - src/logic/ogg/vorbis.rs | 1 - src/probe.rs | 8 ++- src/types/picture.rs | 66 +++++++++---------- src/types/tag.rs | 2 +- 13 files changed, 61 insertions(+), 79 deletions(-) rename benches/{read_from_signature.rs => read_file.rs} (81%) delete mode 100644 benches/read_from_extension.rs diff --git a/benches/read_from_signature.rs b/benches/read_file.rs similarity index 81% rename from benches/read_from_signature.rs rename to benches/read_file.rs index c21e8ccd..aa0062b5 100644 --- a/benches/read_from_signature.rs +++ b/benches/read_file.rs @@ -1,10 +1,10 @@ use criterion::{criterion_group, criterion_main, Criterion}; -use lofty::Tag; +use lofty::Probe; macro_rules! test_read { ($function:ident, $path:expr) => { fn $function() { - Tag::new().read_from_path_signature($path).unwrap(); + Probe::new().read_from_path($path).unwrap(); } }; } @@ -12,18 +12,18 @@ macro_rules! test_read { test_read!(read_aiff, "tests/assets/a_text.aiff"); test_read!(read_ape, "tests/assets/a.ape"); test_read!(read_flac, "tests/assets/a.flac"); -test_read!(read_m4a, "tests/assets/a.m4a"); +//test_read!(read_m4a, "tests/assets/a.m4a"); TODO test_read!(read_mp3, "tests/assets/a.mp3"); test_read!(read_vorbis, "tests/assets/a.ogg"); test_read!(read_opus, "tests/assets/a.opus"); test_read!(read_riff, "tests/assets/a.wav"); fn bench_sig(c: &mut Criterion) { - let mut g = c.benchmark_group("From signature"); + let mut g = c.benchmark_group("File reading"); g.bench_function("AIFF", |b| b.iter(read_aiff)); g.bench_function("APE", |b| b.iter(read_ape)); g.bench_function("FLAC", |b| b.iter(read_flac)); - g.bench_function("MP4", |b| b.iter(read_m4a)); + // g.bench_function("MP4", |b| b.iter(read_m4a)); g.bench_function("MP3", |b| b.iter(read_mp3)); g.bench_function("VORBIS", |b| b.iter(read_vorbis)); g.bench_function("OPUS", |b| b.iter(read_opus)); diff --git a/benches/read_from_extension.rs b/benches/read_from_extension.rs deleted file mode 100644 index 88fc2eaa..00000000 --- a/benches/read_from_extension.rs +++ /dev/null @@ -1,32 +0,0 @@ -use criterion::{criterion_group, criterion_main, Criterion}; -use lofty::Tag; - -macro_rules! test_read { - ($function:ident, $path:expr) => { - fn $function() { - Tag::new().read_from_path($path).unwrap(); - } - }; -} - -test_read!(read_ape, "tests/assets/a.ape"); -test_read!(read_flac, "tests/assets/a.flac"); -test_read!(read_m4a, "tests/assets/a.m4a"); -test_read!(read_mp3, "tests/assets/a.mp3"); -test_read!(read_vorbis, "tests/assets/a.ogg"); -test_read!(read_opus, "tests/assets/a.opus"); -test_read!(read_riff, "tests/assets/a-id3.wav"); - -fn bench_ext(c: &mut Criterion) { - let mut g = c.benchmark_group("From extension"); - g.bench_function("APE", |b| b.iter(read_ape)); - g.bench_function("FLAC", |b| b.iter(read_flac)); - g.bench_function("MP4", |b| b.iter(read_m4a)); - g.bench_function("MP3", |b| b.iter(read_mp3)); - g.bench_function("VORBIS", |b| b.iter(read_vorbis)); - g.bench_function("OPUS", |b| b.iter(read_opus)); - g.bench_function("RIFF", |b| b.iter(read_riff)); -} - -criterion_group!(benches, bench_ext); -criterion_main!(benches); diff --git a/src/lib.rs b/src/lib.rs index e90e6ea8..47ee40ea 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -121,7 +121,9 @@ clippy::let_underscore_drop, clippy::match_wildcard_for_single_variants, clippy::semicolon_if_nothing_returned, - clippy::used_underscore_binding + clippy::used_underscore_binding, + clippy::new_without_default, + clippy::unused_self )] pub use crate::error::{LoftyError, Result}; diff --git a/src/logic/ape/mod.rs b/src/logic/ape/mod.rs index 6d4e927b..1079b118 100644 --- a/src/logic/ape/mod.rs +++ b/src/logic/ape/mod.rs @@ -5,7 +5,7 @@ pub(crate) mod tag; pub(crate) mod write; use crate::types::file::AudioFile; -use crate::{FileProperties, FileType, Result, Tag, TagType, TaggedFile}; +use crate::{FileProperties, Result, Tag, TagType}; use std::io::{Read, Seek}; diff --git a/src/logic/ape/read.rs b/src/logic/ape/read.rs index 52c79206..62f3f60b 100644 --- a/src/logic/ape/read.rs +++ b/src/logic/ape/read.rs @@ -5,6 +5,7 @@ use crate::files::ApeFile; use crate::logic::id3::find_lyrics3v2; use crate::logic::id3::v1::find_id3v1; use crate::logic::id3::v2::find_id3v2; +use crate::logic::id3::v2::read::parse_id3v2; use crate::{FileProperties, LoftyError, Result}; use std::io::{Read, Seek, SeekFrom}; @@ -51,6 +52,14 @@ where // ID3v2 tags are unsupported in APE files, but still possible if let Some(id3v2) = find_id3v2(data, true)? { stream_len -= id3v2.len() as u64; + + let id3v2 = parse_id3v2(&mut &*id3v2)?; + + // Skip over the footer + if id3v2.flags().footer { + data.seek(SeekFrom::Current(10))?; + } + ape_file.id3v2 = Some(id3v2) } diff --git a/src/logic/iff/aiff.rs b/src/logic/iff/aiff.rs index d0ad523a..e271acd8 100644 --- a/src/logic/iff/aiff.rs +++ b/src/logic/iff/aiff.rs @@ -1,6 +1,6 @@ use crate::logic::id3::v2::read::parse_id3v2; use crate::types::file::AudioFile; -use crate::{FileProperties, FileType, LoftyError, Result, TagType, TaggedFile}; +use crate::{FileProperties, LoftyError, Result, TagType}; use crate::{ItemKey, ItemValue, Tag, TagItem}; use std::io::{Read, Seek, SeekFrom}; diff --git a/src/logic/mpeg/mod.rs b/src/logic/mpeg/mod.rs index d2f031b7..b99ed319 100644 --- a/src/logic/mpeg/mod.rs +++ b/src/logic/mpeg/mod.rs @@ -3,7 +3,7 @@ pub(crate) mod header; pub(crate) mod read; use crate::types::file::AudioFile; -use crate::{FileProperties, FileType, Result, Tag, TagType, TaggedFile}; +use crate::{FileProperties, Result, Tag, TagType}; use std::io::{Read, Seek}; diff --git a/src/logic/ogg/flac.rs b/src/logic/ogg/flac.rs index 2cf469e0..229e11a0 100644 --- a/src/logic/ogg/flac.rs +++ b/src/logic/ogg/flac.rs @@ -3,7 +3,6 @@ use super::write::create_comments; use crate::error::{LoftyError, Result}; use crate::picture::Picture; use crate::types::file::AudioFile; -use crate::types::file::{FileType, TaggedFile}; use crate::types::properties::FileProperties; use crate::types::tag::{Tag, TagType}; diff --git a/src/logic/ogg/opus.rs b/src/logic/ogg/opus.rs index af0a2f09..3d0b5c69 100644 --- a/src/logic/ogg/opus.rs +++ b/src/logic/ogg/opus.rs @@ -2,7 +2,6 @@ use super::find_last_page; use crate::error::{LoftyError, Result}; use crate::logic::ogg::constants::{OPUSHEAD, OPUSTAGS}; use crate::types::file::AudioFile; -use crate::types::file::{FileType, TaggedFile}; use crate::types::properties::FileProperties; use crate::types::tag::{Tag, TagType}; diff --git a/src/logic/ogg/vorbis.rs b/src/logic/ogg/vorbis.rs index 470e76a9..cfb5ffa7 100644 --- a/src/logic/ogg/vorbis.rs +++ b/src/logic/ogg/vorbis.rs @@ -2,7 +2,6 @@ use super::find_last_page; use crate::error::{LoftyError, Result}; use crate::logic::ogg::constants::{VORBIS_COMMENT_HEAD, VORBIS_IDENT_HEAD, VORBIS_SETUP_HEAD}; use crate::types::file::AudioFile; -use crate::types::file::{FileType, TaggedFile}; use crate::types::properties::FileProperties; use crate::types::tag::{Tag, TagType}; diff --git a/src/probe.rs b/src/probe.rs index 9a473abc..f9a3ed20 100644 --- a/src/probe.rs +++ b/src/probe.rs @@ -1,4 +1,10 @@ -use crate::files::*; +use crate::logic::iff::aiff::AiffFile; +use crate::logic::ape::ApeFile; +use crate::logic::ogg::flac::FlacFile; +use crate::logic::mpeg::MpegFile; +use crate::logic::ogg::opus::OpusFile; +use crate::logic::ogg::vorbis::VorbisFile; +use crate::logic::iff::wav::WavFile; use crate::types::file::AudioFile; use crate::{FileType, LoftyError, Result, TaggedFile}; diff --git a/src/types/picture.rs b/src/types/picture.rs index 048c6f97..aafd8045 100644 --- a/src/types/picture.rs +++ b/src/types/picture.rs @@ -53,7 +53,22 @@ pub enum MimeType { None, } +impl ToString for MimeType { + fn to_string(&self) -> String { + match self { + MimeType::Jpeg => "image/jpeg".to_string(), + MimeType::Png => "image/png".to_string(), + MimeType::Tiff => "image/tiff".to_string(), + MimeType::Bmp => "image/bmp".to_string(), + MimeType::Gif => "image/gif".to_string(), + MimeType::Unknown(unknown) => unknown.clone(), + MimeType::None => String::new(), + } + } +} + impl MimeType { + #[allow(clippy::should_implement_trait)] /// Get a MimeType from a string pub fn from_str(mime_type: &str) -> Self { match &*mime_type.to_lowercase() { @@ -79,19 +94,6 @@ impl MimeType { MimeType::None => "", } } - - /// Get a String from a MimeType - pub fn to_string(&self) -> String { - match self { - MimeType::Jpeg => "image/jpeg".to_string(), - MimeType::Png => "image/png".to_string(), - MimeType::Tiff => "image/tiff".to_string(), - MimeType::Bmp => "image/bmp".to_string(), - MimeType::Gif => "image/gif".to_string(), - MimeType::Unknown(unknown) => unknown.to_owned(), - MimeType::None => String::new(), - } - } } /// The picture type @@ -150,7 +152,7 @@ impl PictureType { Self::Illustration => 18, Self::BandLogo => 19, Self::PublisherLogo => 20, - Self::Undefined(i) => u8::from(i.to_owned()), + Self::Undefined(i) => *i, } } @@ -294,6 +296,7 @@ impl Picture { } #[cfg(feature = "id3v2")] + #[allow(clippy::single_match_else)] /// Convert a [`Picture`] to a ID3v2 A/PIC byte Vec /// /// NOTE: This does not include the frame header @@ -320,8 +323,8 @@ impl Picture { let mut data = vec![self.text_encoding as u8]; - data.write_all(format.as_bytes()); - data.write_u8(self.pic_type.as_u8()); + data.write_all(format.as_bytes())?; + data.write_u8(self.pic_type.as_u8())?; if let Some(description) = &self.description { data.write_all(&*crate::logic::id3::v2::util::text_utils::encode_text( @@ -332,11 +335,11 @@ impl Picture { } data.write_u8(0)?; - data.write_all(&*self.data); + data.write_all(&*self.data)?; let size = data.len() - 6; - if size as u64 > u32::MAX as u64 { + if size as u64 > u64::from(u32::MAX) { return Err(LoftyError::TooMuchData); } @@ -367,7 +370,7 @@ impl Picture { let size = data.len(); - if size as u64 > u32::MAX as u64 { + if size as u64 > u64::from(u32::MAX) { return Err(LoftyError::TooMuchData); } @@ -377,6 +380,7 @@ impl Picture { } #[cfg(feature = "id3v2")] + #[allow(clippy::single_match_else)] /// Get a [`Picture`] from ID3v2 A/PIC bytes: /// /// NOTE: This expects the frame header to have already been skipped @@ -409,7 +413,7 @@ impl Picture { encoding, true, )? - .map(|s| Cow::from(s)); + .map(Cow::from); let mut data = Vec::new(); cursor.read_to_end(&mut data)?; @@ -429,16 +433,12 @@ impl Picture { }) }, _ => { - let mime_type = if let Some(mime_type) = - crate::logic::id3::v2::util::text_utils::decode_text( - &mut cursor, - encoding, - true, - )? { - MimeType::from_str(&*mime_type) - } else { - MimeType::None - }; + let mime_type = (crate::logic::id3::v2::util::text_utils::decode_text( + &mut cursor, + encoding, + true, + )?) + .map_or(MimeType::None, |mime_type| MimeType::from_str(&*mime_type)); let picture_type = PictureType::from_u8(cursor.read_u8()?); let description = crate::logic::id3::v2::util::text_utils::decode_text( @@ -446,7 +446,7 @@ impl Picture { encoding, true, )? - .map(|s| Cow::from(s)); + .map(Cow::from); let mut data = Vec::new(); cursor.read_to_end(&mut data)?; @@ -468,7 +468,7 @@ impl Picture { }; } - return Err(LoftyError::NotAPicture); + Err(LoftyError::NotAPicture) } #[cfg(feature = "vorbis_comments")] @@ -481,7 +481,7 @@ impl Picture { pub fn as_flac_bytes(&self) -> String { let mut data = Vec::::new(); - let picture_type = (self.pic_type.as_u8() as u32).to_be_bytes(); + let picture_type = u32::from(self.pic_type.as_u8()).to_be_bytes(); let mime_str = self.mime_type.to_string(); let mime_len = mime_str.len() as u32; diff --git a/src/types/tag.rs b/src/types/tag.rs index 58957b6c..b247c82a 100644 --- a/src/types/tag.rs +++ b/src/types/tag.rs @@ -235,7 +235,7 @@ impl Tag { /// Retain tag items based on the predicate /// /// See [`Vec::retain`](std::vec::Vec::retain) - pub fn retain(&mut self, mut f: F) + pub fn retain(&mut self, f: F) where F: FnMut(&TagItem) -> bool, {