mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-10-02 22:26:52 +02:00
feat(metadata): write primary-artist tags in native writers
Native tag writers accept artist_tag_mode=primary and keep only the first credited artist in ARTIST and ALBUMARTIST for every format: FLAC download embedding, FLAC/Opus edits, and the ID3, MP4, APE and RIFF writers. The Android finalizer's FFmpeg fallback applies the same rule. Rust and Kotlin share one separator set (comma, semicolon, ampersand, a spaced x, feat/ft/featuring/with) and test the same fixtures. The Go-parity split_vorbis pattern is unchanged. Refs #618.
This commit is contained in:
1 parent
fe30871e36
commit
41340a4a21
5 files changed
+273
-3
No files matched your search
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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<bool, String> {
|
||||
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<Regex> = 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<String> {
|
||||
static SPLIT: LazyLock<Regex> = 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<u8>, usize, Fields) {
|
||||
let mut comments = Vec::new();
|
||||
put_string(&mut comments, b"SpotiFLAC");
|
||||
|
||||
@@ -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<u8> {
|
||||
[
|
||||
((body.len() + 8) as u32).to_be_bytes().as_slice(),
|
||||
kind,
|
||||
body,
|
||||
]
|
||||
.concat()
|
||||
}
|
||||
|
||||
fn ogg_page(packet: &[u8], sequence: u32, flags: u8) -> Vec<u8> {
|
||||
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<u8> {
|
||||
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"
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user