fix(mp4): preserve hdlr atom when writing new tags to metadata containers with missing ilst

lofty/src/aac/properties.rs

@@ -94,7 +94,8 @@ impl From<AACProperties> for FileProperties {
 			bit_depth: None,
 			channels: Some(input.channels),
 			channel_mask: input.channel_mask,
-		}
+			bitrate_mode: None,
+}
 	}
 }
 

lofty/src/ape/properties.rs

@@ -31,7 +31,8 @@ impl From<ApeProperties> for FileProperties {
 			bit_depth: Some(input.bit_depth),
 			channels: Some(input.channels),
 			channel_mask: None,
-		}
+			bitrate_mode: Some(crate::properties::BitrateMode::Vbr),
+}
 	}
 }
 

lofty/src/flac/properties.rs

@@ -29,7 +29,8 @@ impl From<FlacProperties> for FileProperties {
 			bit_depth: Some(input.bit_depth),
 			channels: Some(input.channels),
 			channel_mask: None,
-		}
+			bitrate_mode: Some(crate::properties::BitrateMode::Vbr),
+}
 	}
 }
 

lofty/src/iff/aiff/properties.rs

@@ -111,7 +111,8 @@ impl From<AiffProperties> for FileProperties {
 			bit_depth: Some(value.sample_size as u8),
 			channels: Some(value.channels as u8),
 			channel_mask: None,
-		}
+			bitrate_mode: Some(crate::properties::BitrateMode::Cbr),
+}
 	}
 }
 

lofty/src/iff/wav/properties.rs

@@ -63,7 +63,8 @@ impl From<WavProperties> for FileProperties {
 			bit_depth: Some(bit_depth),
 			channels: Some(channels),
 			channel_mask,
-		}
+			bitrate_mode: Some(crate::properties::BitrateMode::Cbr),
+}
 	}
 }
 

lofty/src/mp4/ilst/write.rs

@@ -230,17 +230,7 @@ fn save_to_existing(
 		ParseOptions::DEFAULT_PARSING_MODE,
 	)?;
 
-	if tree.is_empty() {
-		// Nothing to do
-		if remove_tag {
-			return Ok(());
-		}
-
-		let meta_end = (meta.start + meta.len) as usize;
-
-		replacement = ilst;
-		range = meta_end..meta_end;
-	} else {
+	if let Some(ilst_idx) = ilst_idx {
 		let existing_ilst = &tree[ilst_idx];
 		let existing_ilst_size = existing_ilst.len;
 
@@ -315,6 +305,16 @@ fn save_to_existing(
 			replacement = ilst;
 			range = range_start as usize..range_end as usize;
 		}
+	} else {
+		// Nothing to do
+		if remove_tag {
+			return Ok(());
+		}
+
+		let meta_end = (meta.start + meta.len) as usize;
+
+		replacement = ilst;
+		range = meta_end..meta_end;
 	}
 
 	drop(write_handle);

lofty/src/mp4/properties.rs

@@ -204,7 +204,8 @@ impl From<Mp4Properties> for FileProperties {
 			bit_depth: input.bit_depth,
 			channels: Some(input.channels),
 			channel_mask: None,
-		}
+			bitrate_mode: None,
+}
 	}
 }
 

lofty/src/mp4/read/mod.rs

@@ -156,11 +156,11 @@ pub(super) fn atom_tree<R>(
 	mut len: u64,
 	up_to: &[u8],
 	parse_mode: ParsingMode,
-) -> Result<(usize, Vec<AtomInfo>)>
+) -> Result<(Option<usize>, Vec<AtomInfo>)>
 where
 	R: Read + Seek,
 {
-	let mut found_idx: usize = 0;
+	let mut found_idx: Option<usize> = None;
 	let mut buf = Vec::new();
 
 	let mut i = 0;
@@ -174,18 +174,15 @@ where
 		len = len.saturating_sub(atom.len);
 
 		if let AtomIdent::Fourcc(ref fourcc) = atom.ident {
-			i += 1;
-
 			if fourcc == up_to {
-				found_idx = i;
+				found_idx = Some(i);
 			}
+			i += 1;
 
 			buf.push(atom);
 		}
 	}
 
-	found_idx = found_idx.saturating_sub(1);
-
 	Ok((found_idx, buf))
 }
 

lofty/src/mpeg/properties.rs

@@ -23,6 +23,7 @@ pub struct MpegProperties {
 	pub(crate) copyright: bool,
 	pub(crate) original: bool,
 	pub(crate) emphasis: Option<Emphasis>,
+	pub(crate) bitrate_mode: Option<crate::properties::BitrateMode>,
 }
 
 impl From<MpegProperties> for FileProperties {
@@ -40,6 +41,7 @@ impl From<MpegProperties> for FileProperties {
 			emphasis: _,
 			mode_extension: _,
 			original: _,
+			bitrate_mode,
 		} = input;
 		let channel_mask = match channel_mode {
 			ChannelMode::SingleChannel => Some(ChannelMask::mono()),
@@ -54,6 +56,7 @@ impl From<MpegProperties> for FileProperties {
 			bit_depth: None,
 			channels: Some(channels),
 			channel_mask,
+			bitrate_mode,
 		}
 	}
 }
@@ -118,6 +121,11 @@ impl MpegProperties {
 	pub fn emphasis(&self) -> Option<Emphasis> {
 		self.emphasis
 	}
+
+	/// Bitrate mode of the audio
+	pub fn bitrate_mode(&self) -> Option<crate::properties::BitrateMode> {
+		self.bitrate_mode
+	}
 }
 
 pub(super) fn read_properties<R>(
@@ -136,6 +144,11 @@ where
 
 	properties.version = first_frame_header.version;
 	properties.layer = first_frame_header.layer;
+	properties.bitrate_mode = match vbr_header.as_ref().map(|h| h.ty) {
+		Some(VbrHeaderType::Info) => Some(crate::properties::BitrateMode::Cbr),
+		Some(VbrHeaderType::Xing | VbrHeaderType::Vbri) => Some(crate::properties::BitrateMode::Vbr),
+		None => Some(crate::properties::BitrateMode::Cbr),
+	};
 	properties.channel_mode = first_frame_header.channel_mode;
 	properties.mode_extension = first_frame_header.mode_extension;
 	properties.copyright = first_frame_header.copyright;

lofty/src/musepack/sv4to6/properties.rs

@@ -35,7 +35,8 @@ impl From<MpcSv4to6Properties> for FileProperties {
 			bit_depth: None,
 			channels: Some(input.channels),
 			channel_mask: None,
-		}
+			bitrate_mode: Some(crate::properties::BitrateMode::Vbr),
+}
 	}
 }
 

lofty/src/musepack/sv7/properties.rs

@@ -142,7 +142,8 @@ impl From<MpcSv7Properties> for FileProperties {
 			bit_depth: None,
 			channels: Some(input.channels),
 			channel_mask: None,
-		}
+			bitrate_mode: Some(crate::properties::BitrateMode::Vbr),
+}
 	}
 }
 

lofty/src/musepack/sv8/properties.rs

@@ -34,7 +34,8 @@ impl From<MpcSv8Properties> for FileProperties {
 			bit_depth: None,
 			channels: Some(input.stream_header.channels),
 			channel_mask: None,
-		}
+			bitrate_mode: Some(crate::properties::BitrateMode::Vbr),
+}
 	}
 }
 

lofty/src/ogg/opus/properties.rs

@@ -37,6 +37,7 @@ impl From<OpusProperties> for FileProperties {
 			} else {
 				Some(input.channel_mask)
 			},
+			bitrate_mode: Some(crate::properties::BitrateMode::Vbr),
 		}
 	}
 }

lofty/src/ogg/speex/properties.rs

@@ -35,7 +35,8 @@ impl From<SpeexProperties> for FileProperties {
 			bit_depth: None,
 			channels: Some(input.channels),
 			channel_mask: None,
-		}
+			bitrate_mode: Some(if input.vbr { crate::properties::BitrateMode::Vbr } else { crate::properties::BitrateMode::Cbr }),
+}
 	}
 }
 

lofty/src/ogg/vorbis/properties.rs

@@ -34,7 +34,8 @@ impl From<VorbisProperties> for FileProperties {
 			bit_depth: None,
 			channels: Some(input.channels),
 			channel_mask: None,
-		}
+			bitrate_mode: Some(crate::properties::BitrateMode::Vbr),
+}
 	}
 }
 

lofty/src/properties/file_properties.rs

@@ -1,6 +1,16 @@
 use super::channel_mask::ChannelMask;
 use std::time::Duration;
 
+/// Bitrate mode of an audio file
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+#[non_exhaustive]
+pub enum BitrateMode {
+	/// Constant Bitrate (CBR)
+	Cbr,
+	/// Variable Bitrate (VBR)
+	Vbr,
+}
+
 /// Various *immutable* audio properties
 #[derive(Debug, PartialEq, Eq, Clone)]
 #[non_exhaustive]
@@ -12,6 +22,7 @@ pub struct FileProperties {
 	pub(crate) bit_depth: Option<u8>,
 	pub(crate) channels: Option<u8>,
 	pub(crate) channel_mask: Option<ChannelMask>,
+	pub(crate) bitrate_mode: Option<BitrateMode>,
 }
 
 impl Default for FileProperties {
@@ -24,6 +35,7 @@ impl Default for FileProperties {
 			bit_depth: None,
 			channels: None,
 			channel_mask: None,
+			bitrate_mode: None,
 		}
 	}
 }
@@ -39,6 +51,7 @@ impl FileProperties {
 		bit_depth: Option<u8>,
 		channels: Option<u8>,
 		channel_mask: Option<ChannelMask>,
+		bitrate_mode: Option<BitrateMode>,
 	) -> Self {
 		Self {
 			duration,
@@ -48,6 +61,7 @@ impl FileProperties {
 			bit_depth,
 			channels,
 			channel_mask,
+			bitrate_mode,
 		}
 	}
 
@@ -86,6 +100,11 @@ impl FileProperties {
 		self.channel_mask
 	}
 
+	/// Bitrate mode of the audio
+	pub fn bitrate_mode(&self) -> Option<BitrateMode> {
+		self.bitrate_mode
+	}
+
 	/// Used for tests
 	#[doc(hidden)]
 	pub fn is_empty(&self) -> bool {
@@ -99,6 +118,7 @@ impl FileProperties {
 				bit_depth: None | Some(0),
 				channels: None | Some(0),
 				channel_mask: None,
+				bitrate_mode: None,
 			}
 		)
 	}

lofty/src/properties/mod.rs

@@ -11,4 +11,4 @@ mod file_properties;
 mod tests;
 
 pub use channel_mask::ChannelMask;
-pub use file_properties::FileProperties;
+pub use file_properties::{BitrateMode, FileProperties};

lofty/src/wavpack/properties.rs

@@ -37,6 +37,7 @@ impl From<WavPackProperties> for FileProperties {
 			} else {
 				Some(input.channel_mask)
 			},
+			bitrate_mode: Some(crate::properties::BitrateMode::Vbr),
 		}
 	}
 }

lofty/tests/files/mp4.rs

@@ -73,3 +73,33 @@ fn read_no_properties() {
 fn read_no_tags() {
 	crate::util::no_tag_test("tests/files/assets/minimal/m4a_codec_aac.m4a", None);
 }
+
+#[test_log::test]
+fn hdlr_preservation_on_tag_recreation() {
+	use lofty::tag::Tag;
+	use std::fs;
+
+	let src = "tests/files/assets/minimal/m4a_codec_aac.m4a";
+	let temp_path = "tests/files/assets/minimal/temp_hdlr_test.m4a";
+	let _ = fs::remove_file(temp_path);
+	fs::copy(src, temp_path).unwrap();
+
+	// 1. Remove all tags
+	TagType::Mp4Ilst.remove_from_path(temp_path).unwrap();
+
+	// 2. Open the file and write a brand new tag
+	let mut tagged_file = Probe::open(temp_path).unwrap().read().unwrap();
+	let tag = Tag::new(TagType::Mp4Ilst);
+	tagged_file.insert_tag(tag);
+	tagged_file
+		.save_to_path(temp_path, lofty::config::WriteOptions::default())
+		.unwrap();
+
+	// 3. Read the file bytes and verify that "hdlr" is present
+	let bytes = fs::read(temp_path).unwrap();
+	let has_hdlr = bytes.windows(4).any(|w| w == b"hdlr");
+
+	let _ = fs::remove_file(temp_path);
+
+	assert!(has_hdlr, "Output M4A is missing the hdlr atom!");
+}