From 8f997314b061860d4122dcd12e982ada8aa07577 Mon Sep 17 00:00:00 2001 From: zarzet Date: Sat, 11 Jul 2026 12:44:53 +0700 Subject: [PATCH] fix(finalizer): stage conversion outputs and sync SAF streams Decrypt, HIGH conversion, and container conversion wrote ffmpeg output directly under a real audio name in the final directory, so a process kill mid-conversion left a partial file that library scans listed as a corrupt track. Outputs now stream into a '.partial' sibling (invisible to scans, real trailing extension so ffmpeg still infers the muxer) and are fsynced and renamed into place only on success. Also fixes the delete-then-rename in the ffmpeg tag embed (a kill between the two lost the file entirely - now renames over the original) and flushes+fsyncs SAF output streams before staged documents are published. --- .../zarz/spotiflac/NativeDownloadFinalizer.kt | 94 +++++++++++++++---- .../com/zarz/spotiflac/SafDownloadHandler.kt | 17 ++++ 2 files changed, 95 insertions(+), 16 deletions(-) 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 5b8fb098..efaa1e81 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt @@ -461,6 +461,7 @@ object NativeDownloadFinalizer { attempts.add(Triple(buildOutputPath(localInput, ".mp4"), false, true)) for ((candidateOutput, mapAudioOnly, forceMov) in attempts) { + val stagedOutput = stagedConversionPath(candidateOutput) try { val audioMap = if (mapAudioOnly) "-map 0:a " else "" // Force the flac muxer when the target extension is @@ -473,20 +474,22 @@ object NativeDownloadFinalizer { candidateOutput.lowercase(Locale.ROOT).endsWith(".flac") -> "-f flac " else -> "" } - val command = "-v error -decryption_key ${q(candidate)} -f $inputFormat -i ${q(localInput)} ${audioMap}-c copy ${muxerOverride}${q(candidateOutput)} -y" + val command = "-v error -decryption_key ${q(candidate)} -f $inputFormat -i ${q(localInput)} ${audioMap}-c copy ${muxerOverride}${q(stagedOutput)} -y" val result = runFFmpeg(command, shouldCancel) lastOutput = result.second - if (result.first && File(candidateOutput).exists()) { + if (result.first && File(stagedOutput).exists() && + promoteStagedConversion(stagedOutput, candidateOutput) + ) { successPath = candidateOutput outputPath = candidateOutput break } - File(candidateOutput).delete() + File(stagedOutput).delete() } catch (e: CancellationException) { - File(candidateOutput).delete() + File(stagedOutput).delete() throw e } catch (e: Exception) { - File(candidateOutput).delete() + File(stagedOutput).delete() throw e } } @@ -535,24 +538,31 @@ object NativeDownloadFinalizer { val localInput = materializeForFFmpeg(context, input, state) val deleteLocalInput = state.filePath.startsWith("content://") val output = buildOutputPath(localInput, ext) + val stagedOutput = stagedConversionPath(output) var adoptedOutput = false try { val command = if (format == "opus") { - "-v error -hide_banner -i ${q(localInput)} -codec:a libopus -b:a $bitrate -vbr on -compression_level 10 -map 0:a ${q(output)} -y" + "-v error -hide_banner -i ${q(localInput)} -codec:a libopus -b:a $bitrate -vbr on -compression_level 10 -map 0:a ${q(stagedOutput)} -y" } else if (format == "aac") { - "-v error -hide_banner -i ${q(localInput)} -codec:a aac -b:a $bitrate -map 0:a -f mp4 ${q(output)} -y" + "-v error -hide_banner -i ${q(localInput)} -codec:a aac -b:a $bitrate -map 0:a -f mp4 ${q(stagedOutput)} -y" } else { - "-v error -hide_banner -i ${q(localInput)} -codec:a libmp3lame -b:a $bitrate -map 0:a -id3v2_version 3 ${q(output)} -y" + "-v error -hide_banner -i ${q(localInput)} -codec:a libmp3lame -b:a $bitrate -map 0:a -id3v2_version 3 ${q(stagedOutput)} -y" } val result = runFFmpeg(command, shouldCancel) - if (!result.first || !File(output).exists()) { + if (!result.first || !File(stagedOutput).exists()) { throw IllegalStateException("HIGH conversion failed: ${result.second}") } + if (!promoteStagedConversion(stagedOutput, output)) { + throw IllegalStateException("failed to publish HIGH conversion output") + } embedBasicMetadata(context, output, input, metadataFormat) replaceStatePath(context, input, state, output, deleteOld = true) adoptedOutput = true } finally { - if (!adoptedOutput) File(output).delete() + if (!adoptedOutput) { + File(stagedOutput).delete() + File(output).delete() + } if (deleteLocalInput) File(localInput).delete() } state.quality = "$displayFormat ${bitrate.removeSuffix("k")}kbps" @@ -578,6 +588,7 @@ object NativeDownloadFinalizer { val localInput = materializeForFFmpeg(context, input, state) val deleteLocalInput = state.filePath.startsWith("content://") val output = buildOutputPath(localInput, ".flac") + val stagedOutput = stagedConversionPath(output) var adoptedOutput = false try { val codec = probePrimaryAudioCodec(localInput, shouldCancel) @@ -597,7 +608,11 @@ object NativeDownloadFinalizer { val nativeFlacOutput = if (localInput.lowercase(Locale.ROOT).endsWith(".flac")) { localInput } else { - File(localInput).copyTo(File(output), overwrite = true).absolutePath + File(localInput).copyTo(File(stagedOutput), overwrite = true) + if (!promoteStagedConversion(stagedOutput, output)) { + throw IllegalStateException("failed to publish native FLAC output") + } + output } embedBasicMetadata(context, nativeFlacOutput, input, "flac") replaceStatePath(context, input, state, nativeFlacOutput, deleteOld = true) @@ -605,17 +620,23 @@ object NativeDownloadFinalizer { return } val result = runFFmpeg( - "-v error -xerror -i ${q(localInput)} -c:a flac -compression_level 8 ${q(output)} -y", + "-v error -xerror -i ${q(localInput)} -c:a flac -compression_level 8 ${q(stagedOutput)} -y", shouldCancel, ) - if (!result.first || !File(output).exists()) { + if (!result.first || !File(stagedOutput).exists()) { throw IllegalStateException("container conversion failed: ${result.second}") } + if (!promoteStagedConversion(stagedOutput, output)) { + throw IllegalStateException("failed to publish container conversion output") + } embedBasicMetadata(context, output, input, "flac") replaceStatePath(context, input, state, output, deleteOld = true) adoptedOutput = true } finally { - if (!adoptedOutput) File(output).delete() + if (!adoptedOutput) { + File(stagedOutput).delete() + File(output).delete() + } if (deleteLocalInput) File(localInput).delete() } } @@ -764,6 +785,7 @@ object NativeDownloadFinalizer { val uri = Uri.parse(path) context.contentResolver.openOutputStream(uri, "wt")?.use { output -> File(tempPath).inputStream().use { input -> input.copyTo(output) } + SafDownloadHandler.syncOutputStream(output) } ?: throw IllegalStateException("failed to write ReplayGain back to SAF") } finally { File(tempPath).delete() @@ -1222,7 +1244,9 @@ object NativeDownloadFinalizer { val ext = normalizeExt(File(path).extension).ifBlank { ".tmp" } val inputFile = File(path) - val temp = File(inputFile.parentFile, "${inputFile.nameWithoutExtension}_tagged$ext") + // ".partial" keeps the temp invisible to library scans while FFmpeg + // still infers the muxer from the real trailing extension. + val temp = File(inputFile.parentFile, "${inputFile.nameWithoutExtension}_tagged.partial$ext") val isM4a = format == "m4a" val isOpus = format == "opus" val coverFile = if (isM4a || isOpus) downloadCoverForMetadata(context, input) else null @@ -1278,7 +1302,11 @@ object NativeDownloadFinalizer { result = runFFmpeg(buildEmbedCommand(true)) } if (result.first && temp.exists()) { - if (inputFile.delete()) { + fsyncQuietly(temp) + // Rename directly over the original: a process kill between a + // delete-first and the rename would lose the file entirely. + adoptedTemp = temp.renameTo(inputFile) + if (!adoptedTemp && inputFile.delete()) { originalDeleted = true adoptedTemp = temp.renameTo(inputFile) } @@ -1291,6 +1319,18 @@ object NativeDownloadFinalizer { } } + /** + * Best-effort fsync so a file's bytes are durable before it is renamed + * over another file; fsync on a fresh handle flushes the page cache pages + * written earlier by ffmpeg in this process. + */ + private fun fsyncQuietly(file: File) { + try { + RandomAccessFile(file, "rw").use { it.fd.sync() } + } catch (_: Exception) { + } + } + private fun createMetadataBlockPicture(coverFile: File): String? { return try { if (!coverFile.exists() || coverFile.length() <= 0L) return null @@ -1520,6 +1560,28 @@ object NativeDownloadFinalizer { } } + /** + * Staged sibling name for conversion outputs: "song.flac" -> "song.partial.flac". + * The ".partial" shape is ignored by library scans and duplicate checks, + * while the real trailing extension still lets FFmpeg infer the muxer. A + * process kill mid-conversion therefore never leaves a partial file under a + * real audio name in the user's music folder. + */ + private fun stagedConversionPath(finalPath: String): String { + val file = File(finalPath) + val ext = file.extension + val name = if (ext.isBlank()) "${file.name}.partial" else "${file.nameWithoutExtension}.partial.$ext" + return File(file.parentFile, name).absolutePath + } + + private fun promoteStagedConversion(stagedPath: String, finalPath: String): Boolean { + val staged = File(stagedPath) + fsyncQuietly(staged) + val final = File(finalPath) + if (staged.renameTo(final)) return true + return final.delete() && staged.renameTo(final) + } + private fun buildOutputPath(inputPath: String, extension: String): String { val ext = normalizeExt(extension).ifBlank { ".tmp" } val file = File(inputPath) diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt index d1d2b451..ef8e3bad 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt @@ -6,6 +6,8 @@ import android.util.Log import androidx.documentfile.provider.DocumentFile import org.json.JSONObject import java.io.File +import java.io.FileOutputStream +import java.io.OutputStream import java.util.Locale /** @@ -35,6 +37,19 @@ object SafDownloadHandler { return synchronized(lock) { block() } } + /** + * Flushes and fsyncs a SAF output stream so the copied bytes are durable + * before the staged document is renamed to its final name; without this a + * power loss right after the rename can leave a truncated "complete" file. + */ + fun syncOutputStream(output: OutputStream) { + try { + output.flush() + (output as? FileOutputStream)?.fd?.sync() + } catch (_: Exception) { + } + } + fun handle(context: Context, requestJson: String, downloader: (String) -> String): String { val req = JSONObject(requestJson) val storageMode = req.optString("storage_mode", "") @@ -187,6 +202,7 @@ object SafDownloadHandler { srcFile.inputStream().use { input -> input.copyTo(output) } + syncOutputStream(output) } ?: throw IllegalStateException("failed to open SAF output stream") srcFile.delete() } catch (e: Exception) { @@ -331,6 +347,7 @@ object SafDownloadHandler { File(srcPath).inputStream().use { input -> input.copyTo(output) } + syncOutputStream(output) } val published = replaceFinalDocument(targetDir, document, finalName)