fix(metadata): preserve original artwork resolution for SAF files

This commit is contained in:
zarzet committed 2026-10-09 10:34:49 +07:00
1 parent 9f2062d495
commit c7b1d0b0ec
5 files changed
+179 -14

No files matched your search

@@ -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
@@ -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
@@ -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_');
@@ -13,6 +13,40 @@ pub struct DescriptorLibraryMetadata {
pub cover_mime: String,
}
fn descriptor_path(descriptor: i32) -> Result<String, AudioTagsError> {
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<Arc<RequestLease>>,
) -> Result<Vec<u8>, 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<Arc<RequestLease>>,
) -> Result<DescriptorLibraryMetadata, AudioTagsError> {
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()
@@ -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