diff --git a/CHANGELOG.md b/CHANGELOG.md index 951511911..1a9ba7c81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **FLAC**: Vendor strings are now retained when writing tags ([PR](https://github.com/Serial-ATA/lofty-rs/pull/443)) + - This behavior already exists for OGG formats. + ## [0.21.0] - 2024-07-29 ### Added diff --git a/lofty/src/flac/block.rs b/lofty/src/flac/block.rs index 6b44595d8..ee2a596ad 100644 --- a/lofty/src/flac/block.rs +++ b/lofty/src/flac/block.rs @@ -13,6 +13,8 @@ pub(in crate::flac) const BLOCK_ID_SEEKTABLE: u8 = 3; pub(in crate::flac) const BLOCK_ID_VORBIS_COMMENTS: u8 = 4; pub(in crate::flac) const BLOCK_ID_PICTURE: u8 = 6; +const BLOCK_HEADER_SIZE: u64 = 4; + pub(crate) struct Block { pub(super) byte: u8, pub(super) ty: u8, @@ -46,7 +48,7 @@ impl Block { data.seek(SeekFrom::Current(i64::from(size)))?; } - let end = data.stream_position()?; + let end = start + u64::from(size) + BLOCK_HEADER_SIZE; Ok(Self { byte, diff --git a/lofty/src/flac/mod.rs b/lofty/src/flac/mod.rs index 72d1d0f73..11349b4a2 100644 --- a/lofty/src/flac/mod.rs +++ b/lofty/src/flac/mod.rs @@ -19,6 +19,8 @@ use crate::picture::{Picture, PictureInformation}; use crate::tag::TagExt; use crate::util::io::{FileLike, Length, Truncate}; +use std::borrow::Cow; + use lofty_attr::LoftyFile; // Exports @@ -67,7 +69,7 @@ impl FlacFile { // We have an existing vorbis comments tag, we can just append our pictures to it if let Some(ref vorbis_comments) = self.vorbis_comments_tag { return VorbisCommentsRef { - vendor: vorbis_comments.vendor.as_str(), + vendor: Cow::from(vorbis_comments.vendor.as_str()), items: vorbis_comments .items .iter() @@ -84,7 +86,7 @@ impl FlacFile { // We have pictures, but no vorbis comments tag, we'll need to create a dummy one if !self.pictures.is_empty() { return VorbisCommentsRef { - vendor: "", + vendor: Cow::from(""), items: std::iter::empty(), pictures: self.pictures.iter().map(|(p, i)| (p, *i)), } diff --git a/lofty/src/flac/write.rs b/lofty/src/flac/write.rs index 599e5546a..e4d349d29 100644 --- a/lofty/src/flac/write.rs +++ b/lofty/src/flac/write.rs @@ -1,4 +1,4 @@ -use super::block::Block; +use super::block::{Block, BLOCK_ID_PADDING, BLOCK_ID_PICTURE, BLOCK_ID_VORBIS_COMMENTS}; use super::read::verify_flac; use crate::config::WriteOptions; use crate::error::{LoftyError, Result}; @@ -9,9 +9,10 @@ use crate::picture::{Picture, PictureInformation}; use crate::tag::{Tag, TagType}; use crate::util::io::{FileLike, Length, Truncate}; -use std::io::{Cursor, Seek, SeekFrom, Write}; +use std::borrow::Cow; +use std::io::{Cursor, Read, Seek, SeekFrom, Write}; -use byteorder::{LittleEndian, WriteBytesExt}; +use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; const BLOCK_HEADER_SIZE: usize = 4; const MAX_BLOCK_SIZE: u32 = 16_777_215; @@ -27,7 +28,7 @@ where let (vendor, items, pictures) = crate::ogg::tag::create_vorbis_comments_ref(tag); let mut comments_ref = VorbisCommentsRef { - vendor, + vendor: Cow::from(vendor), items, pictures, }; @@ -52,7 +53,6 @@ where IP: Iterator, { let stream_info = verify_flac(file)?; - let stream_info_end = stream_info.end as usize; let mut last_block = stream_info.last; @@ -64,14 +64,14 @@ where let mut padding = false; let mut last_block_info = ( stream_info.byte, - stream_info_end - ((stream_info.end - stream_info.start) as u32 + 4) as usize, - stream_info_end, + stream_info.start as usize, + stream_info.end as usize, ); - let mut blocks_remove = Vec::new(); + let mut blocks_to_remove = Vec::new(); while !last_block { - let block = Block::read(&mut cursor, |_| true)?; + let block = Block::read(&mut cursor, |block_ty| block_ty == BLOCK_ID_VORBIS_COMMENTS)?; let start = block.start; let end = block.end; @@ -83,8 +83,27 @@ where } match block_type { - 4 | 6 => blocks_remove.push((start, end)), - 1 => padding = true, + BLOCK_ID_VORBIS_COMMENTS => { + blocks_to_remove.push((start, end)); + + // Retain the original vendor string + let reader = &mut &block.content[..]; + + let vendor_len = reader.read_u32::()?; + let mut vendor = try_vec![0; vendor_len as usize]; + reader.read_exact(&mut vendor)?; + + // TODO: Error on strict? + let Ok(vendor_str) = String::from_utf8(vendor) else { + log::warn!("FLAC vendor string is not valid UTF-8, not re-using"); + tag.vendor = Cow::Borrowed(""); + continue; + }; + + tag.vendor = Cow::Owned(vendor_str); + }, + BLOCK_ID_PICTURE => blocks_to_remove.push((start, end)), + BLOCK_ID_PADDING => padding = true, _ => {}, } } @@ -116,29 +135,29 @@ where let mut comment_blocks = Cursor::new(Vec::new()); - create_comment_block(&mut comment_blocks, tag.vendor, &mut tag.items)?; + create_comment_block(&mut comment_blocks, &*tag.vendor, &mut tag.items)?; let mut comment_blocks = comment_blocks.into_inner(); create_picture_blocks(&mut comment_blocks, &mut tag.pictures)?; - if blocks_remove.is_empty() { + if blocks_to_remove.is_empty() { file_bytes.splice(0..0, comment_blocks); } else { - blocks_remove.sort_unstable(); - blocks_remove.reverse(); + blocks_to_remove.sort_unstable(); + blocks_to_remove.reverse(); - let first = blocks_remove.pop().unwrap(); // Infallible + let first = blocks_to_remove.pop().unwrap(); // Infallible - for (s, e) in &blocks_remove { + for (s, e) in &blocks_to_remove { file_bytes.drain(*s as usize..*e as usize); } file_bytes.splice(first.0 as usize..first.1 as usize, comment_blocks); } - file.seek(SeekFrom::Start(stream_info_end as u64))?; - file.truncate(stream_info_end as u64)?; + file.seek(SeekFrom::Start(stream_info.end))?; + file.truncate(stream_info.end)?; file.write_all(&file_bytes)?; Ok(()) diff --git a/lofty/src/ogg/tag.rs b/lofty/src/ogg/tag.rs index fe7c676b2..da7f49b29 100644 --- a/lofty/src/ogg/tag.rs +++ b/lofty/src/ogg/tag.rs @@ -471,7 +471,7 @@ impl TagExt for VorbisComments { LoftyError: From<::Error>, { VorbisCommentsRef { - vendor: self.vendor.as_str(), + vendor: Cow::from(self.vendor.as_str()), items: self.items.iter().map(|(k, v)| (k.as_str(), v.as_str())), pictures: self.pictures.iter().map(|(p, i)| (p, *i)), } @@ -493,7 +493,7 @@ impl TagExt for VorbisComments { write_options: WriteOptions, ) -> std::result::Result<(), Self::Err> { VorbisCommentsRef { - vendor: self.vendor.as_str(), + vendor: Cow::from(self.vendor.as_str()), items: self.items.iter().map(|(k, v)| (k.as_str(), v.as_str())), pictures: self.pictures.iter().map(|(p, i)| (p, *i)), } @@ -634,7 +634,7 @@ where II: Iterator, IP: Iterator, { - pub vendor: &'a str, + pub vendor: Cow<'a, str>, pub items: II, pub pictures: IP, } @@ -676,8 +676,7 @@ where writer: &mut W, _write_options: WriteOptions, ) -> Result<()> { - let metadata_packet = - super::write::create_metadata_packet(self, &[], self.vendor.as_bytes(), false)?; + let metadata_packet = super::write::create_metadata_packet(self, &[], false)?; writer.write_all(&metadata_packet)?; Ok(()) } diff --git a/lofty/src/ogg/write.rs b/lofty/src/ogg/write.rs index 43471a102..13d137a20 100644 --- a/lofty/src/ogg/write.rs +++ b/lofty/src/ogg/write.rs @@ -7,10 +7,11 @@ use crate::ogg::constants::{OPUSTAGS, VORBIS_COMMENT_HEAD}; use crate::ogg::tag::{create_vorbis_comments_ref, VorbisCommentsRef}; use crate::picture::{Picture, PictureInformation}; use crate::tag::{Tag, TagType}; +use crate::util::io::{FileLike, Length, Truncate}; +use std::borrow::Cow; use std::io::{Cursor, Read, Seek, SeekFrom, Write}; -use crate::util::io::{FileLike, Length, Truncate}; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use ogg_pager::{Packets, Page, PageHeader, CONTAINS_FIRST_PAGE_OF_BITSTREAM}; @@ -58,7 +59,7 @@ where let (vendor, items, pictures) = create_vorbis_comments_ref(tag); let mut comments_ref = VorbisCommentsRef { - vendor, + vendor: Cow::from(vendor), items, pictures, }; @@ -120,9 +121,20 @@ where let mut vendor = try_vec![0; vendor_len as usize]; md_reader.read_exact(&mut vendor)?; + let vendor_str; + match String::from_utf8(vendor) { + Ok(s) => vendor_str = Cow::Owned(s), + Err(_) => { + // TODO: Error on strict? + log::warn!("OGG vendor string is not valid UTF-8, not re-using"); + vendor_str = Cow::Borrowed(""); + }, + } + + tag.vendor = vendor_str; + let add_framing_bit = format == OGGFormat::Vorbis; - let new_metadata_packet = - create_metadata_packet(tag, comment_signature, &vendor, add_framing_bit)?; + let new_metadata_packet = create_metadata_packet(tag, comment_signature, add_framing_bit)?; // Replace the old comment packet packets.set(1, new_metadata_packet); @@ -151,7 +163,6 @@ where pub(super) fn create_metadata_packet<'a, II, IP>( tag: &mut VorbisCommentsRef<'a, II, IP>, comment_signature: &[u8], - vendor: &[u8], add_framing_bit: bool, ) -> Result> where @@ -160,9 +171,10 @@ where { let mut new_comment_packet = Cursor::new(Vec::new()); + let vendor_bytes = tag.vendor.as_bytes(); new_comment_packet.write_all(comment_signature)?; - new_comment_packet.write_u32::(vendor.len() as u32)?; - new_comment_packet.write_all(vendor)?; + new_comment_packet.write_u32::(vendor_bytes.len() as u32)?; + new_comment_packet.write_all(vendor_bytes)?; // Zero out the item count for later let item_count_pos = new_comment_packet.stream_position()?; diff --git a/lofty/src/tag/utils.rs b/lofty/src/tag/utils.rs index fd9663941..a949ba772 100644 --- a/lofty/src/tag/utils.rs +++ b/lofty/src/tag/utils.rs @@ -15,6 +15,7 @@ use ape::tag::ApeTagRef; use iff::aiff::tag::AiffTextChunksRef; use iff::wav::tag::RIFFInfoListRef; +use std::borrow::Cow; use std::io::Write; #[allow(unreachable_patterns)] @@ -75,7 +76,7 @@ pub(crate) fn dump_tag( let (vendor, items, pictures) = create_vorbis_comments_ref(tag); VorbisCommentsRef { - vendor, + vendor: Cow::from(vendor), items, pictures, } diff --git a/lofty/tests/files/flac.rs b/lofty/tests/files/flac.rs index dd4f78aa3..70e934bfb 100644 --- a/lofty/tests/files/flac.rs +++ b/lofty/tests/files/flac.rs @@ -1,10 +1,13 @@ -use lofty::config::{ParseOptions, ParsingMode}; -use lofty::flac::FlacFile; -use lofty::prelude::*; +use crate::temp_file; use std::fs::File; use std::io::Seek; +use lofty::config::{ParseOptions, ParsingMode, WriteOptions}; +use lofty::flac::FlacFile; +use lofty::ogg::VorbisComments; +use lofty::prelude::*; + #[test] fn multiple_vorbis_comments() { let mut file = File::open("tests/files/assets/two_vorbis_comments.flac").unwrap(); @@ -40,3 +43,24 @@ fn read_no_properties() { fn read_no_tags() { crate::no_tag_test!("tests/files/assets/minimal/full_test.flac"); } + +#[test] +fn retain_vendor_string() { + let mut file = temp_file!("tests/files/assets/minimal/full_test.flac"); + + let f = FlacFile::read_from(&mut file, ParseOptions::new()).unwrap(); + file.rewind().unwrap(); + + assert_eq!(f.vorbis_comments().unwrap().vendor(), "Lavf58.76.100"); + + let mut tag = VorbisComments::new(); + tag.set_artist(String::from("Foo Artist")); + tag.set_vendor(String::from("Bar Vendor")); + tag.save_to(&mut file, WriteOptions::new()).unwrap(); + + file.rewind().unwrap(); + let f = FlacFile::read_from(&mut file, ParseOptions::new()).unwrap(); + + // The vendor string should be retained + assert_eq!(f.vorbis_comments().unwrap().vendor(), "Lavf58.76.100"); +}