FLAC: Do not error on multiple VorbisComments when not strict

This is not allowed [by spec](https://xiph.org/flac/format.html#def_VORBIS_COMMENT), but is still possible to encounter in the wild. Now we will just tag whichever tag happens to be latest in the stream and use it, they **will not be merged** like other formats (such as ID3v2 in MP3) where multiple tags are valid.

Author: Serial-ATA

CHANGELOG.md

@@ -23,6 +23,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - `Ilst::remove` will now return all of the removed atoms
   - `Ilst::insert_picture` will now combine all pictures into a single `covr` atom
   - `Ilst::insert` will now merge atoms with the same identifier into a single atom
+- **FLAC**: Allow multiple Vorbis Comment blocks when not using `ParsingMode::Strict`
+  - This is not allowed [by spec](https://xiph.org/flac/format.html#def_VORBIS_COMMENT), but is still possible
+    to encounter in the wild. Now we will just tag whichever tag happens to be latest in the stream and
+    use it, they **will not be merged**.
 
 ## [0.15.0] - 2023-07-11
 

src/flac/read.rs

@@ -11,7 +11,7 @@ use crate::id3::{find_id3v2, ID3FindResults};
 use crate::macros::decode_err;
 use crate::ogg::read::read_comments;
 use crate::picture::Picture;
-use crate::probe::ParseOptions;
+use crate::probe::{ParseOptions, ParsingMode};
 
 use std::io::{Read, Seek, SeekFrom};
 
@@ -74,7 +74,18 @@ where
 		}
 
 		if block.ty == BLOCK_ID_VORBIS_COMMENTS {
-			if flac_file.vorbis_comments_tag.is_some() {
+			// NOTE: According to the spec
+			//
+			// <https://xiph.org/flac/format.html#def_VORBIS_COMMENT>:
+			// "There may be only one VORBIS_COMMENT block in a stream."
+			//
+			// But of course, we can't ever expect any spec compliant inputs, so we just
+			// take whatever happens to be the latest block in the stream. This is safe behavior,
+			// as when writing to a file with multiple tags, we end up removing all `VORBIS_COMMENT`
+			// blocks anyway.
+			if flac_file.vorbis_comments_tag.is_some()
+				&& parse_options.parsing_mode == ParsingMode::Strict
+			{
 				decode_err!(@BAIL Flac, "Streams are only allowed one Vorbis Comments block per stream");
 			}
 

tests/files/assets/two_vorbis_comments.flac

@@ -0,0 +1,31 @@
+use lofty::flac::FlacFile;
+use lofty::{Accessor, AudioFile, ParseOptions, ParsingMode};
+
+use std::fs::File;
+use std::io::Seek;
+
+#[test]
+fn multiple_vorbis_comments() {
+	let mut file = File::open("tests/files/assets/two_vorbis_comments.flac").unwrap();
+
+	// Reading a file with multiple VORBIS_COMMENT blocks should error when using `Strict`, as it is
+	// not allowed by spec.
+	assert!(FlacFile::read_from(
+		&mut file,
+		ParseOptions::new()
+			.read_properties(false)
+			.parsing_mode(ParsingMode::Strict)
+	)
+	.is_err());
+
+	file.rewind().unwrap();
+
+	// But by default, we should just take the last tag in the stream
+	let f = FlacFile::read_from(&mut file, ParseOptions::new().read_properties(false)).unwrap();
+
+	// The first tag has the artist "Artist 1", the second has "Artist 2".
+	assert_eq!(
+		f.vorbis_comments().unwrap().artist().as_deref(),
+		Some("Artist 2")
+	);
+}

tests/files/flac.rs

@@ -1,6 +1,7 @@
 mod aac;
 mod aiff;
 mod ape;
+mod flac;
 mod mp4;
 mod mpc;
 mod mpeg;