FLAC: Retain vendor strings on tag write

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

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,

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)),
 			}

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<Item = (&'a Picture, PictureInformation)>,
 {
 	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::<LittleEndian>()?;
+				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(())

lofty/src/ogg/tag.rs

@@ -471,7 +471,7 @@ impl TagExt for VorbisComments {
 		LoftyError: From<<F as Length>::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<Item = (&'a str, &'a str)>,
 	IP: Iterator<Item = (&'a Picture, PictureInformation)>,
 {
-	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(())
 	}

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<Vec<u8>>
 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::<LittleEndian>(vendor.len() as u32)?;
-	new_comment_packet.write_all(vendor)?;
+	new_comment_packet.write_u32::<LittleEndian>(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()?;

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<W: Write>(
 			let (vendor, items, pictures) = create_vorbis_comments_ref(tag);
 
 			VorbisCommentsRef {
-				vendor,
+				vendor: Cow::from(vendor),
 				items,
 				pictures,
 			}

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");
+}