mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-08-27 13:22:49 +02:00
fix(metadata): keep collaboration albums grouped
This commit is contained in:
@@ -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"))
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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"))
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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<Track> normalizeBatchAlbumArtists(List<Track> tracks) {
|
||||
if (tracks.length < 2) return tracks;
|
||||
|
||||
final albumGroups = <String, List<int>>{};
|
||||
for (var index = 0; index < tracks.length; index++) {
|
||||
final key = _batchAlbumIdentity(tracks[index]);
|
||||
if (key == null) continue;
|
||||
albumGroups.putIfAbsent(key, () => <int>[]).add(index);
|
||||
}
|
||||
|
||||
List<Track>? 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<Track>.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<Track> 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 = <List<String>>[];
|
||||
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<String>()) {
|
||||
final keys = splitArtistNames(
|
||||
rawCredit,
|
||||
).map((name) => name.toLowerCase()).toSet();
|
||||
if (keys.length == sharedKeys.length && keys.containsAll(sharedKeys)) {
|
||||
return rawCredit;
|
||||
}
|
||||
}
|
||||
|
||||
final displayNames = <String>[];
|
||||
final seen = <String>{};
|
||||
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<DownloadQueueState> {
|
||||
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 =
|
||||
|
||||
@@ -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 = <Track>[
|
||||
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),
|
||||
<String>[
|
||||
'Falling In Reverse',
|
||||
'Falling In Reverse, Jelly Roll',
|
||||
'Falling In Reverse, Marilyn Manson',
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
test('preserves a stable joint album credit', () {
|
||||
final tracks = <Track>[
|
||||
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 = <Track>[
|
||||
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');
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user