mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-08-02 09:08:35 +02:00
fix(download): complete quality variants reliably
This commit is contained in:
@@ -157,6 +157,24 @@ internal object NativeFinalizationPolicy {
|
||||
return "$stem - $qualityLabel$extension"
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the user-facing name that a deferred SAF download was assigned
|
||||
* before its audio was materialized in the app cache. Container and
|
||||
* decryption passes may replace [currentFileName] with a temporary
|
||||
* `native_saf_work_*` name, which must never become the published name.
|
||||
*/
|
||||
fun logicalOutputFileName(
|
||||
deferredSafPublish: Boolean,
|
||||
resultSafFileName: String?,
|
||||
requestSafFileName: String?,
|
||||
currentFileName: String,
|
||||
): String {
|
||||
if (!deferredSafPublish) return currentFileName
|
||||
return normalizeOptional(resultSafFileName)
|
||||
?: normalizeOptional(requestSafFileName)
|
||||
?: currentFileName
|
||||
}
|
||||
|
||||
fun resolvePreferredDecryptionExtension(
|
||||
inputPath: String,
|
||||
requested: String,
|
||||
|
||||
@@ -20,6 +20,7 @@ import com.zarz.spotiflac.NativeFinalizationPolicy.displayAudioQuality
|
||||
import com.zarz.spotiflac.NativeFinalizationPolicy.formatIndexTag
|
||||
import com.zarz.spotiflac.NativeFinalizationPolicy.isLosslessAudioCodec
|
||||
import com.zarz.spotiflac.NativeFinalizationPolicy.isLossyAudioCodec
|
||||
import com.zarz.spotiflac.NativeFinalizationPolicy.logicalOutputFileName
|
||||
import com.zarz.spotiflac.NativeFinalizationPolicy.normalizeAudioCodec
|
||||
import com.zarz.spotiflac.NativeFinalizationPolicy.resolvePreferredDecryptionExtension
|
||||
import gobackend.Gobackend
|
||||
@@ -61,12 +62,18 @@ internal fun NativeDownloadFinalizer.finalizeQualityVariantFilename(
|
||||
return
|
||||
}
|
||||
|
||||
val logicalFileName = logicalOutputFileName(
|
||||
deferredSafPublish = isDeferredSafPublish(input),
|
||||
resultSafFileName = input.result.optString("saf_final_file_name", ""),
|
||||
requestSafFileName = input.request.optString("saf_file_name", ""),
|
||||
currentFileName = state.fileName,
|
||||
)
|
||||
val preferredName = applyQualityVariantFilenameLabel(
|
||||
fileName = state.fileName,
|
||||
fileName = logicalFileName,
|
||||
stagingLabel = stagingLabel,
|
||||
qualityLabel = qualityLabel,
|
||||
)
|
||||
if (preferredName == state.fileName) return
|
||||
if (preferredName == logicalFileName && preferredName == state.fileName) return
|
||||
input.result.put("quality_variant_file_name", preferredName)
|
||||
if (isDeferredSafPublish(input)) {
|
||||
state.fileName = preferredName
|
||||
|
||||
@@ -150,6 +150,38 @@ class NativeFinalizationPolicyTest {
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun deferredSafNamingNeverPublishesTheNativeCacheName() {
|
||||
val logicalName = NativeFinalizationPolicy.logicalOutputFileName(
|
||||
deferredSafPublish = true,
|
||||
resultSafFileName = "Sunidhi Chauhan - Aisa Jadoo - qv_ab12cd34.flac",
|
||||
requestSafFileName = "fallback.flac",
|
||||
currentFileName = "native_saf_work_603020549715656640.m4a",
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
"Sunidhi Chauhan - Aisa Jadoo - 16bit-44.1kHz.flac",
|
||||
NativeFinalizationPolicy.applyQualityVariantFilenameLabel(
|
||||
fileName = logicalName,
|
||||
stagingLabel = "qv_ab12cd34",
|
||||
qualityLabel = "16bit-44.1kHz",
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun nonSafNamingStillFollowsTheCurrentConvertedFile() {
|
||||
assertEquals(
|
||||
"converted.flac",
|
||||
NativeFinalizationPolicy.logicalOutputFileName(
|
||||
deferredSafPublish = false,
|
||||
resultSafFileName = "ignored.flac",
|
||||
requestSafFileName = "ignored-too.flac",
|
||||
currentFileName = "converted.flac",
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun decryptionExtensionAndIndexTagsHaveStableFallbacks() {
|
||||
assertEquals(
|
||||
|
||||
@@ -74,6 +74,15 @@ Future<void> persistBeforePublishingDownloadCompletion({
|
||||
publish();
|
||||
}
|
||||
|
||||
/// Keeps native-worker finalization visually distinct from completion.
|
||||
/// Download bytes may reach 100% before SAF publishing, metadata persistence,
|
||||
/// and queue reconciliation have finished.
|
||||
double nativeWorkerFinalizingProgress(double progress) {
|
||||
final normalized = progress.clamp(0.0, 1.0).toDouble();
|
||||
if (normalized <= 0) return 0.95;
|
||||
return min(normalized, 0.99);
|
||||
}
|
||||
|
||||
final _invalidFolderChars = RegExp(r'[<>:"/\\|?*]');
|
||||
final _trimDotsAndSpacesRegex = RegExp(r'^[. ]+|[. ]+$');
|
||||
final _trimUnderscoresAndSpacesRegex = RegExp(r'^[_ ]+|[_ ]+$');
|
||||
@@ -1488,7 +1497,6 @@ class DownloadQueueNotifier extends Notifier<DownloadQueueState> {
|
||||
_verificationRetryGuard.retainItems(remainingIds);
|
||||
_rateLimitRetriedItemIds.removeWhere((id) => !remainingIds.contains(id));
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
final downloadQueueProvider =
|
||||
|
||||
@@ -803,7 +803,7 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier {
|
||||
updateItemStatus(
|
||||
itemId,
|
||||
DownloadStatus.finalizing,
|
||||
progress: progress <= 0 ? 0.95 : progress,
|
||||
progress: nativeWorkerFinalizingProgress(progress),
|
||||
);
|
||||
continue;
|
||||
}
|
||||
@@ -811,12 +811,34 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier {
|
||||
if (status == 'completed') {
|
||||
final result = itemSnapshot['result'];
|
||||
if (result is Map) {
|
||||
reconciledIds.add(itemId);
|
||||
await _completeAndroidNativeWorkerItem(
|
||||
context,
|
||||
Map<String, dynamic>.from(result),
|
||||
settings,
|
||||
);
|
||||
try {
|
||||
await _completeAndroidNativeWorkerItem(
|
||||
context,
|
||||
Map<String, dynamic>.from(result),
|
||||
settings,
|
||||
);
|
||||
reconciledIds.add(itemId);
|
||||
} catch (e, stack) {
|
||||
// A native output can be complete while Library persistence or
|
||||
// Dart-side adoption fails. Do not leave the queue permanently at
|
||||
// 100%: surface the reconciliation failure and keep processing
|
||||
// the rest of the worker batch.
|
||||
_log.e(
|
||||
'Failed to reconcile completed native worker item $itemId: $e',
|
||||
e,
|
||||
stack,
|
||||
);
|
||||
if (_findItemById(itemId) != null) {
|
||||
updateItemStatus(
|
||||
itemId,
|
||||
DownloadStatus.failed,
|
||||
error: 'Downloaded file could not be added to the Library: $e',
|
||||
errorType: DownloadErrorType.unknown,
|
||||
);
|
||||
_failedInSession++;
|
||||
}
|
||||
reconciledIds.add(itemId);
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -537,7 +537,12 @@ class _AlbumScreenState extends ConsumerState<AlbumScreen>
|
||||
child: TrackListTile(
|
||||
track: track,
|
||||
isInHistory: isInHistory,
|
||||
onDownload: () => _downloadTrack(context, track),
|
||||
onDownload: ({bool forceQualityPicker = false}) =>
|
||||
_downloadTrack(
|
||||
context,
|
||||
track,
|
||||
forceQualityPicker: forceQualityPicker,
|
||||
),
|
||||
clickableArtist: true,
|
||||
leading: SizedBox(
|
||||
width: 32,
|
||||
@@ -559,12 +564,17 @@ class _AlbumScreenState extends ConsumerState<AlbumScreen>
|
||||
);
|
||||
}
|
||||
|
||||
void _downloadTrack(BuildContext context, Track track) {
|
||||
void _downloadTrack(
|
||||
BuildContext context,
|
||||
Track track, {
|
||||
bool forceQualityPicker = false,
|
||||
}) {
|
||||
downloadSingleTrack(
|
||||
context,
|
||||
ref,
|
||||
track,
|
||||
recommendedService: _recommendedDownloadService(),
|
||||
forceQualityPicker: forceQualityPicker,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -423,7 +423,12 @@ extension _ArtistScreenSections on _ArtistScreenState {
|
||||
final isQueued = queueItem != null;
|
||||
|
||||
return InkWell(
|
||||
onTap: () => _handlePopularTrackTap(track, isQueued: isQueued),
|
||||
onTap: () => _handlePopularTrackTap(
|
||||
track,
|
||||
isQueued: isQueued,
|
||||
isInHistory: isInHistory,
|
||||
isInLocalLibrary: isInLocalLibrary,
|
||||
),
|
||||
onLongPress: () => TrackCollectionQuickActions.showTrackOptionsSheet(
|
||||
context,
|
||||
ref,
|
||||
@@ -531,9 +536,20 @@ extension _ArtistScreenSections on _ArtistScreenState {
|
||||
);
|
||||
}
|
||||
|
||||
void _handlePopularTrackTap(Track track, {required bool isQueued}) async {
|
||||
void _handlePopularTrackTap(
|
||||
Track track, {
|
||||
required bool isQueued,
|
||||
required bool isInHistory,
|
||||
required bool isInLocalLibrary,
|
||||
}) async {
|
||||
if (isQueued) return;
|
||||
|
||||
final settings = ref.read(settingsProvider);
|
||||
if (settings.allowQualityVariants && (isInHistory || isInLocalLibrary)) {
|
||||
_downloadTrack(track);
|
||||
return;
|
||||
}
|
||||
|
||||
final playedLocal = await playLocalIfAvailable(context, ref, track);
|
||||
if (playedLocal) {
|
||||
return;
|
||||
|
||||
@@ -323,8 +323,13 @@ class _PlaylistScreenState extends ConsumerState<PlaylistScreen>
|
||||
child: TrackListTile(
|
||||
track: track,
|
||||
isInHistory: isInHistory,
|
||||
onDownload: () =>
|
||||
_downloadTrack(context, track, playlistPosition: index + 1),
|
||||
onDownload: ({bool forceQualityPicker = false}) =>
|
||||
_downloadTrack(
|
||||
context,
|
||||
track,
|
||||
playlistPosition: index + 1,
|
||||
forceQualityPicker: forceQualityPicker,
|
||||
),
|
||||
leading: track.coverUrl != null
|
||||
? CachedCoverImage(
|
||||
imageUrl: track.coverUrl!,
|
||||
@@ -358,6 +363,7 @@ class _PlaylistScreenState extends ConsumerState<PlaylistScreen>
|
||||
BuildContext context,
|
||||
Track track, {
|
||||
int? playlistPosition,
|
||||
bool forceQualityPicker = false,
|
||||
}) {
|
||||
downloadSingleTrack(
|
||||
context,
|
||||
@@ -366,6 +372,7 @@ class _PlaylistScreenState extends ConsumerState<PlaylistScreen>
|
||||
recommendedService: _recommendedDownloadService(),
|
||||
playlistName: _playlistName,
|
||||
playlistPosition: playlistPosition,
|
||||
forceQualityPicker: forceQualityPicker,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -11,14 +11,15 @@ import 'package:spotiflac_android/widgets/in_library_badge.dart';
|
||||
import 'package:spotiflac_android/widgets/preview_button.dart';
|
||||
import 'package:spotiflac_android/widgets/track_collection_quick_actions.dart';
|
||||
|
||||
/// Track row shared by the album and playlist screens. Tap plays the local
|
||||
/// copy when one exists and otherwise triggers [onDownload]; long-press opens
|
||||
/// the track options sheet. Callers supply [leading] (track number, cover
|
||||
/// art, ...) and choose whether the artist name links to the artist screen.
|
||||
/// Track row shared by the album and playlist screens. Tap offers another
|
||||
/// download when quality variants are enabled, otherwise it plays the local
|
||||
/// copy when one exists and falls back to [onDownload]. Long-press opens the
|
||||
/// track options sheet. Callers supply [leading] (track number, cover art,
|
||||
/// ...) and choose whether the artist name links to the artist screen.
|
||||
class TrackListTile extends ConsumerWidget {
|
||||
final Track track;
|
||||
final bool isInHistory;
|
||||
final VoidCallback onDownload;
|
||||
final void Function({bool forceQualityPicker}) onDownload;
|
||||
final Widget leading;
|
||||
final bool clickableArtist;
|
||||
|
||||
@@ -118,7 +119,12 @@ class TrackListTile extends ConsumerWidget {
|
||||
TrackCollectionQuickActions(track: track),
|
||||
],
|
||||
),
|
||||
onTap: () => _handleTap(context, ref, isQueued: isQueued),
|
||||
onTap: () => _handleTap(
|
||||
context,
|
||||
ref,
|
||||
isQueued: isQueued,
|
||||
isInLocalLibrary: isInLocalLibrary,
|
||||
),
|
||||
onLongPress: () => TrackCollectionQuickActions.showTrackOptionsSheet(
|
||||
context,
|
||||
ref,
|
||||
@@ -133,9 +139,16 @@ class TrackListTile extends ConsumerWidget {
|
||||
BuildContext context,
|
||||
WidgetRef ref, {
|
||||
required bool isQueued,
|
||||
required bool isInLocalLibrary,
|
||||
}) async {
|
||||
if (isQueued) return;
|
||||
|
||||
final settings = ref.read(settingsProvider);
|
||||
if (settings.allowQualityVariants && (isInHistory || isInLocalLibrary)) {
|
||||
onDownload(forceQualityPicker: true);
|
||||
return;
|
||||
}
|
||||
|
||||
final playedLocal = await playLocalIfAvailable(context, ref, track);
|
||||
if (playedLocal) {
|
||||
return;
|
||||
|
||||
@@ -19,6 +19,14 @@ import 'package:spotiflac_android/utils/path_match_keys.dart';
|
||||
import 'package:spotiflac_android/utils/string_utils.dart';
|
||||
|
||||
void main() {
|
||||
group('native worker progress', () {
|
||||
test('does not publish 100 percent while finalization is pending', () {
|
||||
expect(nativeWorkerFinalizingProgress(0), 0.95);
|
||||
expect(nativeWorkerFinalizingProgress(0.7), 0.7);
|
||||
expect(nativeWorkerFinalizingProgress(1), 0.99);
|
||||
});
|
||||
});
|
||||
|
||||
group('native worker contracts', () {
|
||||
final finalizerSource = File(
|
||||
'android/app/src/main/kotlin/com/zarz/spotiflac/'
|
||||
|
||||
Reference in New Issue
Block a user