diff --git a/Cargo.lock b/Cargo.lock index 7687527..824ec55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12,6 +12,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aae1277d39aeec15cb388266ecc24b11c80469deae6067e17a1a7aa9e5c1f234" +[[package]] +name = "anyhow" +version = "1.0.38" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "afddf7f520a80dbf76e6f50a35bca42a2331ef227a28b3b6dc5c2e2338d114b1" + [[package]] name = "autocfg" version = "1.0.1" @@ -202,6 +208,7 @@ dependencies = [ name = "id3-image" version = "0.2.0" dependencies = [ + "anyhow", "id3", "image", "structopt", diff --git a/Cargo.toml b/Cargo.toml index a69f03d..8dc0ec0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ categories = [ "command-line-utilities", "filesystem", "multimedia" ] travis-ci = { repository = "AndrewRadev/id3-image" } [dependencies] +anyhow = "1.0.38" id3 = "0.6.2" image = "0.23.13" structopt = { version = "0.3.21", default-features = false } diff --git a/src/lib.rs b/src/lib.rs index 5806eb4..7bc7f26 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ use std::path::Path; -use std::error::Error; + +use anyhow::anyhow; /// Embed the image from `image_filename` into `music_filename`, in-place. Any errors reading ID3 /// tags from the music file or parsing the image get propagated upwards. @@ -7,12 +8,10 @@ use std::error::Error; /// The image is encoded as a JPEG with a 90% quality setting, and embedded as a "Front cover". /// Tags get written as ID3v2.3. /// -pub fn embed_image(music_filename: &Path, image_filename: &Path) -> Result<(), Box> { - let mut tag = id3::Tag::read_from_path(&music_filename). - map_err(|e| format!("Error reading music file {:?}: {}", music_filename, e))?; - +pub fn embed_image(music_filename: &Path, image_filename: &Path) -> anyhow::Result<()> { + let mut tag = read_tag(music_filename)?; let image = image::open(&image_filename). - map_err(|e| format!("Error reading image {:?}: {}", image_filename, e))?; + map_err(|e| anyhow!("Error reading image {:?}: {}", image_filename, e))?; let mut encoded_image_bytes = Vec::new(); // Unwrap: Writing to a Vec should always succeed; @@ -26,7 +25,7 @@ pub fn embed_image(music_filename: &Path, image_filename: &Path) -> Result<(), B }); tag.write_to_path(music_filename, id3::Version::Id3v23). - map_err(|e| format!("Error writing image to music file {:?}: {}", music_filename, e))?; + map_err(|e| anyhow!("Error writing image to music file {:?}: {}", music_filename, e))?; Ok(()) } @@ -37,24 +36,22 @@ pub fn embed_image(music_filename: &Path, image_filename: &Path) -> Result<(), B /// Any errors from parsing id3 tags will be propagated. The function will also return an error if /// there's no embedded images in the mp3 file. /// -pub fn extract_first_image(music_filename: &Path, image_filename: &Path) -> Result<(), Box> { - let tag = id3::Tag::read_from_path(&music_filename). - map_err(|e| format!("Error reading music file {:?}: {}", music_filename, e))?; - +pub fn extract_first_image(music_filename: &Path, image_filename: &Path) -> anyhow::Result<()> { + let tag = read_tag(music_filename)?; let first_picture = tag.pictures().next(); if let Some(p) = first_picture { match image::load_from_memory(&p.data) { Ok(image) => { image.save(&image_filename). - map_err(|e| format!("Couldn't write image file {:?}: {}", image_filename, e))?; + map_err(|e| anyhow!("Couldn't write image file {:?}: {}", image_filename, e))?; }, - Err(e) => return Err(format!("Couldn't load image: {}", e).into()), + Err(e) => return Err(anyhow!("Couldn't load image: {}", e)), }; Ok(()) } else { - Err("No image found in music file".into()) + Err(anyhow!("No image found in music file")) } } @@ -63,14 +60,19 @@ pub fn extract_first_image(music_filename: &Path, image_filename: &Path) -> Resu /// /// If the mp3 file's ID3 tags can't be parsed, the error will be propagated upwards. /// -pub fn remove_images(music_filename: &Path) -> Result<(), Box> { - let mut tag = id3::Tag::read_from_path(&music_filename). - map_err(|e| format!("Error reading music file {:?}: {}", music_filename, e))?; - +pub fn remove_images(music_filename: &Path) -> anyhow::Result<()> { + let mut tag = read_tag(music_filename)?; tag.remove("APIC"); tag.write_to_path(music_filename, id3::Version::Id3v23). - map_err(|e| format!("Error updating music file {:?}: {}", music_filename, e))?; + map_err(|e| anyhow!("Error updating music file {:?}: {}", music_filename, e))?; Ok(()) } + +fn read_tag(path: &Path) -> anyhow::Result { + id3::Tag::read_from_path(&path).or_else(|e| { + eprintln!("Warning: file metadata is corrupted, trying to read partial tag: {}", path.display()); + e.partial_tag.clone().ok_or_else(|| anyhow!("Error reading music file {:?}: {}", path, e)) + }) +} diff --git a/tests/fixtures/attempt_1_broken.mp3 b/tests/fixtures/attempt_1_broken.mp3 new file mode 100644 index 0000000..f4b2cbb Binary files /dev/null and b/tests/fixtures/attempt_1_broken.mp3 differ diff --git a/tests/test_processing.rs b/tests/test_processing.rs index 1f1f647..f001f78 100644 --- a/tests/test_processing.rs +++ b/tests/test_processing.rs @@ -75,6 +75,17 @@ fn test_successful_jpeg_image_embedding() { assert!(tag.pictures().count() > 0); } +#[test] +fn test_successful_jpeg_image_embedding_with_a_broken_file() { + let song = Fixture::copy("attempt_1_broken.mp3"); + let image = Fixture::copy("attempt_1.jpg"); + + embed_image(&song, &image).unwrap(); + + let tag = read_tag(&song); + assert!(tag.pictures().count() > 0); +} + #[test] fn test_successful_png_image_embedding() { let song = Fixture::copy("attempt_1_no_image.mp3"); @@ -122,6 +133,22 @@ fn test_removing_and_adding_an_image() { assert!(tag.pictures().count() > 0); } +#[test] +fn test_removing_and_adding_an_image_to_a_broken_file() { + let song = Fixture::copy("attempt_1_broken.mp3"); + let image = Fixture::copy("attempt_1.jpg"); + + remove_images(&song).unwrap(); + + let tag = read_tag(&song); + assert!(tag.pictures().count() == 0); + + embed_image(&song, &image).unwrap(); + + let tag = read_tag(&song); + assert!(tag.pictures().count() > 0); +} + #[test] fn test_extracting_a_jpg_image() { let song = Fixture::copy("attempt_1.mp3"); @@ -136,6 +163,16 @@ fn test_extracting_a_jpg_image() { assert!(image.exists()); } +#[test] +fn test_extracting_a_jpg_image_from_a_broken_file() { + let song = Fixture::copy("attempt_1_broken.mp3"); + let image = Fixture::blank("attempt_1.jpg"); + + extract_first_image(&song, &image).unwrap(); + + assert!(image.exists()); +} + #[test] fn test_extracting_a_png_image() { let song = Fixture::copy("attempt_1.mp3");