diff --git a/android/app/src/androidTest/kotlin/com/zarz/spotiflac/NativeDownloadContainerTest.kt b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/NativeDownloadContainerTest.kt index be7d0629..4015b17e 100644 --- a/android/app/src/androidTest/kotlin/com/zarz/spotiflac/NativeDownloadContainerTest.kt +++ b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/NativeDownloadContainerTest.kt @@ -106,6 +106,54 @@ class NativeDownloadContainerTest { } finally { covers.deleteRecursively() } } + @Test + fun descriptorArtworkExportPreservesOriginalResolutionAndBytes() { + val context = InstrumentationRegistry.getInstrumentation().targetContext + val directory = File(context.cacheDir, "original-artwork-${System.nanoTime()}").apply { mkdirs() } + try { + for ((width, height, format) in listOf( + Triple(3000, 3000, android.graphics.Bitmap.CompressFormat.JPEG), + Triple(4500, 4500, android.graphics.Bitmap.CompressFormat.PNG), + Triple(3000, 1500, android.graphics.Bitmap.CompressFormat.PNG), + )) { + val artwork = File(directory, "cover-$width-$height.img") + val bitmap = android.graphics.Bitmap.createBitmap(width, height, android.graphics.Bitmap.Config.ARGB_8888) + try { + bitmap.eraseColor(android.graphics.Color.GREEN) + artwork.outputStream().use { assertTrue(bitmap.compress(format, 95, it)) } + } finally { bitmap.recycle() } + val original = artwork.readBytes() + val input = File(directory, "track-$width-$height.flac") + val fixture = NativeDownloadFinalizer.runFFmpegArguments(arrayOf( + "-v", "error", "-f", "lavfi", "-i", "sine=frequency=997:sample_rate=48000", + "-i", artwork.path, "-t", "0.1", "-map", "0:a", "-map", "1:v", + "-c:a", "flac", "-c:v", "copy", "-disposition:v", "attached_pic", input.path, + )) + assertTrue(fixture.second, fixture.first) + val before = java.security.MessageDigest.getInstance("SHA-256").digest(input.readBytes()) + val exported = File(directory, "export-$width-$height.img") + android.os.ParcelFileDescriptor.open(input, android.os.ParcelFileDescriptor.MODE_READ_ONLY).use { descriptor -> + backend.extractCoverToFile("/proc/self/fd/${descriptor.fd}", exported.path, input.name) + assertTrue("$width x $height artwork must remain byte-identical", exported.readBytes().contentEquals(original)) + val bounds = android.graphics.BitmapFactory.Options().apply { inJustDecodeBounds = true } + android.graphics.BitmapFactory.decodeFile(exported.path, bounds) + assertEquals(width, bounds.outWidth) + assertEquals(height, bounds.outHeight) + assertTrue(descriptor.statSize > 0) + if (width == 3000) { + val preview = com.spotiflac.backend.readLibraryMetadataFromDescriptor( + descriptor.fd, input.name, "2026-10-09T00:00:00Z", true, null, + ).coverBytes + android.graphics.BitmapFactory.decodeByteArray(preview, 0, preview.size, bounds) + assertEquals(800, bounds.outWidth) + assertEquals(if (height == 3000) 800 else 400, bounds.outHeight) + } + } + assertTrue(before.contentEquals(java.security.MessageDigest.getInstance("SHA-256").digest(input.readBytes()))) + } + } finally { directory.deleteRecursively() } + } + @Test fun nativeFormatsPreserveTagsLyricsAndReplayGain() { val context = InstrumentationRegistry.getInstrumentation().targetContext diff --git a/android/app/src/rust/kotlin/com/zarz/spotiflac/SelectedCoreBackend.kt b/android/app/src/rust/kotlin/com/zarz/spotiflac/SelectedCoreBackend.kt index a474698f..7db44ff3 100644 --- a/android/app/src/rust/kotlin/com/zarz/spotiflac/SelectedCoreBackend.kt +++ b/android/app/src/rust/kotlin/com/zarz/spotiflac/SelectedCoreBackend.kt @@ -451,14 +451,14 @@ internal object RustCoreBackend : CoreBackend { audioPath.removePrefix("/proc/self/fd/").toIntOrNull() } else null if (descriptor != null) { - val result = com.spotiflac.backend.readLibraryMetadataFromDescriptor( - descriptor, hint, java.time.Instant.now().toString(), true, null, + val artwork = com.spotiflac.backend.extractCoverFromDescriptor( + descriptor, hint, null, ) - check(result.coverBytes.isNotEmpty()) { "No embedded artwork" } + check(artwork.isNotEmpty()) { "No embedded artwork" } val output = File(outputPath) val staged = File.createTempFile("cover_", ".tmp", output.parentFile) try { - staged.writeBytes(result.coverBytes) + staged.writeBytes(artwork) check(staged.renameTo(output)) { "Could not publish artwork" } } finally { staged.delete() } return diff --git a/lib/services/downloaded_embedded_cover_resolver.dart b/lib/services/downloaded_embedded_cover_resolver.dart index fc3cfce0..19b64280 100644 --- a/lib/services/downloaded_embedded_cover_resolver.dart +++ b/lib/services/downloaded_embedded_cover_resolver.dart @@ -70,7 +70,9 @@ class DownloadedEmbeddedCoverResolver { static const int _maxConcurrentExtractions = 2; static const int _maxQueuedBackgroundExtractions = 24; static const String _persistentCacheDirectoryName = 'embedded_cover_previews'; - static const String _persistentCacheVersion = 'v1'; + // v1 SAF previews could contain the Library's 800px thumbnail instead of + // original embedded artwork. Re-extract even when the audio has not changed. + static const String _persistentCacheVersion = 'v2'; static const int _maxPersistentCacheEntries = 384; static const int _maxPersistentCacheBytes = 160 << 20; static const int _persistentCacheSweepTargetBytes = 128 << 20; @@ -902,6 +904,13 @@ class DownloadedEmbeddedCoverResolver { final name = p.basename(entity.path); try { final stat = await entity.stat(); + if (name.startsWith('v1_') && name.endsWith('.jpg')) { + await entity.delete(); + await _deleteFileIfPresent( + File(_persistentSourceSidecarPath(entity.path)), + ); + continue; + } final isAbandonedImageStage = name.startsWith('.$_persistentCacheVersion') && name.contains('.stage_'); diff --git a/rust_backend/crates/mobile/src/library_metadata.rs b/rust_backend/crates/mobile/src/library_metadata.rs index 990c9379..8eb6707f 100644 --- a/rust_backend/crates/mobile/src/library_metadata.rs +++ b/rust_backend/crates/mobile/src/library_metadata.rs @@ -13,6 +13,40 @@ pub struct DescriptorLibraryMetadata { pub cover_mime: String, } +fn descriptor_path(descriptor: i32) -> Result { + if descriptor < 0 { + return Err("invalid audio descriptor".to_owned().into()); + } + let directory = if cfg!(any(target_os = "android", target_os = "linux")) { + "/proc/self/fd" + } else { + "/dev/fd" + }; + Ok(format!("{directory}/{descriptor}")) +} + +/// Read original embedded artwork for metadata, export and editing. Unlike the +/// Library scan result, these bytes must never be resized or re-encoded. +/// The native caller retains the descriptor until this synchronous call returns. +#[uniffi::export] +pub fn extract_cover_from_descriptor( + descriptor: i32, + hint: String, + lease: Option>, +) -> Result, AudioTagsError> { + let check = || check_lease(lease.as_deref()); + check()?; + let path = descriptor_path(descriptor)?; + let mut file = open_audio_file(&path)?; + let format = tags::library_extension(&path, &hint); + let format = if matches!(format.as_str(), "mp4" | "aac") { + "m4a" + } else { + &format + }; + Ok(tags::extract_cover(&mut file, format, &check)?.data) +} + /// The caller keeps the descriptor open until this synchronous read returns. /// Duplicating its descriptor preserves SAF access, including single-open /// AppFuse proxies, without resolving a protected path or copying the audio. @@ -24,17 +58,9 @@ pub fn read_library_metadata_from_descriptor( include_cover: bool, lease: Option>, ) -> Result { - if descriptor < 0 { - return Err("invalid audio descriptor".to_owned().into()); - } + let path = descriptor_path(descriptor)?; let check = || check_lease(lease.as_deref()); check()?; - let directory = if cfg!(any(target_os = "android", target_os = "linux")) { - "/proc/self/fd" - } else { - "/dev/fd" - }; - let path = format!("{directory}/{descriptor}"); let mut file = open_audio_file(&path)?; let format = tags::library_extension(&path, &hint); let (metadata, mut artwork) = if include_cover && format == "flac" { @@ -85,6 +111,49 @@ mod tests { use std::io::Write; use std::os::fd::AsRawFd; + #[test] + fn original_artwork_is_copied_without_decoding_or_changing_the_source() { + // Extraction preserves even formats the thumbnail decoder cannot read. + let artwork = b"original embedded image bytes"; + let mut picture = 3_u32.to_be_bytes().to_vec(); + picture.extend(10_u32.to_be_bytes()); + picture.extend(b"image/jpeg"); + picture.extend(0_u32.to_be_bytes()); + for value in [4500_u32, 4500, 24, 0, artwork.len() as u32] { + picture.extend(value.to_be_bytes()); + } + picture.extend(artwork); + let mut flac = b"fLaC".to_vec(); + flac.push(0x86); + flac.extend(&u32::try_from(picture.len()).unwrap().to_be_bytes()[1..]); + flac.extend(picture); + flac.extend([0xff, 0xf8]); + let mut source = tempfile::tempfile().unwrap(); + source.write_all(&flac).unwrap(); + let descriptor = source.as_raw_fd(); + assert_eq!( + extract_cover_from_descriptor(descriptor, "Track.FLAC".into(), None).unwrap(), + artwork + ); + assert_eq!(source.metadata().unwrap().len(), flac.len() as u64); + use std::io::{Read, Seek, SeekFrom}; + source.seek(SeekFrom::Start(0)).unwrap(); + let mut unchanged = Vec::new(); + source.read_to_end(&mut unchanged).unwrap(); + assert_eq!(unchanged, flac); + + use crate::cancellation::{CancellationDomain, CancellationRegistry}; + let registry = CancellationRegistry::new(CancellationDomain::ExtensionRequest); + let lease = registry.acquire("artwork".into()).unwrap(); + registry.cancel("artwork".into()).unwrap(); + assert!( + extract_cover_from_descriptor(descriptor, "Track.flac".into(), Some(lease)).is_err() + ); + assert!(source.metadata().is_ok()); + assert!(extract_cover_from_descriptor(-1, "Track.flac".into(), None).is_err()); + assert!(extract_cover_from_descriptor(descriptor, "Track.xyz".into(), None).is_err()); + } + #[test] fn descriptor_remains_open_and_audio_does_not_need_a_path_grant() { let path = std::env::temp_dir().join(format!("library-fd-{}.wav", std::process::id())); @@ -124,6 +193,9 @@ mod tests { assert_eq!(json["bitDepth"], 16); assert!(metadata.cover_bytes.is_empty()); assert!(source.metadata().unwrap().is_file()); + assert!( + extract_cover_from_descriptor(source.as_raw_fd(), "Track.wav".into(), None).is_err() + ); assert!( read_library_metadata_from_descriptor(-1, "x.flac".into(), "".into(), false, None) .is_err() diff --git a/test/downloaded_embedded_cover_resolver_test.dart b/test/downloaded_embedded_cover_resolver_test.dart index 19e4cbb1..fa41dbb6 100644 --- a/test/downloaded_embedded_cover_resolver_test.dart +++ b/test/downloaded_embedded_cover_resolver_test.dart @@ -584,6 +584,42 @@ void main() { }, ); + test('legacy SAF thumbnails are replaced without changing the audio', () async { + const source = 'content://covers/document/original-artwork.flac'; + sourcePaths.add(source); + var extractions = 0; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(backendChannel, (call) async { + if (call.method == 'getSafFileModTimes') { + return jsonEncode({source: 1234}); + } + expect(call.method, 'extractCoverToFile'); + extractions++; + await File( + (call.arguments as Map)['output_path'] as String, + ).writeAsBytes([1, 2, 3]); + return jsonEncode({'success': true}); + }); + final preview = (await DownloadedEmbeddedCoverResolver.resolveOrExtract( + source, + ))!; + await DownloadedEmbeddedCoverResolver.resetMemoryStateForTesting(); + final legacy = preview.replaceFirst('v2_', 'v1_'); + expect(legacy, isNot(preview)); + await File(preview).rename(legacy); + await File('$preview.source').rename('$legacy.source'); + await File(legacy).writeAsBytes([8, 0, 0]); + + final refreshed = await DownloadedEmbeddedCoverResolver.resolveOrExtract( + source, + ); + expect(extractions, 2); + expect(await File(refreshed!).readAsBytes(), [1, 2, 3]); + await DownloadedEmbeddedCoverResolver.runPersistentCacheMaintenanceForTesting(); + expect(await File(legacy).exists(), isFalse); + expect(await File('$legacy.source').exists(), isFalse); + }); + test('invalidate removes persistent variants after memory reset', () async { final sourcePath = await createAudioFixture('persistent-invalidate'); TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger