Vorbis/Ape: Verify FlagCompilation item contents on merge

CHANGELOG.md

@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+- **VorbisComments**/**ApeTag**: Verify contents of `ItemKey::FlagCompilation` during `Tag` merge ([PR](https://github.com/Serial-ATA/lofty-rs/pull/387))
+
 ## [0.19.2] - 2024-04-26
 
 ### Added

lofty/src/ape/tag/mod.rs

@@ -10,12 +10,13 @@ use crate::tag::item::ItemValueRef;
 use crate::tag::{
 	try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType,
 };
+use crate::util::flag_item;
+use crate::util::io::{FileLike, Truncate};
 
 use std::borrow::Cow;
 use std::io::Write;
 use std::ops::Deref;
 
-use crate::util::io::{FileLike, Truncate};
 use lofty_attr::tag;
 
 macro_rules! impl_accessor {
@@ -163,6 +164,20 @@ impl ApeTag {
 			ItemKey::TrackTotal => set_number(&item, |number| self.set_track_total(number)),
 			ItemKey::DiscNumber => set_number(&item, |number| self.set_disk(number)),
 			ItemKey::DiscTotal => set_number(&item, |number| self.set_disk_total(number)),
+
+			// Normalize flag items
+			ItemKey::FlagCompilation => {
+				let Some(text) = item.item_value.text() else {
+					return;
+				};
+
+				let Some(flag) = flag_item(text) else {
+					return;
+				};
+
+				let value = u8::from(flag).to_string();
+				self.insert(ApeItem::text("Compilation", value));
+			},
 			_ => {
 				if let Ok(item) = item.try_into() {
 					self.insert(item);

lofty/src/id3/v2/tag.rs

@@ -21,6 +21,7 @@ use crate::picture::{Picture, PictureType, TOMBSTONE_PICTURE};
 use crate::tag::{
 	try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType,
 };
+use crate::util::flag_item;
 use crate::util::io::{FileLike, Length, Truncate};
 use crate::util::text::{decode_text, TextDecodeOptions, TextEncoding};
 
@@ -1387,21 +1388,21 @@ impl MergeTag for SplitTagRemainder {
 
 		// Flag items
 		for item_key in [&ItemKey::FlagCompilation, &ItemKey::FlagPodcast] {
-			let Some(flag_value) = tag.take_strings(item_key).next() else {
+			let Some(text) = tag.take_strings(item_key).next() else {
 				continue;
 			};
 
-			if flag_value != "1" && flag_value != "0" {
+			let Some(flag_value) = flag_item(&text) else {
 				continue;
-			}
+			};
 
 			let frame_id = item_key
 				.map_key(TagType::Id3v2, false)
 				.expect("valid frame id");
 
 			merged.frames.push(new_text_frame(
 				FrameId::Valid(Cow::Borrowed(frame_id)),
-				flag_value,
+				u8::from(flag_value).to_string(),
 				FrameFlags::default(),
 			));
 		}

lofty/src/mp4/ilst/mod.rs

@@ -12,6 +12,7 @@ use crate::picture::{Picture, PictureType, TOMBSTONE_PICTURE};
 use crate::tag::{
 	try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType,
 };
+use crate::util::flag_item;
 use crate::util::io::{FileLike, Length, Truncate};
 use atom::{AdvisoryRating, Atom, AtomData};
 
@@ -705,18 +706,10 @@ impl MergeTag for SplitTagRemainder {
 					ItemKey::DiscNumber => convert_to_uint(&mut discs.0, text.as_str()),
 					ItemKey::DiscTotal => convert_to_uint(&mut discs.1, text.as_str()),
 					ItemKey::FlagCompilation | ItemKey::FlagPodcast => {
-						let Ok(num) = text.as_str().parse::<u8>() else {
+						let Some(data) = flag_item(text.as_str()) else {
 							continue;
 						};
 
-						let data = match num {
-							0 => false,
-							1 => true,
-							_ => {
-								// Ignore all other, unexpected values
-								continue;
-							},
-						};
 						merged.atoms.push(Atom {
 							ident: ident.into_owned(),
 							data: AtomDataStorage::Single(AtomData::Bool(data)),

lofty/src/ogg/tag.rs

@@ -9,12 +9,13 @@ use crate::probe::Probe;
 use crate::tag::{
 	try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType,
 };
+use crate::util::flag_item;
+use crate::util::io::{FileLike, Length, Truncate};
 
 use std::borrow::Cow;
 use std::io::Write;
 use std::ops::Deref;
 
-use crate::util::io::{FileLike, Length, Truncate};
 use lofty_attr::tag;
 
 macro_rules! impl_accessor {
@@ -575,10 +576,19 @@ impl MergeTag for SplitTagRemainder {
 			let item_value = item.item_value;
 
 			// Discard binary items, as they are not allowed in Vorbis comments
-			let (ItemValue::Text(val) | ItemValue::Locator(val)) = item_value else {
+			let (ItemValue::Text(mut val) | ItemValue::Locator(mut val)) = item_value else {
 				continue;
 			};
 
+			// Normalize flag items
+			if item_key == ItemKey::FlagCompilation {
+				let Some(flag) = flag_item(&val) else {
+					continue;
+				};
+
+				val = u8::from(flag).to_string();
+			}
+
 			let key;
 			match item_key {
 				ItemKey::Unknown(unknown) => {

lofty/src/util/mod.rs

@@ -2,3 +2,11 @@ pub(crate) mod alloc;
 pub mod io;
 pub(crate) mod math;
 pub(crate) mod text;
+
+pub(crate) fn flag_item(item: &str) -> Option<bool> {
+	match item {
+		"1" | "true" => Some(true),
+		"0" | "false" => Some(false),
+		_ => None,
+	}
+}