From c72857c3d7d8a220f2782af7f393e52c31528da8 Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Sun, 28 Nov 2021 12:55:11 -0500 Subject: [PATCH] Add tag conversion tests --- src/logic/ape/tag/item.rs | 10 ++ src/logic/ape/tag/mod.rs | 65 +++++----- src/logic/ape/tag/read.rs | 2 +- src/logic/ape/tag/write.rs | 27 +++-- src/logic/id3/v1/constants.rs | 4 +- src/logic/id3/v1/tag.rs | 4 +- src/logic/iff/wav/tag/mod.rs | 7 ++ src/logic/mp4/ilst/mod.rs | 8 ++ src/types/item.rs | 3 +- src/types/tag.rs | 5 - tests/tags/conversions.rs | 219 ++++++++++++++++++++++++++++++++++ tests/tags/main.rs | 8 ++ tests/tags/read.rs | 27 ++--- 13 files changed, 319 insertions(+), 70 deletions(-) create mode 100644 tests/tags/conversions.rs diff --git a/src/logic/ape/tag/item.rs b/src/logic/ape/tag/item.rs index 3f79480e..2f9f73e3 100644 --- a/src/logic/ape/tag/item.rs +++ b/src/logic/ape/tag/item.rs @@ -32,6 +32,14 @@ impl ApeItem { pub fn set_read_only(&mut self) { self.read_only = true } + + pub fn key(&self) -> &str { + &self.key + } + + pub fn value(&self) -> &ItemValue { + &self.value + } } impl TryFrom for ApeItem { @@ -53,6 +61,7 @@ impl TryFrom for ApeItem { pub(in crate::logic) struct ApeItemRef<'a> { pub read_only: bool, + pub key: &'a str, pub value: ItemValueRef<'a>, } @@ -60,6 +69,7 @@ impl<'a> Into> for &'a ApeItem { fn into(self) -> ApeItemRef<'a> { ApeItemRef { read_only: self.read_only, + key: self.key(), value: (&self.value).into(), } } diff --git a/src/logic/ape/tag/mod.rs b/src/logic/ape/tag/mod.rs index 3d8c9a6b..6d23788e 100644 --- a/src/logic/ape/tag/mod.rs +++ b/src/logic/ape/tag/mod.rs @@ -7,7 +7,6 @@ use crate::logic::ape::tag::item::{ApeItem, ApeItemRef}; use crate::types::item::{ItemKey, ItemValue, TagItem}; use crate::types::tag::{Tag, TagType}; -use std::collections::HashMap; use std::convert::TryInto; use std::fs::File; use std::io::{Read, Seek}; @@ -16,20 +15,30 @@ use std::io::{Read, Seek}; /// An APE tag pub struct ApeTag { pub read_only: bool, - pub(super) items: HashMap, + pub(super) items: Vec, } impl ApeTag { pub fn get_key(&self, key: &str) -> Option<&ApeItem> { - self.items.get(key) + self.items + .iter() + .find(|i| i.key().eq_ignore_ascii_case(key)) } - pub fn push_item(&mut self, value: ApeItem) { - let _ = self.items.insert(value.key.clone(), value); + pub fn insert(&mut self, value: ApeItem) { + self.remove_key(value.key()); + self.items.push(value); } pub fn remove_key(&mut self, key: &str) { - let _ = self.items.remove(key); + self.items + .iter() + .position(|i| i.key() == key) + .map(|p| self.items.remove(p)); + } + + pub fn items(&self) -> &[ApeItem] { + &self.items } } @@ -50,7 +59,7 @@ impl From for Tag { fn from(input: ApeTag) -> Self { let mut tag = Tag::new(TagType::Ape); - for (_, item) in input.items { + for item in input.items { let item = TagItem::new(ItemKey::from_key(&TagType::Ape, &*item.key), item.value); tag.insert_item_unchecked(item) @@ -66,7 +75,7 @@ impl From for ApeTag { for item in input.items { if let Ok(ape_item) = item.try_into() { - ape_tag.push_item(ape_item) + ape_tag.insert(ape_item) } } @@ -75,7 +84,7 @@ impl From for ApeTag { if let Ok(item) = ApeItem::new(key.to_string(), ItemValue::Binary(pic.as_ape_bytes())) { - ape_tag.push_item(item) + ape_tag.insert(item) } } } @@ -86,34 +95,28 @@ impl From for ApeTag { pub(in crate::logic) struct ApeTagRef<'a> { read_only: bool, - pub(super) items: HashMap<&'a str, ApeItemRef<'a>>, + pub(super) items: Box> + 'a>, } impl<'a> ApeTagRef<'a> { - pub(crate) fn write_to(&self, file: &mut File) -> Result<()> { + pub(crate) fn write_to(&mut self, file: &mut File) -> Result<()> { write::write_to(file, self) } } impl<'a> Into> for &'a Tag { fn into(self) -> ApeTagRef<'a> { - let mut items = HashMap::<&'a str, ApeItemRef<'a>>::new(); - - for item in &self.items { - let key = item.key().map_key(&TagType::Ape, true).unwrap(); - - items.insert( - key, - ApeItemRef { - read_only: false, - value: (&item.item_value).into(), - }, - ); - } - ApeTagRef { read_only: false, - items, + items: Box::new(self.items.iter().filter_map(|i| { + i.key().map_key(&TagType::Ape, true).map_or(None, |key| { + Some(ApeItemRef { + read_only: false, + key, + value: (&i.item_value).into(), + }) + }) + })), } } } @@ -122,15 +125,7 @@ impl<'a> Into> for &'a ApeTag { fn into(self) -> ApeTagRef<'a> { ApeTagRef { read_only: self.read_only, - items: { - let mut items = HashMap::<&str, ApeItemRef<'a>>::new(); - - for (k, v) in &self.items { - items.insert(k.as_str(), v.into()); - } - - items - }, + items: Box::new(self.items.iter().map(|i| i.into())), } } } diff --git a/src/logic/ape/tag/read.rs b/src/logic/ape/tag/read.rs index f5c652a1..2b3fb0a4 100644 --- a/src/logic/ape/tag/read.rs +++ b/src/logic/ape/tag/read.rs @@ -88,7 +88,7 @@ where item.set_read_only() } - tag.push_item(item); + tag.insert(item); } // Version 1 doesn't include a header diff --git a/src/logic/ape/tag/write.rs b/src/logic/ape/tag/write.rs index 3fc4af1c..af9d2040 100644 --- a/src/logic/ape/tag/write.rs +++ b/src/logic/ape/tag/write.rs @@ -13,7 +13,7 @@ use std::io::{Cursor, Read, Seek, SeekFrom, Write}; use byteorder::{LittleEndian, WriteBytesExt}; -pub(in crate::logic) fn write_to(data: &mut File, tag: &ApeTagRef) -> Result<()> { +pub(in crate::logic) fn write_to(data: &mut File, tag: &mut ApeTagRef) -> Result<()> { match Probe::new().file_type(data) { Some(ft) if ft == FileType::APE || ft == FileType::MP3 => {}, _ => return Err(LoftyError::UnsupportedTag), @@ -39,7 +39,7 @@ pub(in crate::logic) fn write_to(data: &mut File, tag: &ApeTagRef) -> Result<()> let (mut existing, size) = read_ape_tag(data, false)?; // Only keep metadata around that's marked read only - existing.items.retain(|_i, v| v.read_only); + existing.items.retain(|i| i.read_only); if !existing.items.is_empty() { read_only = Some(existing) @@ -70,7 +70,7 @@ pub(in crate::logic) fn write_to(data: &mut File, tag: &ApeTagRef) -> Result<()> let (mut existing, size) = read_ape_tag(data, true)?; - existing.items.retain(|_, v| v.read_only); + existing.items.retain(|i| i.read_only); if !existing.items.is_empty() { read_only = Some(existing) @@ -86,7 +86,7 @@ pub(in crate::logic) fn write_to(data: &mut File, tag: &ApeTagRef) -> Result<()> // Preserve any metadata marked as read only let tag = if let Some(read_only) = read_only { - create_ape_tag(&Into::::into(&read_only))? + create_ape_tag(&mut Into::::into(&read_only))? } else { create_ape_tag(tag)? }; @@ -115,17 +115,20 @@ pub(in crate::logic) fn write_to(data: &mut File, tag: &ApeTagRef) -> Result<()> Ok(()) } -fn create_ape_tag(tag: &ApeTagRef) -> Result> { +fn create_ape_tag(tag: &mut ApeTagRef) -> Result> { + let items = &mut tag.items; + let mut peek = items.peekable(); + // Unnecessary to write anything if there's no metadata - if tag.items.is_empty() { + if peek.peek().is_none() { Ok(Vec::::new()) } else { let mut tag_write = Cursor::new(Vec::::new()); - let item_count = tag.items.len() as u32; + let mut item_count = 0_u32; - for (k, v) in &tag.items { - let (mut flags, value) = match v.value { + for item in peek { + let (mut flags, value) = match item.value { ItemValueRef::Binary(value) => { tag_write.write_u32::(value.len() as u32)?; @@ -143,14 +146,16 @@ fn create_ape_tag(tag: &ApeTagRef) -> Result> { }, }; - if v.read_only { + if item.read_only { flags |= 1_u32 } tag_write.write_u32::(flags)?; - tag_write.write_all(k.as_bytes())?; + tag_write.write_all(item.key.as_bytes())?; tag_write.write_u8(0)?; tag_write.write_all(value)?; + + item_count += 1; } let size = tag_write.get_ref().len(); diff --git a/src/logic/id3/v1/constants.rs b/src/logic/id3/v1/constants.rs index 8af04276..b6dc5193 100644 --- a/src/logic/id3/v1/constants.rs +++ b/src/logic/id3/v1/constants.rs @@ -196,10 +196,12 @@ pub const GENRES: [&str; 192] = [ "Psybient", ]; -pub const VALID_ITEMKEYS: [ItemKey; 5] = [ +pub const VALID_ITEMKEYS: [ItemKey; 7] = [ ItemKey::TrackTitle, ItemKey::TrackArtist, ItemKey::AlbumTitle, ItemKey::Year, ItemKey::Comment, + ItemKey::TrackNumber, + ItemKey::Genre, ]; diff --git a/src/logic/id3/v1/tag.rs b/src/logic/id3/v1/tag.rs index 71bb1abe..3d10c2c1 100644 --- a/src/logic/id3/v1/tag.rs +++ b/src/logic/id3/v1/tag.rs @@ -106,7 +106,7 @@ impl From for Id3v1Tag { year: input.get_string(&ItemKey::Year).map(str::to_owned), comment: input.get_string(&ItemKey::Comment).map(str::to_owned), track_number: input - .get_string(&ItemKey::Genre) + .get_string(&ItemKey::TrackNumber) .map(|g| g.parse::().ok()) .and_then(|g| g), genre: input @@ -155,7 +155,7 @@ impl<'a> Into> for &'a Tag { year: self.get_string(&ItemKey::Year), comment: self.get_string(&ItemKey::Comment), track_number: self - .get_string(&ItemKey::Genre) + .get_string(&ItemKey::TrackNumber) .map(|g| g.parse::().ok()) .and_then(|g| g), genre: self diff --git a/src/logic/iff/wav/tag/mod.rs b/src/logic/iff/wav/tag/mod.rs index 9a7de86c..ba4a9b9a 100644 --- a/src/logic/iff/wav/tag/mod.rs +++ b/src/logic/iff/wav/tag/mod.rs @@ -16,6 +16,13 @@ pub struct RiffInfoList { } impl RiffInfoList { + pub fn get(&self, key: &str) -> Option<&str> { + self.items + .iter() + .find(|(k, _)| k == key) + .map(|(_, v)| v.as_str()) + } + pub fn insert(&mut self, key: String, value: String) { if valid_key(key.as_str()) { self.items diff --git a/src/logic/mp4/ilst/mod.rs b/src/logic/mp4/ilst/mod.rs index eb4053f3..7a6d06b5 100644 --- a/src/logic/mp4/ilst/mod.rs +++ b/src/logic/mp4/ilst/mod.rs @@ -119,6 +119,14 @@ impl Atom { pub fn new(ident: AtomIdent, data: AtomData) -> Self { Self { ident, data } } + + pub fn ident(&self) -> &AtomIdent { + &self.ident + } + + pub fn data(&self) -> &AtomData { + &self.data + } } #[derive(Eq, PartialEq, Debug)] diff --git a/src/types/item.rs b/src/types/item.rs index bf2443c7..3e01de6f 100644 --- a/src/types/item.rs +++ b/src/types/item.rs @@ -342,7 +342,8 @@ item_keys!( // Style Genre => [ TagType::Id3v2 => "TCON", TagType::Mp4Atom => "\u{a9}gen", - TagType::VorbisComments => "GENRE", TagType::RiffInfo => "IGNR" + TagType::VorbisComments => "GENRE", TagType::RiffInfo => "IGNR", + TagType::Ape => "Genre" ], InitialKey => [ TagType::Id3v2 => "TKEY" diff --git a/src/types/tag.rs b/src/types/tag.rs index 2604ffcb..652c7e05 100644 --- a/src/types/tag.rs +++ b/src/types/tag.rs @@ -156,11 +156,6 @@ impl Tag { /// Insert a [`TagItem`], replacing any existing one of the same type /// /// NOTE: This **will** verify an [`ItemKey`] mapping exists for the target [`TagType`] - /// - /// # Warning - /// - /// When dealing with ID3v2, it may be necessary to use [`insert_item_unchecked`](Tag::insert_item_unchecked). - /// See [`id3`](crate::id3::v2) for an explanation. pub fn insert_item(&mut self, item: TagItem) -> bool { if item.re_map(&self.tag_type).is_some() { self.insert_item_unchecked(item); diff --git a/tests/tags/conversions.rs b/tests/tags/conversions.rs new file mode 100644 index 00000000..e46a67cb --- /dev/null +++ b/tests/tags/conversions.rs @@ -0,0 +1,219 @@ +use crate::{APE, ID3V1, ID3V2, ILST, RIFF_INFO, VORBIS_COMMENTS}; + +use lofty::ape::ApeTag; +use lofty::id3::v1::Id3v1Tag; +use lofty::id3::v2::{FrameValue, Id3v2Tag, LanguageFrame, TextEncoding}; +use lofty::iff::RiffInfoList; +use lofty::mp4::{AtomData, AtomIdent, Ilst}; +use lofty::ogg::VorbisComments; +use lofty::{ItemKey, ItemValue, Tag, TagType}; + +fn create_tag(tag_type: TagType) -> Tag { + let mut tag = Tag::new(tag_type); + + tag.insert_text(ItemKey::TrackTitle, String::from("Foo title")); + tag.insert_text(ItemKey::TrackArtist, String::from("Bar artist")); + tag.insert_text(ItemKey::AlbumTitle, String::from("Baz album")); + tag.insert_text(ItemKey::Comment, String::from("Qux comment")); + tag.insert_text(ItemKey::TrackNumber, String::from("1")); + tag.insert_text(ItemKey::Genre, String::from("Classical")); + + tag +} + +fn verify_tag(tag: &Tag, track_number: bool, genre: bool) { + assert_eq!(tag.get_string(&ItemKey::TrackTitle), Some("Foo title")); + assert_eq!(tag.get_string(&ItemKey::TrackArtist), Some("Bar artist")); + assert_eq!(tag.get_string(&ItemKey::AlbumTitle), Some("Baz album")); + assert_eq!(tag.get_string(&ItemKey::Comment), Some("Qux comment")); + + if track_number { + assert_eq!(tag.get_string(&ItemKey::TrackNumber), Some("1")); + } + + if genre { + assert_eq!(tag.get_string(&ItemKey::Genre), Some("Classical")); + } +} + +#[test] +fn ape_to_tag() { + let ape = ApeTag::read_from(&mut std::io::Cursor::new(&APE[..])).unwrap(); + + let tag: Tag = ape.into(); + + verify_tag(&tag, true, true); +} + +#[test] +fn tag_to_ape() { + fn verify_key(tag: &ApeTag, key: &str, expected_val: &str) { + assert_eq!( + tag.get_key(key).map(|i| i.value()), + Some(&ItemValue::Text(String::from(expected_val))) + ); + } + + let tag = create_tag(TagType::Ape); + + let ape_tag: ApeTag = tag.into(); + + verify_key(&ape_tag, "Title", "Foo title"); + verify_key(&ape_tag, "Artist", "Bar artist"); + verify_key(&ape_tag, "Album", "Baz album"); + verify_key(&ape_tag, "Comment", "Qux comment"); + verify_key(&ape_tag, "Track", "1"); + verify_key(&ape_tag, "Genre", "Classical"); +} + +#[test] +fn id3v1_to_tag() { + let id3v1 = Id3v1Tag::read_from(ID3V1); + + let tag: Tag = id3v1.into(); + + verify_tag(&tag, true, true); +} + +#[test] +fn tag_to_id3v1() { + let tag = create_tag(TagType::Id3v1); + + let id3v1_tag: Id3v1Tag = tag.into(); + + assert_eq!(id3v1_tag.title.as_deref(), Some("Foo title")); + assert_eq!(id3v1_tag.artist.as_deref(), Some("Bar artist")); + assert_eq!(id3v1_tag.album.as_deref(), Some("Baz album")); + assert_eq!(id3v1_tag.comment.as_deref(), Some("Qux comment")); + assert_eq!(id3v1_tag.track_number, Some(1)); + assert_eq!(id3v1_tag.genre, Some(32)); +} + +#[test] +fn id3v2_to_tag() { + let id3v2 = Id3v2Tag::read_from(&mut &ID3V2[..]).unwrap(); + + let tag: Tag = id3v2.into(); + + verify_tag(&tag, true, true); +} + +#[test] +fn tag_to_id3v2() { + fn verify_frame(tag: &Id3v2Tag, id: &str, value: &str) { + let frame = tag.get(id); + + assert!(frame.is_some()); + + let frame = frame.unwrap(); + + assert_eq!( + frame.content(), + &FrameValue::Text { + encoding: TextEncoding::UTF8, + value: String::from(value) + } + ); + } + + let tag = create_tag(TagType::Id3v2); + + let id3v2_tag: Id3v2Tag = tag.into(); + + verify_frame(&id3v2_tag, "TIT2", "Foo title"); + verify_frame(&id3v2_tag, "TPE1", "Bar artist"); + verify_frame(&id3v2_tag, "TALB", "Baz album"); + + let frame = id3v2_tag.get("COMM").unwrap(); + assert_eq!( + frame.content(), + &FrameValue::Comment(LanguageFrame { + encoding: TextEncoding::Latin1, + language: String::from("eng"), + description: String::new(), + content: String::from("Qux comment") + }) + ); + + verify_frame(&id3v2_tag, "TRCK", "1"); + verify_frame(&id3v2_tag, "TCON", "Classical"); +} + +#[test] +fn ilst_to_tag() { + let ilst = Ilst::read_from(&mut &ILST[..], (ILST.len() - 1) as u64).unwrap(); + + let tag: Tag = ilst.into(); + + verify_tag(&tag, false, true); +} + +#[test] +fn tag_to_ilst() { + fn verify_atom(ilst: &Ilst, ident: [u8; 4], data: &str) { + let atom = ilst.atom(&AtomIdent::Fourcc(ident)).unwrap(); + + let data = AtomData::UTF8(String::from(data)); + + assert_eq!(atom.data(), &data); + } + + let tag = create_tag(TagType::Mp4Atom); + + let ilst: Ilst = tag.into(); + + verify_atom(&ilst, *b"\xa9nam", "Foo title"); + verify_atom(&ilst, *b"\xa9ART", "Bar artist"); + verify_atom(&ilst, *b"\xa9alb", "Baz album"); + verify_atom(&ilst, *b"\xa9cmt", "Qux comment"); + verify_atom(&ilst, *b"\xa9gen", "Classical"); +} + +#[test] +fn riff_info_to_tag() { + let riff_info = RiffInfoList::read_from( + &mut std::io::Cursor::new(&RIFF_INFO), + (RIFF_INFO.len() - 1) as u64, + ) + .unwrap(); + + let tag: Tag = riff_info.into(); + + verify_tag(&tag, true, false); +} + +#[test] +fn tag_to_riff_info() { + let tag = create_tag(TagType::RiffInfo); + + let riff_info: RiffInfoList = tag.into(); + + assert_eq!(riff_info.get("INAM"), Some("Foo title")); + assert_eq!(riff_info.get("IART"), Some("Bar artist")); + assert_eq!(riff_info.get("IPRD"), Some("Baz album")); + assert_eq!(riff_info.get("ICMT"), Some("Qux comment")); + assert_eq!(riff_info.get("IPRT"), Some("1")); +} + +#[test] +fn vorbis_comments_to_tag() { + let vorbis_comments = VorbisComments::read_from(&mut &VORBIS_COMMENTS[..]).unwrap(); + + let tag: Tag = vorbis_comments.into(); + + verify_tag(&tag, true, true); +} + +#[test] +fn tag_to_vorbis_comments() { + let tag = create_tag(TagType::VorbisComments); + + let vorbis_comments: VorbisComments = tag.into(); + + assert_eq!(vorbis_comments.get_item("TITLE"), Some("Foo title")); + assert_eq!(vorbis_comments.get_item("ARTIST"), Some("Bar artist")); + assert_eq!(vorbis_comments.get_item("ALBUM"), Some("Baz album")); + assert_eq!(vorbis_comments.get_item("COMMENT"), Some("Qux comment")); + assert_eq!(vorbis_comments.get_item("TRACKNUMBER"), Some("1")); + assert_eq!(vorbis_comments.get_item("GENRE"), Some("Classical")); +} diff --git a/tests/tags/main.rs b/tests/tags/main.rs index 0bec210a..907a3fba 100644 --- a/tests/tags/main.rs +++ b/tests/tags/main.rs @@ -1 +1,9 @@ +mod conversions; mod read; + +const APE: [u8; 209] = *include_bytes!("assets/test.apev2"); +const ID3V1: [u8; 128] = *include_bytes!("assets/test.id3v1"); +const ID3V2: [u8; 1168] = *include_bytes!("assets/test.id3v2"); +const ILST: [u8; 1024] = *include_bytes!("assets/test.ilst"); +const RIFF_INFO: [u8; 100] = *include_bytes!("assets/test.riff"); +const VORBIS_COMMENTS: [u8; 152] = *include_bytes!("assets/test.vorbis"); diff --git a/tests/tags/read.rs b/tests/tags/read.rs index 7290d6ca..0502d2d0 100644 --- a/tests/tags/read.rs +++ b/tests/tags/read.rs @@ -6,12 +6,7 @@ use lofty::mp4::{Atom, AtomData, AtomIdent, Ilst}; use lofty::ogg::VorbisComments; use lofty::ItemValue; -const APE: [u8; 209] = *include_bytes!("assets/test.apev2"); -const ID3V1: [u8; 128] = *include_bytes!("assets/test.id3v1"); -const ID3V2: [u8; 1168] = *include_bytes!("assets/test.id3v2"); -const ILST: [u8; 1024] = *include_bytes!("assets/test.ilst"); -const RIFF_INFO: [u8; 100] = *include_bytes!("assets/test.riff"); -const VORBIS_COMMENTS: [u8; 152] = *include_bytes!("assets/test.vorbis"); +use crate::{APE, ID3V1, ID3V2, ILST, RIFF_INFO, VORBIS_COMMENTS}; #[test] fn read_ape() { @@ -53,17 +48,21 @@ fn read_ape() { ) .unwrap(); - expected_tag.push_item(title_item); - expected_tag.push_item(artist_item); - expected_tag.push_item(album_item); - expected_tag.push_item(comment_item); - expected_tag.push_item(year_item); - expected_tag.push_item(track_number_item); - expected_tag.push_item(genre_item); + expected_tag.insert(title_item); + expected_tag.insert(artist_item); + expected_tag.insert(album_item); + expected_tag.insert(comment_item); + expected_tag.insert(year_item); + expected_tag.insert(track_number_item); + expected_tag.insert(genre_item); let parsed_tag = ApeTag::read_from(&mut std::io::Cursor::new(APE)).unwrap(); - assert_eq!(expected_tag, parsed_tag); + assert_eq!(expected_tag.items().len(), parsed_tag.items().len()); + + for item in expected_tag.items() { + assert!(parsed_tag.items().contains(item)) + } } #[test]