diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt index a1fa9283..0247b723 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt @@ -1068,7 +1068,14 @@ object NativeDownloadFinalizer { .put("title", trackString(input, "name", input.request.optString("track_name", ""))) .put("artist", trackString(input, "artistName", input.request.optString("artist_name", ""))) .put("album", trackString(input, "albumName", input.request.optString("album_name", ""))) - .put("album_artist", trackString(input, "albumArtist", input.request.optString("album_artist", ""))) + .put( + "album_artist", + NativeFinalizationPolicy.authoritativeAlbumArtist( + requestValue = requestString(input, "album_artist"), + trackValue = trackString(input, "albumArtist", ""), + providerResultValue = resultString(input, "album_artist"), + ), + ) .put("track_number", trackInt(input, "trackNumber", input.request.optInt("track_number", 0))) .put("disc_number", trackInt(input, "discNumber", input.request.optInt("disc_number", 0))) .put("isrc", trackString(input, "isrc", input.request.optString("isrc", ""))) @@ -1229,7 +1236,11 @@ object NativeDownloadFinalizer { val albumId = trackString(input, "albumId", "") if (albumId.isNotBlank()) return "id:$albumId" val albumName = trackString(input, "albumName", input.request.optString("album_name", "")) - val albumArtist = trackString(input, "albumArtist", input.request.optString("album_artist", "")) + val albumArtist = NativeFinalizationPolicy.authoritativeAlbumArtist( + requestValue = requestString(input, "album_artist"), + trackValue = trackString(input, "albumArtist", ""), + providerResultValue = resultString(input, "album_artist"), + ) return "name:$albumName|$albumArtist" } @@ -1248,7 +1259,16 @@ object NativeDownloadFinalizer { values.put("track_name", result.optString("title", "").ifBlank { trackString(input, "name", input.request.optString("track_name", "")) }) values.put("artist_name", result.optString("artist", "").ifBlank { trackString(input, "artistName", input.request.optString("artist_name", "")) }) values.put("album_name", result.optString("album", "").ifBlank { trackString(input, "albumName", input.request.optString("album_name", "")) }) - values.put("album_artist", normalizeOptional(resultString(input, "album_artist").ifBlank { trackString(input, "albumArtist", requestString(input, "album_artist")) })) + values.put( + "album_artist", + normalizeOptional( + NativeFinalizationPolicy.authoritativeAlbumArtist( + requestValue = requestString(input, "album_artist"), + trackValue = trackString(input, "albumArtist", ""), + providerResultValue = resultString(input, "album_artist"), + ), + ), + ) values.put("cover_url", normalizeOptional(metadataCoverUrl(input).ifBlank { resultString(input, "cover_url") })) values.put("file_path", state.filePath) values.put("storage_mode", input.request.optString("storage_mode", "app")) 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 1c930178..77b6323b 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt @@ -277,6 +277,22 @@ internal object NativeFinalizationPolicy { return if (total > 0) "$number/$total" else number.toString() } + /** + * The app request contains the album artist after batch normalization and + * user metadata filters. Provider results are only a fallback: preferring + * them would reintroduce per-track collaboration credits while finalizing. + */ + fun authoritativeAlbumArtist( + requestValue: String?, + trackValue: String?, + providerResultValue: String?, + ): String { + return normalizeOptional(requestValue) + ?: normalizeOptional(trackValue) + ?: normalizeOptional(providerResultValue) + ?: "" + } + 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 acf4b942..1a4170c0 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.authoritativeAlbumArtist import com.zarz.spotiflac.NativeFinalizationPolicy.displayAudioQuality import com.zarz.spotiflac.NativeFinalizationPolicy.formatIndexTag import com.zarz.spotiflac.NativeFinalizationPolicy.isLosslessAudioCodec @@ -257,9 +258,11 @@ internal fun NativeDownloadFinalizer.embedBasicMetadata(context: Context, path: val album = resultString(input, "album").ifBlank { trackString(input, "albumName", requestString(input, "album_name")) } - val albumArtist = resultString(input, "album_artist").ifBlank { - trackString(input, "albumArtist", requestString(input, "album_artist")) - } + val albumArtist = authoritativeAlbumArtist( + requestValue = requestString(input, "album_artist"), + trackValue = trackString(input, "albumArtist", ""), + providerResultValue = resultString(input, "album_artist"), + ) val date = resultString(input, "release_date").ifBlank { resultString(input, "date").ifBlank { trackString(input, "releaseDate", requestString(input, "release_date")) 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 cf2bc816..8808e8ce 100644 --- a/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt +++ b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt @@ -96,6 +96,26 @@ class NativeFinalizationPolicyTest { ) } + @Test + fun normalizedRequestAlbumArtistOverridesPerTrackProviderCredits() { + assertEquals( + "Falling In Reverse", + NativeFinalizationPolicy.authoritativeAlbumArtist( + requestValue = "Falling In Reverse", + trackValue = "Falling In Reverse, Jelly Roll", + providerResultValue = "Falling In Reverse, Jelly Roll", + ), + ) + assertEquals( + "Provider Album Artist", + NativeFinalizationPolicy.authoritativeAlbumArtist( + requestValue = "", + trackValue = null, + providerResultValue = "Provider Album Artist", + ), + ) + } + @Test fun displayQualityUsesMeasuredLosslessSpecifications() { assertEquals( diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 71105155..765744c0 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -129,6 +129,115 @@ bool isStorageWriteFailure({String? errorType, String? errorMessage}) { message.contains('failed to copy extension output to saf'); } +/// Makes the album-artist credit stable for tracks from the same album. +/// +/// Some metadata providers expose every `MAIN` track artist as the album +/// artist. On collaboration tracks that turns one album into values such as +/// "Artist", "Artist, Guest A", and "Artist, Guest B". Music libraries then +/// split the files into separate albums. Keep the complete track artist credit +/// intact, but reduce inconsistent album-artist credits to the artist names +/// shared by every track in that album. +List normalizeBatchAlbumArtists(List tracks) { + if (tracks.length < 2) return tracks; + + final albumGroups = >{}; + for (var index = 0; index < tracks.length; index++) { + final key = _batchAlbumIdentity(tracks[index]); + if (key == null) continue; + albumGroups.putIfAbsent(key, () => []).add(index); + } + + List? normalized; + for (final indices in albumGroups.values) { + if (indices.length < 2) continue; + final albumTracks = [for (final index in indices) tracks[index]]; + final canonicalArtist = _sharedBatchAlbumArtist(albumTracks); + if (canonicalArtist == null) continue; + + for (final index in indices) { + final currentArtist = normalizeOptionalString(tracks[index].albumArtist); + if (currentArtist == canonicalArtist) continue; + normalized ??= List.of(tracks); + normalized[index] = tracks[index].copyWith( + albumArtist: canonicalArtist, + ); + } + } + + return normalized ?? tracks; +} + +String? _batchAlbumIdentity(Track track) { + final source = normalizeOptionalString(track.source)?.toLowerCase() ?? ''; + final albumId = normalizeOptionalString(track.albumId)?.toLowerCase(); + if (albumId != null) return '$source|id:$albumId'; + + final albumName = normalizeOptionalString(track.albumName)?.toLowerCase(); + if (albumName == null) return null; + + final cover = normalizeOptionalString( + normalizeCoverReference(track.coverUrl), + )?.toLowerCase(); + if (cover != null) return '$source|cover:$albumName|$cover'; + + final releaseDate = normalizeOptionalString(track.releaseDate) ?? ''; + final totalTracks = track.totalTracks ?? 0; + final primaryArtist = primaryArtistName( + track.artistName, + albumArtist: track.albumArtist, + ).trim().toLowerCase(); + return '$source|meta:$albumName|$releaseDate|$totalTracks|$primaryArtist'; +} + +String? _sharedBatchAlbumArtist(List tracks) { + final albumArtists = tracks + .map((track) => normalizeOptionalString(track.albumArtist)) + .toList(growable: false); + final firstAlbumArtist = albumArtists.first; + if (firstAlbumArtist != null && + albumArtists.every( + (artist) => artist?.toLowerCase() == firstAlbumArtist.toLowerCase(), + )) { + return firstAlbumArtist; + } + + final credits = >[]; + for (var index = 0; index < tracks.length; index++) { + final rawCredit = albumArtists[index] ?? tracks[index].artistName; + final names = splitArtistNames(rawCredit); + if (names.isEmpty) return null; + credits.add(names); + } + + final sharedKeys = credits.first + .map((name) => name.toLowerCase()) + .toSet(); + for (final credit in credits.skip(1)) { + final keys = credit.map((name) => name.toLowerCase()).toSet(); + sharedKeys.removeWhere((name) => !keys.contains(name)); + } + if (sharedKeys.isEmpty) return null; + + // Preserve a provider's original separator whenever one of its credits is + // already exactly the shared album credit. + for (final rawCredit in albumArtists.whereType()) { + final keys = splitArtistNames( + rawCredit, + ).map((name) => name.toLowerCase()).toSet(); + if (keys.length == sharedKeys.length && keys.containsAll(sharedKeys)) { + return rawCredit; + } + } + + final displayNames = []; + final seen = {}; + for (final name in credits.first) { + final key = name.toLowerCase(); + if (sharedKeys.contains(key) && seen.add(key)) displayNames.add(name); + } + return displayNames.isEmpty ? null : displayNames.join(', '); +} + final _invalidFolderChars = RegExp(r'[<>:"/\\|?*]'); final _trimDotsAndSpacesRegex = RegExp(r'^[. ]+|[. ]+$'); final _trimUnderscoresAndSpacesRegex = RegExp(r'^[_ ]+|[_ ]+$'); @@ -748,8 +857,9 @@ class DownloadQueueNotifier extends Notifier { final takenIds = state.items.map((item) => item.id).toSet(); final shouldAssignPlaylistPositions = playlistName != null && playlistName.trim().isNotEmpty; - final fromBatch = tracks.length > 1; - final newItems = tracks.asMap().entries.map((entry) { + final normalizedTracks = normalizeBatchAlbumArtists(tracks); + final fromBatch = normalizedTracks.length > 1; + final newItems = normalizedTracks.asMap().entries.map((entry) { final track = entry.value; final index = entry.key; final explicitPosition = diff --git a/test/batch_album_artist_normalization_test.dart b/test/batch_album_artist_normalization_test.dart new file mode 100644 index 00000000..f33a7de8 --- /dev/null +++ b/test/batch_album_artist_normalization_test.dart @@ -0,0 +1,97 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/models/track.dart'; +import 'package:spotiflac_android/providers/download_queue_provider.dart'; + +Track albumTrack({ + required String id, + required String artist, + required String albumArtist, + String albumId = 'album:1', + String albumName = 'Popular Monster', +}) { + return Track( + id: id, + name: 'Track $id', + artistName: artist, + albumName: albumName, + albumArtist: albumArtist, + albumId: albumId, + duration: 180, + source: 'test-provider', + ); +} + +void main() { + test('uses the shared album artist across collaboration tracks', () { + final tracks = [ + albumTrack( + id: '1', + artist: 'Falling In Reverse', + albumArtist: 'Falling In Reverse', + ), + albumTrack( + id: '2', + artist: 'Falling In Reverse, Jelly Roll', + albumArtist: 'Falling In Reverse, Jelly Roll', + ), + albumTrack( + id: '3', + artist: 'Falling In Reverse, Marilyn Manson', + albumArtist: 'Falling In Reverse, Marilyn Manson', + ), + ]; + + final normalized = normalizeBatchAlbumArtists(tracks); + + expect( + normalized.map((track) => track.albumArtist), + everyElement('Falling In Reverse'), + ); + expect( + normalized.map((track) => track.artistName), + [ + 'Falling In Reverse', + 'Falling In Reverse, Jelly Roll', + 'Falling In Reverse, Marilyn Manson', + ], + ); + }); + + test('preserves a stable joint album credit', () { + final tracks = [ + albumTrack(id: '1', artist: 'Artist A', albumArtist: 'Artist A & B'), + albumTrack(id: '2', artist: 'Artist B', albumArtist: 'Artist A & B'), + ]; + + final normalized = normalizeBatchAlbumArtists(tracks); + + expect( + normalized.map((track) => track.albumArtist), + everyElement('Artist A & B'), + ); + }); + + test('does not combine credits from different albums in one batch', () { + final tracks = [ + albumTrack( + id: '1', + artist: 'Artist A, Guest', + albumArtist: 'Artist A, Guest', + albumId: 'album:a', + albumName: 'Shared title', + ), + albumTrack( + id: '2', + artist: 'Artist B, Guest', + albumArtist: 'Artist B, Guest', + albumId: 'album:b', + albumName: 'Shared title', + ), + ]; + + final normalized = normalizeBatchAlbumArtists(tracks); + + expect(normalized[0].albumArtist, 'Artist A, Guest'); + expect(normalized[1].albumArtist, 'Artist B, Guest'); + }); +}