Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ID3v2: Reduce memory allocations for FrameID #102

Merged
merged 2 commits into from
Jan 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/id3/v2/frame/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ use crate::error::{ID3v2Error, ID3v2ErrorKind, Result};
use crate::id3::v2::util::upgrade::{upgrade_v2, upgrade_v3};
use crate::id3::v2::FrameID;

use std::borrow::Cow;
use std::io::Read;

pub(crate) fn parse_v2_header<R>(reader: &mut R) -> Result<Option<(FrameID, u32, FrameFlags)>>
pub(crate) fn parse_v2_header<R>(
reader: &mut R,
) -> Result<Option<(FrameID<'static>, u32, FrameFlags)>>
where
R: Read,
{
Expand All @@ -22,7 +25,7 @@ where

let id_str = std::str::from_utf8(&frame_header[..3])
.map_err(|_| ID3v2Error::new(ID3v2ErrorKind::BadFrameID))?;
let id = upgrade_v2(id_str).unwrap_or(id_str);
let id = upgrade_v2(id_str).map_or_else(|| Cow::Owned(id_str.to_owned()), Cow::Borrowed);

let frame_id = FrameID::new(id)?;

Expand All @@ -35,7 +38,7 @@ where
pub(crate) fn parse_header<R>(
reader: &mut R,
synchsafe: bool,
) -> Result<Option<(FrameID, u32, FrameFlags)>>
) -> Result<Option<(FrameID<'static>, u32, FrameFlags)>>
where
R: Read,
{
Expand All @@ -59,7 +62,7 @@ where
frame_id_end = 3;
}

let mut id_str = std::str::from_utf8(&frame_header[..frame_id_end])
let id_str = std::str::from_utf8(&frame_header[..frame_id_end])
.map_err(|_| ID3v2Error::new(ID3v2ErrorKind::BadFrameID))?;

let mut size = u32::from_be_bytes([
Expand All @@ -70,21 +73,24 @@ where
]);

// Now upgrade the FrameID
if invalid_v2_frame {
let id = if invalid_v2_frame {
if let Some(id) = upgrade_v2(id_str) {
id_str = id;
Cow::Borrowed(id)
} else {
Cow::Owned(id_str.to_owned())
}
} else if !synchsafe {
id_str = upgrade_v3(id_str).unwrap_or(id_str);
}
upgrade_v3(id_str).map_or_else(|| Cow::Owned(id_str.to_owned()), Cow::Borrowed)
} else {
Cow::Owned(id_str.to_owned())
};
let frame_id = FrameID::new(id)?;

// unsynch the frame size if necessary
if synchsafe {
size = crate::id3::v2::util::unsynch_u32(size);
}

let frame_id = FrameID::new(id_str)?;

let flags = u16::from_be_bytes([frame_header[8], frame_header[9]]);
let flags = parse_flags(flags, synchsafe);

Expand Down
37 changes: 23 additions & 14 deletions src/id3/v2/frame/id.rs
Original file line number Diff line number Diff line change
@@ -1,45 +1,47 @@
use std::borrow::Cow;

use crate::error::{ID3v2Error, ID3v2ErrorKind, LoftyError, Result};
use crate::tag::item::ItemKey;
use crate::tag::TagType;

/// An `ID3v2` frame ID
#[derive(PartialEq, Clone, Debug, Eq, Hash)]
pub enum FrameID {
pub enum FrameID<'a> {
/// A valid `ID3v2.3/4` frame
Valid(String),
Valid(Cow<'a, str>),
/// When an `ID3v2.2` key couldn't be upgraded
///
/// This **will not** be written. It is up to the user to upgrade and store the key as [`Id3v2Frame::Valid`](Self::Valid).
///
/// The entire frame is stored as [`ItemValue::Binary`](crate::ItemValue::Binary).
Outdated(String),
Outdated(Cow<'a, str>),
}

impl FrameID {
impl<'a> FrameID<'a> {
/// Attempts to create a `FrameID` from an ID string
///
/// # Errors
///
/// * `id` contains invalid characters (must be 'A'..='Z' and '0'..='9')
/// * `id` is an invalid length (must be 3 or 4)
pub fn new(id: &str) -> Result<Self> {
Self::verify_id(id)?;
pub fn new(id: Cow<'a, str>) -> Result<Self> {
Self::verify_id(&id)?;

match id.len() {
3 => Ok(FrameID::Outdated(id.to_string())),
4 => Ok(FrameID::Valid(id.to_string())),
3 => Ok(FrameID::Outdated(id)),
4 => Ok(FrameID::Valid(id)),
_ => Err(ID3v2Error::new(ID3v2ErrorKind::BadFrameID).into()),
}
}

/// Extracts the string from the ID
pub fn as_str(&self) -> &str {
match self {
FrameID::Valid(v) | FrameID::Outdated(v) => v.as_str(),
FrameID::Valid(v) | FrameID::Outdated(v) => &v,
}
}

pub(crate) fn verify_id(id_str: &str) -> Result<()> {
pub(super) fn verify_id(id_str: &str) -> Result<()> {
for c in id_str.chars() {
if !c.is_ascii_uppercase() && !c.is_ascii_digit() {
return Err(ID3v2Error::new(ID3v2ErrorKind::BadFrameID).into());
Expand All @@ -48,24 +50,31 @@ impl FrameID {

Ok(())
}

pub(super) fn into_owned(self) -> FrameID<'static> {
match self {
Self::Valid(inner) => FrameID::Valid(Cow::Owned(inner.into_owned())),
Self::Outdated(inner) => FrameID::Outdated(Cow::Owned(inner.into_owned())),
}
}
}

impl TryFrom<&ItemKey> for FrameID {
impl<'a> TryFrom<&'a ItemKey> for FrameID<'a> {
type Error = LoftyError;

fn try_from(value: &ItemKey) -> std::prelude::rust_2015::Result<Self, Self::Error> {
fn try_from(value: &'a ItemKey) -> std::prelude::rust_2015::Result<Self, Self::Error> {
match value {
ItemKey::Unknown(unknown)
if unknown.len() == 4
&& unknown
.chars()
.all(|c| c.is_ascii_uppercase() || c.is_ascii_digit()) =>
{
Ok(Self::Valid(unknown.clone()))
Ok(Self::Valid(Cow::Borrowed(unknown)))
},
k => k.map_key(TagType::ID3v2, false).map_or(
Err(ID3v2Error::new(ID3v2ErrorKind::BadFrameID).into()),
|id| Ok(Self::Valid(id.to_string())),
|id| Ok(Self::Valid(Cow::Borrowed(id))),
),
}
}
Expand Down
34 changes: 17 additions & 17 deletions src/id3/v2/frame/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ use std::hash::{Hash, Hasher};
/// `ID3v2.3`, unlike `ID3v2.2`, stores frame IDs in 4 characters like `ID3v2.4`. There are some IDs that need upgrading (See [`upgrade_v3`]),
/// but anything that fails to be upgraded **will not** be stored as [`FrameID::Outdated`], as it is likely not an issue to write.
#[derive(Clone, Debug, Eq)]
pub struct Frame {
pub(super) id: FrameID,
pub struct Frame<'a> {
pub(super) id: FrameID<'a>,
pub(super) value: FrameValue,
pub(super) flags: FrameFlags,
}

impl PartialEq for Frame {
impl<'a> PartialEq for Frame<'a> {
fn eq(&self, other: &Self) -> bool {
match self.value {
FrameValue::Text { .. } => self.id == other.id,
Expand All @@ -52,7 +52,7 @@ impl PartialEq for Frame {
}
}

impl Hash for Frame {
impl<'a> Hash for Frame<'a> {
fn hash<H: Hasher>(&self, state: &mut H) {
match self.value {
FrameValue::Text { .. } => self.id.hash(state),
Expand All @@ -64,7 +64,7 @@ impl Hash for Frame {
}
}

impl Frame {
impl<'a> Frame<'a> {
/// Create a new frame
///
/// NOTE: This will accept both `ID3v2.2` and `ID3v2.3/4` frame IDs
Expand All @@ -73,16 +73,16 @@ impl Frame {
///
/// * `id` is less than 3 or greater than 4 bytes
/// * `id` contains non-ascii characters
pub fn new(id: &str, value: FrameValue, flags: FrameFlags) -> Result<Self> {
pub fn new(id: Cow<'a, str>, value: FrameValue, flags: FrameFlags) -> Result<Self> {
let id_updated = match id.len() {
// An ID with a length of 4 could be either V3 or V4.
4 => match upgrade_v3(id) {
4 => match upgrade_v3(&id) {
None => id,
Some(upgraded) => upgraded,
Some(upgraded) => Cow::Borrowed(upgraded),
},
3 => match upgrade_v2(id) {
3 => match upgrade_v2(&id) {
None => id,
Some(upgraded) => upgraded,
Some(upgraded) => Cow::Borrowed(upgraded),
},
_ => return Err(ID3v2Error::new(ID3v2ErrorKind::BadFrameID).into()),
};
Expand Down Expand Up @@ -113,9 +113,9 @@ impl Frame {
}

// Used internally, has no correctness checks
pub(crate) fn text(id: &str, content: String) -> Self {
pub(crate) fn text(id: Cow<'a, str>, content: String) -> Self {
Self {
id: FrameID::Valid(String::from(id)),
id: FrameID::Valid(id),
value: FrameValue::Text {
encoding: TextEncoding::UTF8,
value: content,
Expand Down Expand Up @@ -256,11 +256,11 @@ pub struct FrameFlags {
pub data_length_indicator: Option<u32>,
}

impl From<TagItem> for Option<Frame> {
impl From<TagItem> for Option<Frame<'static>> {
fn from(input: TagItem) -> Self {
let frame_id;
let value;
match input.key().try_into() {
match input.key().try_into().map(FrameID::into_owned) {
Ok(id) => {
// We make the VERY bold assumption the language is English
value = match (&id, input.item_value) {
Expand Down Expand Up @@ -307,15 +307,15 @@ impl From<TagItem> for Option<Frame> {
Err(_) => match input.item_key.map_key(TagType::ID3v2, true) {
Some(desc) => match input.item_value {
ItemValue::Text(text) => {
frame_id = FrameID::Valid(String::from("TXXX"));
frame_id = FrameID::Valid(Cow::Borrowed("TXXX"));
value = FrameValue::UserText(EncodedTextFrame {
encoding: TextEncoding::UTF8,
description: String::from(desc),
content: text,
})
},
ItemValue::Locator(locator) => {
frame_id = FrameID::Valid(String::from("WXXX"));
frame_id = FrameID::Valid(Cow::Borrowed("WXXX"));
value = FrameValue::UserURL(EncodedTextFrame {
encoding: TextEncoding::UTF8,
description: String::from(desc),
Expand All @@ -342,7 +342,7 @@ pub(crate) struct FrameRef<'a> {
pub flags: FrameFlags,
}

impl<'a> Frame {
impl<'a> Frame<'a> {
pub(crate) fn as_opt_ref(&'a self) -> Option<FrameRef<'a>> {
if let FrameID::Valid(id) = &self.id {
Some(FrameRef {
Expand Down
2 changes: 1 addition & 1 deletion src/id3/v2/frame/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::io::Read;

use byteorder::{BigEndian, ReadBytesExt};

impl Frame {
impl<'a> Frame<'a> {
pub(crate) fn read<R>(reader: &mut R, version: ID3v2Version) -> Result<(Option<Self>, bool)>
where
R: Read,
Expand Down
Loading