diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt index c0f8317a..7fde4094 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt @@ -1,6 +1,7 @@ package com.zarz.spotiflac import java.util.Locale +import java.util.regex.Pattern import kotlin.math.roundToInt /** @@ -373,6 +374,27 @@ internal object NativeFinalizationPolicy { ?: "" } + // Same separator set as the Rust writer and Dart's primaryArtistTagValue; + // Unicode classes keep \s aligned with their whitespace handling. + private val primaryArtistSeparator = Pattern.compile( + "\\s*[,;&]\\s*|\\s+x\\s+|\\s+(?:feat(?:uring)?|ft|with)\\.?(?:\\s+|$)", + Pattern.CASE_INSENSITIVE or Pattern.UNICODE_CHARACTER_CLASS, + ).toRegex() + + /** + * Artist tag value for [mode]. Only "primary" changes the value: the first + * credited artist for every format. Joined and split values pass through; + * the backend writer owns Vorbis splitting. + */ + fun artistTagValue(value: String, mode: String?): String { + if (!mode.orEmpty().trim().equals("primary", ignoreCase = true)) return value + val trimmed = value.trim() + return trimmed.split(primaryArtistSeparator) + .map(String::trim) + .firstOrNull(String::isNotEmpty) + ?: trimmed + } + private fun audioFormatForPath(filePath: String, fileName: String): String? { for (candidate in listOf(filePath, fileName)) { val lower = candidate.trim().lowercase(Locale.ROOT) diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerMedia.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerMedia.kt index 731336ac..578491ba 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerMedia.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerMedia.kt @@ -16,6 +16,7 @@ import com.antonkarpenko.ffmpegkit.ReturnCode import com.zarz.spotiflac.SafDownloadHandler.mimeTypeForExt import com.zarz.spotiflac.SafDownloadHandler.normalizeExt import com.zarz.spotiflac.NativeFinalizationPolicy.applyQualityVariantFilenameLabel +import com.zarz.spotiflac.NativeFinalizationPolicy.artistTagValue import com.zarz.spotiflac.NativeFinalizationPolicy.authoritativeAlbumArtist import com.zarz.spotiflac.NativeFinalizationPolicy.displayAudioQuality import com.zarz.spotiflac.NativeFinalizationPolicy.formatIndexTag @@ -314,6 +315,7 @@ internal fun NativeDownloadFinalizer.embedBasicMetadata(context: Context, path: val shouldEmbedLyrics = shouldResolveLyrics && NativeFinalizationPolicy.hasUsableLyricsContent(lyrics) && !lyrics.trim().equals("[instrumental:true]", ignoreCase = true) + val artistTagMode = input.request.optString("artist_tag_mode", "") // FLAC, MP3, Opus, and M4A all have backend tag writers that edit the // tag block atomically without an ffmpeg remux (which drops foreign // frames and rewrites the whole container). The backend answers @@ -326,7 +328,7 @@ internal fun NativeDownloadFinalizer.embedBasicMetadata(context: Context, path: .put("artist", artist) .put("album", album) .put("album_artist", albumArtist) - .put("artist_tag_mode", input.request.optString("artist_tag_mode", "")) + .put("artist_tag_mode", artistTagMode) .put("date", date) .put("isrc", isrc) .put("composer", composer) @@ -368,11 +370,13 @@ internal fun NativeDownloadFinalizer.embedBasicMetadata(context: Context, path: val isOpus = format == "opus" val coverFile = if (isM4a || isOpus) downloadCoverForMetadata(context, input) else null val labelKey = if (isM4a) "organization" else "label" + // The backend writer applies the artist mode itself; this FFmpeg fallback + // must store the same primary artist when that mode is selected. val metadataPairs = mutableListOf( "title" to title, - "artist" to artist, + "artist" to artistTagValue(artist, artistTagMode), "album" to album, - "album_artist" to albumArtist, + "album_artist" to artistTagValue(albumArtist, artistTagMode), "date" to date, "track" to trackNumber, "disc" to discNumber, diff --git a/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt index 29821c49..62d3ee15 100644 --- a/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt +++ b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt @@ -7,6 +7,39 @@ import org.junit.Assert.assertTrue import org.junit.Test class NativeFinalizationPolicyTest { + // Shared with the Rust and Dart primary-artist tests; keep in sync. + @Test + fun primaryArtistModeKeepsTheFirstCreditedArtist() { + for ((input, expected) in listOf( + "Calle 24, Chino Pacas" to "Calle 24", + "Calle 24 & Chino Pacas" to "Calle 24", + "Artist A; Artist B" to "Artist A", + "Artist A feat. Artist B" to "Artist A", + "Artist A Feat Artist B" to "Artist A", + "Artist A ft. Artist B" to "Artist A", + "Artist A featuring Artist B" to "Artist A", + "Artist A with Artist B" to "Artist A", + "Artist A x Artist B" to "Artist A", + "Artist A X Artist B" to "Artist A", + " , Artist A, Artist B" to "Artist A", + "Malcolm X" to "Malcolm X", + "Artist Without Fear" to "Artist Without Fear", + "Maxx" to "Maxx", + "AC/DC" to "AC/DC", + " Various Artists " to "Various Artists", + "" to "", + )) { + assertEquals(input, expected, NativeFinalizationPolicy.artistTagValue(input, " Primary ")) + } + // Joined and split values are left to the backend writer unchanged. + for (mode in listOf("joined", "split_vorbis", "", null)) { + assertEquals( + "Artist A, Artist B", + NativeFinalizationPolicy.artistTagValue("Artist A, Artist B", mode), + ) + } + } + @Test fun durationUsesDeclaredUnitsForShortTracksAndLongPerformances() { assertEquals(250L, NativeFinalizationPolicy.durationMilliseconds(250, 0)) diff --git a/rust_backend/crates/core/src/tags/write.rs b/rust_backend/crates/core/src/tags/write.rs index f3b45126..34f6c496 100644 --- a/rust_backend/crates/core/src/tags/write.rs +++ b/rust_backend/crates/core/src/tags/write.rs @@ -12,6 +12,7 @@ pub(super) use id3::cover as embedded_cover; use super::{CheckedReader, bytes, exact, pair, seek}; use crate::matching::{lowercase, uppercase}; use regex::Regex; +use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; use std::io::{BufReader, Cursor, Read, Seek, SeekFrom, Write}; use std::sync::LazyLock; @@ -50,6 +51,7 @@ pub fn rewrite_audio_tags( ) -> Result<(), String> { check()?; validate_fields(fields)?; + let fields = &*artist_mode_fields(fields); let mut source = BufReader::new(CheckedReader { reader: source, check, @@ -99,6 +101,7 @@ pub fn rewrite_flac_tags_if_changed( ) -> Result { check()?; validate_fields(fields)?; + let fields = &*artist_mode_fields(fields); // Read only metadata and the frame sync needed for validation. A buffered // reader here would also pull audio into memory on the no-op path. let mut source = CheckedReader { @@ -368,6 +371,8 @@ fn flac_header( if matches!(key.as_str(), "ARTIST" | "ALBUMARTIST") { let values = if mode.trim().eq_ignore_ascii_case("split_vorbis") { split_artists(value) + } else if primary_artist_mode(mode) { + vec![primary_artist(value)] } else { vec![value.clone()] }; @@ -644,6 +649,46 @@ fn metadata_fields(metadata: &super::AudioMetadata, fields: &Fields) -> Fields { result } +fn primary_artist_mode(mode: &str) -> bool { + mode.trim().eq_ignore_ascii_case("primary") +} + +/// The first credited artist, or the trimmed value when nothing separates it. +/// Dart and Kotlin share this exact separator set so every writer agrees; +/// Go-parity split mode keeps its own `split_artists` pattern. +fn primary_artist(value: &str) -> String { + static SEPARATOR: LazyLock = LazyLock::new(|| { + Regex::new(r"(?i)\s*[,;&]\s*|\s+x\s+|\s+(?:feat(?:uring)?|ft|with)\.?(?:\s+|$)").unwrap() + }); + let value = value.trim(); + SEPARATOR + .split(value) + .map(str::trim) + .find(|part| !part.is_empty()) + .unwrap_or(value) + .to_owned() +} + +/// Applies the "primary" artist tag mode to editor fields before any format +/// writer runs, so ID3, MP4, APE, RIFF and Vorbis all store the same name. +fn artist_mode_fields(fields: &Fields) -> Cow<'_, Fields> { + if !fields + .get("artist_tag_mode") + .is_some_and(|mode| primary_artist_mode(mode)) + { + return Cow::Borrowed(fields); + } + let mut fields = fields.clone(); + for key in ["artist", "album_artist"] { + if let Some(value) = fields.get_mut(key) + && !value.trim().is_empty() + { + *value = primary_artist(value); + } + } + Cow::Owned(fields) +} + fn split_artists(value: &str) -> Vec { static SPLIT: LazyLock = LazyLock::new(|| { Regex::new(r"(?-u:\s*(?:,|&|\bx\b)\s*|\s+\b(?:feat(?:uring)?|ft|with)\.?\s*)").unwrap() @@ -783,6 +828,32 @@ mod tests { } } + /// Shared with the Dart and Kotlin primary-artist tests; keep in sync. + #[test] + fn primary_artist_uses_the_shared_separator_set() { + for (input, expected) in [ + ("Calle 24, Chino Pacas", "Calle 24"), + ("Calle 24 & Chino Pacas", "Calle 24"), + ("Artist A; Artist B", "Artist A"), + ("Artist A feat. Artist B", "Artist A"), + ("Artist A Feat Artist B", "Artist A"), + ("Artist A ft. Artist B", "Artist A"), + ("Artist A featuring Artist B", "Artist A"), + ("Artist A with Artist B", "Artist A"), + ("Artist A x Artist B", "Artist A"), + ("Artist A X Artist B", "Artist A"), + (" , Artist A, Artist B", "Artist A"), + ("Malcolm X", "Malcolm X"), + ("Artist Without Fear", "Artist Without Fear"), + ("Maxx", "Maxx"), + ("AC/DC", "AC/DC"), + (" Various Artists ", "Various Artists"), + ("", ""), + ] { + assert_eq!(primary_artist(input), expected, "{input:?}"); + } + } + fn editable_flac() -> (Vec, usize, Fields) { let mut comments = Vec::new(); put_string(&mut comments, b"SpotiFLAC"); diff --git a/rust_backend/crates/core/tests/artist_tag_mode.rs b/rust_backend/crates/core/tests/artist_tag_mode.rs new file mode 100644 index 00000000..872b2ffa --- /dev/null +++ b/rust_backend/crates/core/tests/artist_tag_mode.rs @@ -0,0 +1,140 @@ +use spotiflac_core::tags::{embed_flac_metadata, read_audio_tags, rewrite_audio_tags}; +use std::collections::BTreeMap; +use std::io::Cursor; + +const PAYLOAD: &[u8] = b"\xff\xf8\x12\x34unchanged audio payload"; +const CREDITS: &str = "Artist A, Artist B & Artist C"; + +fn atom(kind: &[u8; 4], body: &[u8]) -> Vec { + [ + ((body.len() + 8) as u32).to_be_bytes().as_slice(), + kind, + body, + ] + .concat() +} + +fn ogg_page(packet: &[u8], sequence: u32, flags: u8) -> Vec { + let mut header = vec![0; 27]; + header[..4].copy_from_slice(b"OggS"); + header[5] = flags; + header[14..18].copy_from_slice(&1_u32.to_le_bytes()); + header[18..22].copy_from_slice(&sequence.to_le_bytes()); + header[26] = 1; + header.push(packet.len() as u8); + header.extend(packet); + header +} + +fn source(format: &str) -> Vec { + match format { + "flac" => [b"fLaC\x80\0\0\x22".as_slice(), &[0; 34], PAYLOAD].concat(), + "mp3" | "ape" => PAYLOAD.to_vec(), + "m4a" => [ + atom(b"ftyp", b"M4A \0\0\0\0"), + atom(b"moov", &[]), + atom(b"mdat", PAYLOAD), + ] + .concat(), + "opus" => [ + ogg_page(b"OpusHead\x01\x02\0\0\x80\xbb\0\0\0\0\0", 0, 2), + ogg_page(b"OpusTags\0\0\0\0\0\0\0\0", 1, 0), + ogg_page(PAYLOAD, 2, 4), + ] + .concat(), + "wav" | "aiff" => { + let aiff = format == "aiff"; + let mut body = if aiff { b"AIFFSSND" } else { b"WAVEdata" }.to_vec(); + let length = PAYLOAD.len() as u32; + body.extend(if aiff { + length.to_be_bytes() + } else { + length.to_le_bytes() + }); + body.extend(PAYLOAD); + if length % 2 == 1 { + body.push(0); + } + let length = body.len() as u32; + [ + if aiff { b"FORM" } else { b"RIFF" }.as_slice(), + &if aiff { + length.to_be_bytes() + } else { + length.to_le_bytes() + }, + &body, + ] + .concat() + } + _ => unreachable!(), + } +} + +fn tagged(format: &str, mode: &str) -> (String, String) { + let fields = BTreeMap::from([ + ("title".into(), "Example".into()), + ("artist".into(), CREDITS.into()), + ("album_artist".into(), CREDITS.into()), + ("artist_tag_mode".into(), mode.into()), + ]); + let mut output = Vec::new(); + rewrite_audio_tags( + &mut Cursor::new(source(format)), + &mut output, + format, + &fields, + None, + &|| Ok(()), + ) + .unwrap(); + let tags = read_audio_tags(&mut Cursor::new(&output), format, &|| Ok(())).unwrap(); + (tags.artist, tags.album_artist) +} + +#[test] +fn primary_mode_keeps_only_the_first_artist_in_every_native_writer() { + for format in ["flac", "mp3", "m4a", "opus", "ape", "wav", "aiff"] { + assert_eq!( + tagged(format, "primary"), + ("Artist A".into(), "Artist A".into()), + "{format}" + ); + // Mode values are matched like split_vorbis: trimmed, any case. + assert_eq!(tagged(format, " Primary ").0, "Artist A", "{format}"); + assert_eq!( + tagged(format, "joined"), + (CREDITS.into(), CREDITS.into()), + "{format}" + ); + } +} + +#[test] +fn primary_mode_applies_to_flac_download_embedding() { + let fields = BTreeMap::from([ + ("TITLE".into(), "Example".into()), + ("ARTIST".into(), "Calle 24, Chino Pacas".into()), + ("ALBUMARTIST".into(), "Calle 24 feat. Chino Pacas".into()), + ]); + let mut output = Vec::new(); + embed_flac_metadata( + &mut Cursor::new(source("flac")), + &mut output, + &fields, + "primary", + None, + &|| Ok(()), + ) + .unwrap(); + let tags = read_audio_tags(&mut Cursor::new(&output), "flac", &|| Ok(())).unwrap(); + assert_eq!(tags.artist, "Calle 24"); + assert_eq!(tags.album_artist, "Calle 24"); + assert!(output.ends_with(PAYLOAD)); + let comments = String::from_utf8_lossy(&output); + assert_eq!( + comments.matches("ARTIST=").count(), + 2, + "one ARTIST, one ALBUMARTIST" + ); +}