From d1bc6d436b7b58d5af6a29843911ef624657447f Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 4 Jan 2023 20:45:47 +0100 Subject: [PATCH] Allow to create AtomIdent constants with 'static lifetime --- src/mp4/atom_info.rs | 50 +++++++++++++++++++++++++++++++++++++------ src/mp4/ilst/atom.rs | 32 ++++++++++++++++----------- src/mp4/ilst/mod.rs | 46 ++++++++++++++++++++------------------- src/mp4/ilst/ref.rs | 32 +++------------------------ src/mp4/ilst/write.rs | 6 +++--- 5 files changed, 94 insertions(+), 72 deletions(-) diff --git a/src/mp4/atom_info.rs b/src/mp4/atom_info.rs index cff39ab8..9194d5d2 100644 --- a/src/mp4/atom_info.rs +++ b/src/mp4/atom_info.rs @@ -1,13 +1,14 @@ use crate::error::{ErrorKind, LoftyError, Result}; use crate::macros::{err, try_vec}; +use std::borrow::Cow; use std::io::{Read, Seek, SeekFrom}; use byteorder::{BigEndian, ReadBytesExt}; /// Represents an `MP4` atom identifier #[derive(Eq, PartialEq, Debug, Clone)] -pub enum AtomIdent { +pub enum AtomIdent<'a> { /// A four byte identifier /// /// Many FOURCCs start with `0xA9` (©), and should be human-readable. @@ -25,17 +26,39 @@ pub enum AtomIdent { /// ``` Freeform { /// A string using a reverse DNS naming convention - mean: String, + mean: Cow<'a, str>, /// A string identifying the atom - name: String, + name: Cow<'a, str>, }, } +impl<'a> AtomIdent<'a> { + pub(crate) fn as_borrowed(&'a self) -> Self { + match self { + Self::Fourcc(fourcc) => AtomIdent::Fourcc(*fourcc), + Self::Freeform { mean, name } => AtomIdent::Freeform { + mean: Cow::Borrowed(&mean), + name: Cow::Borrowed(&name), + }, + } + } + + pub(crate) fn into_owned(self) -> AtomIdent<'static> { + match self { + Self::Fourcc(fourcc) => AtomIdent::Fourcc(fourcc), + Self::Freeform { mean, name } => AtomIdent::Freeform { + mean: Cow::Owned(mean.into_owned()), + name: Cow::Owned(name.into_owned()), + }, + } + } +} + pub(crate) struct AtomInfo { pub(crate) start: u64, pub(crate) len: u64, pub(crate) extended: bool, - pub(crate) ident: AtomIdent, + pub(crate) ident: AtomIdent<'static>, } impl AtomInfo { @@ -99,14 +122,17 @@ impl AtomInfo { } } -fn parse_freeform(data: &mut R, reader_size: u64) -> Result +fn parse_freeform(data: &mut R, reader_size: u64) -> Result> where R: Read + Seek, { let mean = freeform_chunk(data, b"mean", reader_size)?; let name = freeform_chunk(data, b"name", reader_size - 4)?; - Ok(AtomIdent::Freeform { mean, name }) + Ok(AtomIdent::Freeform { + mean: mean.into(), + name: name.into(), + }) } fn freeform_chunk(data: &mut R, name: &[u8], reader_size: u64) -> Result @@ -136,3 +162,15 @@ where )), } } + +#[cfg(test)] +mod tests { + use super::*; + + // Verify that we could create freeform AtomIdent constants + #[allow(dead_code)] + const FREEFORM_ATOM_IDENT: AtomIdent<'_> = AtomIdent::Freeform { + mean: Cow::Borrowed("mean"), + name: Cow::Borrowed("name"), + }; +} diff --git a/src/mp4/ilst/atom.rs b/src/mp4/ilst/atom.rs index b96d8bf5..eba30b59 100644 --- a/src/mp4/ilst/atom.rs +++ b/src/mp4/ilst/atom.rs @@ -80,14 +80,14 @@ impl<'a> Iterator for AtomDataStorageIter<'a> { /// Represents an `MP4` atom #[derive(PartialEq, Clone)] -pub struct Atom { - pub(crate) ident: AtomIdent, +pub struct Atom<'a> { + pub(crate) ident: AtomIdent<'a>, pub(super) data: AtomDataStorage, } -impl Atom { +impl<'a> Atom<'a> { /// Create a new [`Atom`] - pub fn new(ident: AtomIdent, data: AtomData) -> Self { + pub fn new(ident: AtomIdent<'a>, data: AtomData) -> Self { Self { ident, data: AtomDataStorage::Single(data), @@ -97,7 +97,7 @@ impl Atom { /// Create a new [`Atom`] from a collection of [`AtomData`]s /// /// This will return `None` if `data` is empty, as empty atoms are useless. - pub fn from_collection(ident: AtomIdent, mut data: Vec) -> Option { + pub fn from_collection(ident: AtomIdent<'a>, mut data: Vec) -> Option { let data = match data.len() { 0 => return None, 1 => AtomDataStorage::Single(data.swap_remove(0)), @@ -108,8 +108,8 @@ impl Atom { } /// Returns the atom's [`AtomIdent`] - pub fn ident(&self) -> &AtomIdent { - &self.ident + pub fn ident(&self) -> AtomIdent<'_> { + self.ident.as_borrowed() } /// Returns the atom's [`AtomData`] @@ -128,22 +128,30 @@ impl Atom { } // Used internally, has no correctness checks - pub(crate) fn unknown_implicit(ident: AtomIdent, data: Vec) -> Self { + pub(crate) fn unknown_implicit(ident: AtomIdent<'_>, data: Vec) -> Self { Self { - ident, + ident: ident.into_owned(), data: AtomDataStorage::Single(AtomData::Unknown { code: 0, data }), } } - pub(crate) fn text(ident: AtomIdent, data: String) -> Self { + pub(crate) fn text(ident: AtomIdent<'_>, data: String) -> Self { Self { - ident, + ident: ident.into_owned(), data: AtomDataStorage::Single(AtomData::UTF8(data)), } } + + pub(crate) fn into_owned(self) -> Atom<'static> { + let Self { ident, data } = self; + Atom { + ident: ident.into_owned(), + data, + } + } } -impl Debug for Atom { +impl Debug for Atom<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("Atom") .field("ident", &self.ident) diff --git a/src/mp4/ilst/mod.rs b/src/mp4/ilst/mod.rs index 1fb987ec..0fef16e7 100644 --- a/src/mp4/ilst/mod.rs +++ b/src/mp4/ilst/mod.rs @@ -12,7 +12,6 @@ use crate::tag::item::{ItemKey, ItemValue, TagItem}; use crate::tag::{Tag, TagType}; use crate::traits::{Accessor, TagExt}; use atom::{AdvisoryRating, Atom, AtomData}; -use r#ref::AtomIdentRef; use std::borrow::Cow; use std::fs::{File, OpenOptions}; @@ -21,11 +20,11 @@ use std::path::Path; use lofty_attr::tag; -const ARTIST: AtomIdent = AtomIdent::Fourcc(*b"\xa9ART"); -const TITLE: AtomIdent = AtomIdent::Fourcc(*b"\xa9nam"); -const ALBUM: AtomIdent = AtomIdent::Fourcc(*b"\xa9alb"); -const GENRE: AtomIdent = AtomIdent::Fourcc(*b"\xa9gen"); -const COMMENT: AtomIdent = AtomIdent::Fourcc(*b"\xa9cmt"); +const ARTIST: AtomIdent<'_> = AtomIdent::Fourcc(*b"\xa9ART"); +const TITLE: AtomIdent<'_> = AtomIdent::Fourcc(*b"\xa9nam"); +const ALBUM: AtomIdent<'_> = AtomIdent::Fourcc(*b"\xa9alb"); +const GENRE: AtomIdent<'_> = AtomIdent::Fourcc(*b"\xa9gen"); +const COMMENT: AtomIdent<'_> = AtomIdent::Fourcc(*b"\xa9cmt"); macro_rules! impl_accessor { ($($name:ident => $const:ident;)+) => { @@ -79,33 +78,33 @@ macro_rules! impl_accessor { #[derive(Default, PartialEq, Debug, Clone)] #[tag(description = "An MP4 ilst atom", supported_formats(MP4))] pub struct Ilst { - pub(crate) atoms: Vec, + pub(crate) atoms: Vec>, } impl Ilst { /// Returns all of the tag's atoms - pub fn atoms(&self) -> &[Atom] { + pub fn atoms(&self) -> &[Atom<'static>] { &self.atoms } /// Get an item by its [`AtomIdent`] - pub fn atom(&self, ident: &AtomIdent) -> Option<&Atom> { + pub fn atom(&self, ident: &AtomIdent<'_>) -> Option<&Atom<'static>> { self.atoms.iter().find(|a| &a.ident == ident) } /// Inserts an [`Atom`] - pub fn insert_atom(&mut self, atom: Atom) { - self.atoms.push(atom); + pub fn insert_atom(&mut self, atom: Atom<'_>) { + self.atoms.push(atom.into_owned()); } /// Inserts an [`Atom`], replacing any atom with the same [`AtomIdent`] - pub fn replace_atom(&mut self, atom: Atom) { + pub fn replace_atom(&mut self, atom: Atom<'_>) { self.remove_atom(&atom.ident); - self.atoms.push(atom); + self.atoms.push(atom.into_owned()); } /// Remove an atom by its [`AtomIdent`] - pub fn remove_atom(&mut self, ident: &AtomIdent) { + pub fn remove_atom(&mut self, ident: &AtomIdent<'_>) { self.atoms .iter() .position(|a| &a.ident == ident) @@ -117,14 +116,14 @@ impl Ilst { /// See [`Vec::retain`](std::vec::Vec::retain) pub fn retain(&mut self, f: F) where - F: FnMut(&Atom) -> bool, + F: FnMut(&Atom<'_>) -> bool, { self.atoms.retain(f) } /// Returns all pictures pub fn pictures(&self) -> impl Iterator { - const COVR: AtomIdent = AtomIdent::Fourcc(*b"covr"); + const COVR: AtomIdent<'_> = AtomIdent::Fourcc(*b"covr"); self.atoms.iter().filter_map(|a| match a.ident { COVR => { @@ -326,7 +325,7 @@ impl Accessor for Ilst { impl TagExt for Ilst { type Err = LoftyError; - type RefKey<'a> = &'a AtomIdent; + type RefKey<'a> = &'a AtomIdent<'a>; fn contains<'a>(&'a self, key: Self::RefKey<'a>) -> bool { self.atoms.iter().any(|atom| &atom.ident == key) @@ -455,7 +454,7 @@ impl From for Ilst { for item in input.items { let key = item.item_key; - if let Some(ident) = item_key_to_ident(&key).map(Into::into) { + if let Some(ident) = item_key_to_ident(&key) { let data = match item.item_value { ItemValue::Text(text) => text, _ => continue, @@ -467,7 +466,7 @@ impl From for Ilst { ItemKey::DiscNumber => convert_to_uint(&mut discs.0, data.as_str()), ItemKey::DiscTotal => convert_to_uint(&mut discs.1, data.as_str()), _ => ilst.atoms.push(Atom { - ident, + ident: ident.into_owned(), data: AtomDataStorage::Single(AtomData::UTF8(data)), }), } @@ -492,7 +491,7 @@ impl From for Ilst { } } -fn item_key_to_ident(key: &ItemKey) -> Option> { +fn item_key_to_ident(key: &ItemKey) -> Option> { key.map_key(TagType::MP4ilst, true).and_then(|ident| { if ident.starts_with("----") { let mut split = ident.split(':'); @@ -503,7 +502,10 @@ fn item_key_to_ident(key: &ItemKey) -> Option> { let name = split.next(); if let (Some(mean), Some(name)) = (mean, name) { - Some(AtomIdentRef::Freeform { mean, name }) + Some(AtomIdent::Freeform { + mean: Cow::Borrowed(mean), + name: Cow::Borrowed(name), + }) } else { None } @@ -511,7 +513,7 @@ fn item_key_to_ident(key: &ItemKey) -> Option> { let fourcc = ident.chars().map(|c| c as u8).collect::>(); if let Ok(fourcc) = TryInto::<[u8; 4]>::try_into(fourcc) { - Some(AtomIdentRef::Fourcc(fourcc)) + Some(AtomIdent::Fourcc(fourcc)) } else { None } diff --git a/src/mp4/ilst/ref.rs b/src/mp4/ilst/ref.rs index 501ef7cd..7228b11d 100644 --- a/src/mp4/ilst/ref.rs +++ b/src/mp4/ilst/ref.rs @@ -36,42 +36,16 @@ where } } -impl Atom { +impl<'a> Atom<'a> { pub(super) fn as_ref(&self) -> AtomRef<'_, impl IntoIterator> { AtomRef { - ident: (&self.ident).into(), + ident: self.ident.as_borrowed(), data: (&self.data).into_iter(), } } } pub(crate) struct AtomRef<'a, I> { - pub(crate) ident: AtomIdentRef<'a>, + pub(crate) ident: AtomIdent<'a>, pub(crate) data: I, } - -pub(crate) enum AtomIdentRef<'a> { - Fourcc([u8; 4]), - Freeform { mean: &'a str, name: &'a str }, -} - -impl<'a> Into> for &'a AtomIdent { - fn into(self) -> AtomIdentRef<'a> { - match self { - AtomIdent::Fourcc(fourcc) => AtomIdentRef::Fourcc(*fourcc), - AtomIdent::Freeform { mean, name } => AtomIdentRef::Freeform { mean, name }, - } - } -} - -impl<'a> From> for AtomIdent { - fn from(input: AtomIdentRef<'a>) -> Self { - match input { - AtomIdentRef::Fourcc(fourcc) => AtomIdent::Fourcc(fourcc), - AtomIdentRef::Freeform { mean, name } => AtomIdent::Freeform { - mean: mean.to_string(), - name: name.to_string(), - }, - } - } -} diff --git a/src/mp4/ilst/write.rs b/src/mp4/ilst/write.rs index a7e3f856..af5607f8 100644 --- a/src/mp4/ilst/write.rs +++ b/src/mp4/ilst/write.rs @@ -3,7 +3,7 @@ use crate::error::{FileEncodingError, Result}; use crate::file::FileType; use crate::macros::{err, try_vec}; use crate::mp4::atom_info::{AtomIdent, AtomInfo}; -use crate::mp4::ilst::r#ref::{AtomIdentRef, AtomRef}; +use crate::mp4::ilst::r#ref::AtomRef; use crate::mp4::moov::Moov; use crate::mp4::read::{atom_tree, meta_is_full, nested_atom, verify_mp4, AtomReader}; use crate::mp4::AtomData; @@ -322,8 +322,8 @@ where writer.write_all(&[0; 4])?; match atom.ident { - AtomIdentRef::Fourcc(ref fourcc) => writer.write_all(fourcc)?, - AtomIdentRef::Freeform { mean, name } => write_freeform(mean, name, &mut writer)?, + AtomIdent::Fourcc(ref fourcc) => writer.write_all(fourcc)?, + AtomIdent::Freeform { mean, name } => write_freeform(&mean, &name, &mut writer)?, } write_atom_data(atom.data, &mut writer)?;