mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-09-29 04:42:02 +02:00
perf(audio): validate FLAC remuxes before encoder fallback
This commit is contained in:
@@ -778,16 +778,18 @@ object NativeDownloadFinalizer {
|
||||
adoptedOutput = true
|
||||
return
|
||||
}
|
||||
val result = runFFmpeg(
|
||||
"-v error -xerror -i ${q(localInput)} -c:a flac -compression_level 8 ${q(stagedOutput)} -y",
|
||||
shouldCancel,
|
||||
convertToStagedFlac(
|
||||
input = localInput,
|
||||
stagedOutput = stagedOutput,
|
||||
codec = codec,
|
||||
execute = { arguments -> runFFmpegArguments(arguments, shouldCancel) },
|
||||
checkCancelled = { checkCancelled(shouldCancel) },
|
||||
)
|
||||
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")
|
||||
}
|
||||
// Keep metadata failures before adoption so the source survives
|
||||
// and the unsuccessful output is removed by the local cleanup.
|
||||
embedBasicMetadata(context, output, input, "flac")
|
||||
replaceStatePath(context, input, state, output, deleteOld = true)
|
||||
adoptedOutput = true
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
package com.zarz.spotiflac
|
||||
|
||||
import java.io.File
|
||||
|
||||
/** Creates a validated staged FLAC; publication and source ownership stay with the caller. */
|
||||
internal fun convertToStagedFlac(
|
||||
input: String,
|
||||
stagedOutput: String,
|
||||
codec: String,
|
||||
execute: (Array<String>) -> Pair<Boolean, String>,
|
||||
checkCancelled: () -> Unit,
|
||||
) {
|
||||
val staged = File(stagedOutput)
|
||||
var complete = false
|
||||
var diagnostic = ""
|
||||
val encoders = if (NativeFinalizationPolicy.normalizeAudioCodec(codec) == "flac") {
|
||||
listOf(arrayOf("-c:a", "copy"), arrayOf("-c:a", "flac", "-compression_level", "8"))
|
||||
} else {
|
||||
listOf(arrayOf("-c:a", "flac", "-compression_level", "8"))
|
||||
}
|
||||
try {
|
||||
for (encoder in encoders) {
|
||||
checkCancelled()
|
||||
check(!staged.exists() || staged.delete()) { "failed to remove staged FLAC output" }
|
||||
val result = execute(
|
||||
arrayOf("-v", "error", "-xerror", "-i", input) +
|
||||
encoder + arrayOf("-f", "flac", stagedOutput, "-y"),
|
||||
)
|
||||
checkCancelled()
|
||||
diagnostic = result.second
|
||||
val hasHeader = result.first && staged.isFile && staged.length() > 42L &&
|
||||
staged.inputStream().use { stream ->
|
||||
val header = ByteArray(4)
|
||||
stream.read(header) == 4 && header.contentEquals(byteArrayOf(0x66, 0x4c, 0x61, 0x43))
|
||||
}
|
||||
if (!hasHeader) {
|
||||
if (result.first) diagnostic = "conversion produced no valid FLAC header"
|
||||
continue
|
||||
}
|
||||
if ("copy" !in encoder) {
|
||||
complete = true
|
||||
return
|
||||
}
|
||||
// A remux does not decode audio, so verify the whole stream to
|
||||
// reject corrupt later frames just as the encoder did previously.
|
||||
// Cancellation must never trigger an encoding retry.
|
||||
val validation = execute(
|
||||
arrayOf(
|
||||
"-v", "error", "-xerror", "-err_detect", "crccheck+explode", "-i", stagedOutput,
|
||||
"-map", "0:a:0", "-f", "null", "-",
|
||||
),
|
||||
)
|
||||
checkCancelled()
|
||||
diagnostic = validation.second
|
||||
if (validation.first) {
|
||||
complete = true
|
||||
return
|
||||
}
|
||||
}
|
||||
throw IllegalStateException("container conversion failed: $diagnostic")
|
||||
} finally {
|
||||
if (!complete) staged.delete()
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,165 @@
|
||||
package com.zarz.spotiflac
|
||||
|
||||
import java.io.File
|
||||
import java.nio.file.Files
|
||||
import java.util.concurrent.CancellationException
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertThrows
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assume.assumeTrue
|
||||
import org.junit.Test
|
||||
|
||||
class NativeFlacConversionTest {
|
||||
private fun withFiles(block: (File, File) -> Unit) {
|
||||
val directory = Files.createTempDirectory("native-flac-conversion-").toFile()
|
||||
try {
|
||||
val input = File(directory, "input.m4a").apply { writeText("original audio") }
|
||||
block(input, File(directory, "output.partial.flac"))
|
||||
assertEquals("original audio", input.readText())
|
||||
} finally {
|
||||
directory.deleteRecursively()
|
||||
}
|
||||
}
|
||||
|
||||
private fun writeFlacHeader(file: File) {
|
||||
file.writeBytes(byteArrayOf(0x66, 0x4c, 0x61, 0x43) + ByteArray(64))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun validRemuxSkipsEncoderAndLeavesSourceUntouched() = withFiles { input, output ->
|
||||
val commands = mutableListOf<List<String>>()
|
||||
convertToStagedFlac(input.path, output.path, "flac", { arguments ->
|
||||
commands.add(arguments.toList())
|
||||
if ("copy" in arguments) writeFlacHeader(output)
|
||||
true to ""
|
||||
}, {})
|
||||
assertEquals(2, commands.size)
|
||||
assertTrue("copy" in commands.first())
|
||||
assertFalse("-map" in commands.first())
|
||||
assertTrue("null" in commands.last())
|
||||
assertFalse("-frames:a" in commands.last())
|
||||
assertFalse(commands.any { "-compression_level" in it })
|
||||
assertTrue(output.exists())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun failedRemuxOrValidationRetriesEncoderWithCleanStage() {
|
||||
for (failure in listOf("command", "header", "decode")) {
|
||||
withFiles { input, output ->
|
||||
var encoded = false
|
||||
convertToStagedFlac(input.path, output.path, "flac", { arguments ->
|
||||
when {
|
||||
"copy" in arguments -> {
|
||||
if (failure == "header") output.writeText("invalid FLAC") else writeFlacHeader(output)
|
||||
(failure != "command") to "remux diagnostic"
|
||||
}
|
||||
"-compression_level" in arguments -> {
|
||||
assertFalse(output.exists())
|
||||
encoded = true
|
||||
writeFlacHeader(output)
|
||||
true to ""
|
||||
}
|
||||
else -> (encoded || failure != "decode") to "decode diagnostic"
|
||||
}
|
||||
}, {})
|
||||
assertTrue(encoded)
|
||||
assertTrue(output.exists())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun nonFlacSkipsRemuxAndFailedEncodeRemovesPartialOutput() = withFiles { input, output ->
|
||||
var calls = 0
|
||||
assertThrows(IllegalStateException::class.java) {
|
||||
convertToStagedFlac(input.path, output.path, "alac", { arguments ->
|
||||
calls++
|
||||
assertFalse("copy" in arguments)
|
||||
output.writeText("partial")
|
||||
false to "encoder failed"
|
||||
}, {})
|
||||
}
|
||||
assertEquals(1, calls)
|
||||
assertFalse(output.exists())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cancellationAfterRemuxDoesNotFallbackAndRemovesPartialOutput() = withFiles { input, output ->
|
||||
var cancelled = false
|
||||
var calls = 0
|
||||
assertThrows(CancellationException::class.java) {
|
||||
convertToStagedFlac(input.path, output.path, "flac", {
|
||||
calls++
|
||||
writeFlacHeader(output)
|
||||
cancelled = true
|
||||
false to "cancelled"
|
||||
}, {
|
||||
if (cancelled) throw CancellationException()
|
||||
})
|
||||
}
|
||||
assertEquals(1, calls)
|
||||
assertFalse(output.exists())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun hostFlacInMp4RemuxAndFallbackPreserveDecodedPcmAndArtwork() {
|
||||
val ffmpeg = System.getenv("SPOTIFLAC_TEST_FFMPEG").orEmpty()
|
||||
assumeTrue("Set SPOTIFLAC_TEST_FFMPEG for host media fixtures", File(ffmpeg).canExecute())
|
||||
fun execute(arguments: Array<String>): Pair<Boolean, String> {
|
||||
val process = ProcessBuilder(listOf(ffmpeg) + arguments).redirectErrorStream(true).start()
|
||||
val output = process.inputStream.bufferedReader().use { it.readText() }
|
||||
return (process.waitFor() == 0) to output
|
||||
}
|
||||
fun run(vararg arguments: String): String {
|
||||
val result = execute(arrayOf(*arguments))
|
||||
assertTrue(result.second, result.first)
|
||||
return result.second.trim()
|
||||
}
|
||||
val directory = Files.createTempDirectory("native-flac-fixture-").toFile()
|
||||
try {
|
||||
val cover = File(directory, "cover.png")
|
||||
run("-v", "error", "-f", "lavfi", "-i", "color=c=red:s=32x32", "-frames:v", "1", cover.path)
|
||||
val coverHash = run("-v", "error", "-i", cover.path, "-map", "0:v:0", "-f", "hash", "-hash", "sha256", "-")
|
||||
for ((sampleRate, sampleFormat) in listOf(44100 to "s16", 96000 to "s32")) {
|
||||
val source = File(directory, "source-$sampleRate.flac")
|
||||
val input = File(directory, "input-$sampleRate.m4a")
|
||||
run(
|
||||
"-v", "error", "-f", "lavfi", "-i", "aevalsrc=0.1*sin(2*PI*997*t):s=$sampleRate",
|
||||
"-t", "0.2", "-c:a", "flac", "-sample_fmt", sampleFormat, source.path,
|
||||
)
|
||||
run(
|
||||
"-v", "error", "-i", source.path, "-i", cover.path, "-map", "0:a:0", "-map", "1:v:0",
|
||||
"-c", "copy", "-disposition:v", "attached_pic", "-strict", "-2", "-f", "mp4", input.path,
|
||||
)
|
||||
val sourceHash = run("-v", "error", "-i", source.path, "-map", "0:a:0", "-c:a", "pcm_s32le", "-f", "hash", "-hash", "sha256", "-")
|
||||
for (mode in listOf("remux", "failed-remux", "corrupt-remux")) {
|
||||
val output = File(directory, "output-$sampleRate-$mode.partial.flac")
|
||||
var encoded = false
|
||||
convertToStagedFlac(input.path, output.path, "flac", { arguments ->
|
||||
if ("-compression_level" in arguments) encoded = true
|
||||
if (mode == "failed-remux" && "copy" in arguments) {
|
||||
false to "forced remux failure"
|
||||
} else {
|
||||
execute(arguments).also { result ->
|
||||
if (mode == "corrupt-remux" && "copy" in arguments && result.first) {
|
||||
val bytes = output.readBytes()
|
||||
for (index in bytes.size - 50 until bytes.size - 34) {
|
||||
bytes[index] = (bytes[index].toInt() xor 0x55).toByte()
|
||||
}
|
||||
output.writeBytes(bytes)
|
||||
}
|
||||
}
|
||||
}
|
||||
}, {})
|
||||
assertEquals(mode != "remux", encoded)
|
||||
assertEquals(sourceHash, run("-v", "error", "-i", output.path, "-map", "0:a:0", "-c:a", "pcm_s32le", "-f", "hash", "-hash", "sha256", "-"))
|
||||
assertEquals(coverHash, run("-v", "error", "-i", output.path, "-map", "0:v:0", "-f", "hash", "-hash", "sha256", "-"))
|
||||
assertTrue(input.exists())
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
directory.deleteRecursively()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1236,7 +1236,10 @@ class _DownloadRun {
|
||||
DownloadStatus.finalizing,
|
||||
progress: 0.95,
|
||||
);
|
||||
final flacPath = await FFmpegService.convertM4aToFlac(tempPath);
|
||||
final flacPath = await FFmpegService.convertM4aToFlac(
|
||||
tempPath,
|
||||
sourceCodec: codec,
|
||||
);
|
||||
if (flacPath == null) {
|
||||
_log.w('FFmpeg conversion returned null, keeping M4A file');
|
||||
branch = 'convertFailed';
|
||||
@@ -1364,6 +1367,7 @@ class _DownloadRun {
|
||||
);
|
||||
final flacPath = await FFmpegService.convertM4aToFlac(
|
||||
currentFilePath,
|
||||
sourceCodec: codec,
|
||||
);
|
||||
|
||||
if (flacPath != null) {
|
||||
|
||||
@@ -664,6 +664,7 @@ class FFmpegService {
|
||||
|
||||
static Future<String?> convertM4aToFlac(
|
||||
String inputPath, {
|
||||
String? sourceCodec,
|
||||
@visibleForTesting Future<FFmpegResult> Function(List<String>)? execute,
|
||||
}) async {
|
||||
final plan = await _conversionOutputPlan(
|
||||
@@ -672,7 +673,48 @@ class FFmpegService {
|
||||
deleteOriginal: true,
|
||||
);
|
||||
try {
|
||||
final result = await (execute ?? _executeWithArguments)([
|
||||
final run = execute ?? _executeWithArguments;
|
||||
final codec = sourceCodec ?? await probePrimaryAudioCodec(inputPath);
|
||||
if (codec?.trim().toLowerCase() == 'flac') {
|
||||
final remux = await run([
|
||||
'-v',
|
||||
'error',
|
||||
'-xerror',
|
||||
'-i',
|
||||
inputPath,
|
||||
'-c:a',
|
||||
'copy',
|
||||
'-f',
|
||||
'flac',
|
||||
plan.workingPath,
|
||||
'-y',
|
||||
]);
|
||||
if (remux.success && await _hasNativeFlacHeader(plan.workingPath)) {
|
||||
final validation = await run([
|
||||
'-v',
|
||||
'error',
|
||||
'-xerror',
|
||||
'-err_detect',
|
||||
'crccheck+explode',
|
||||
'-i',
|
||||
plan.workingPath,
|
||||
'-map',
|
||||
'0:a:0',
|
||||
'-f',
|
||||
'null',
|
||||
'-',
|
||||
]);
|
||||
if (validation.success) {
|
||||
return await _finalizeConversionOutput(
|
||||
plan: plan,
|
||||
inputPath: inputPath,
|
||||
deleteOriginal: true,
|
||||
);
|
||||
}
|
||||
}
|
||||
_log.w('FLAC remux validation failed; retrying with the encoder');
|
||||
}
|
||||
final result = await run([
|
||||
'-v',
|
||||
'error',
|
||||
'-xerror',
|
||||
@@ -700,6 +742,22 @@ class FFmpegService {
|
||||
return null;
|
||||
}
|
||||
|
||||
static Future<bool> _hasNativeFlacHeader(String path) async {
|
||||
final file = File(path);
|
||||
if (!await file.exists() || await file.length() <= 42) return false;
|
||||
final input = await file.open();
|
||||
try {
|
||||
final magic = await input.read(4);
|
||||
return magic.length == 4 &&
|
||||
magic[0] == 0x66 &&
|
||||
magic[1] == 0x4c &&
|
||||
magic[2] == 0x61 &&
|
||||
magic[3] == 0x43;
|
||||
} finally {
|
||||
await input.close();
|
||||
}
|
||||
}
|
||||
|
||||
/// Corrects a native FLAC payload's suffix without replacing a sibling file.
|
||||
static Future<String> ensureNativeFlacExtension(String inputPath) async {
|
||||
if (inputPath.toLowerCase().endsWith('.flac')) return inputPath;
|
||||
|
||||
@@ -28,6 +28,7 @@ void main() {
|
||||
).writeAsString('existing');
|
||||
final result = await FFmpegService.convertM4aToFlac(
|
||||
source.path,
|
||||
sourceCodec: 'alac',
|
||||
execute: (arguments) async {
|
||||
expect(arguments[arguments.indexOf('-i') + 1], source.path);
|
||||
expect(await source.exists(), isTrue);
|
||||
@@ -45,6 +46,7 @@ void main() {
|
||||
final source = await input('Song.m4a');
|
||||
final result = await FFmpegService.convertM4aToFlac(
|
||||
source.path,
|
||||
sourceCodec: 'alac',
|
||||
execute: (arguments) async {
|
||||
await File(arguments[arguments.length - 2]).writeAsString('partial');
|
||||
return FFmpegResult(success: false, returnCode: 1, output: 'failure');
|
||||
@@ -64,7 +66,11 @@ void main() {
|
||||
(_) async => FFmpegResult(success: true, returnCode: 0, output: ''),
|
||||
]) {
|
||||
expect(
|
||||
await FFmpegService.convertM4aToFlac(source.path, execute: execute),
|
||||
await FFmpegService.convertM4aToFlac(
|
||||
source.path,
|
||||
sourceCodec: 'alac',
|
||||
execute: execute,
|
||||
),
|
||||
isNull,
|
||||
);
|
||||
expect(await source.readAsString(), 'source');
|
||||
@@ -77,6 +83,7 @@ void main() {
|
||||
final source = await input('Song.flac');
|
||||
final result = await FFmpegService.convertM4aToFlac(
|
||||
source.path,
|
||||
sourceCodec: 'alac',
|
||||
execute: (arguments) async {
|
||||
expect(arguments[arguments.length - 2], isNot(source.path));
|
||||
expect(await source.readAsString(), 'source');
|
||||
@@ -101,14 +108,97 @@ void main() {
|
||||
}
|
||||
|
||||
final results = await Future.wait([
|
||||
FFmpegService.convertM4aToFlac(first.path, execute: execute),
|
||||
FFmpegService.convertM4aToFlac(second.path, execute: execute),
|
||||
FFmpegService.convertM4aToFlac(
|
||||
first.path,
|
||||
sourceCodec: 'alac',
|
||||
execute: execute,
|
||||
),
|
||||
FFmpegService.convertM4aToFlac(
|
||||
second.path,
|
||||
sourceCodec: 'alac',
|
||||
execute: execute,
|
||||
),
|
||||
]);
|
||||
expect(results.toSet(), hasLength(2));
|
||||
expect(results, everyElement(isNotNull));
|
||||
expect(await directory.list().length, 2);
|
||||
});
|
||||
|
||||
test(
|
||||
'FLAC payload is remuxed and decoded before deleting its source',
|
||||
() async {
|
||||
final source = await input('Song.m4a');
|
||||
final commands = <List<String>>[];
|
||||
final result = await FFmpegService.convertM4aToFlac(
|
||||
source.path,
|
||||
sourceCodec: 'flac',
|
||||
execute: (arguments) async {
|
||||
commands.add(arguments);
|
||||
expect(await source.exists(), isTrue);
|
||||
if (arguments.contains('copy')) {
|
||||
// Preserve FFmpeg's existing attached-artwork stream selection.
|
||||
expect(arguments, isNot(contains('-map')));
|
||||
await File(arguments[arguments.length - 2]).writeAsBytes([
|
||||
0x66,
|
||||
0x4c,
|
||||
0x61,
|
||||
0x43,
|
||||
...List<int>.filled(64, 0),
|
||||
]);
|
||||
} else {
|
||||
expect(arguments, containsAllInOrder(['-f', 'null', '-']));
|
||||
expect(
|
||||
arguments,
|
||||
containsAllInOrder(['-err_detect', 'crccheck+explode', '-i']),
|
||||
);
|
||||
expect(arguments, isNot(contains('-frames:a')));
|
||||
}
|
||||
return FFmpegResult(success: true, returnCode: 0, output: '');
|
||||
},
|
||||
);
|
||||
expect(result, isNotNull);
|
||||
expect(commands, hasLength(2));
|
||||
expect(await source.exists(), isFalse);
|
||||
},
|
||||
);
|
||||
|
||||
for (final failure in ['copy', 'header', 'decode']) {
|
||||
test(
|
||||
'invalid FLAC $failure falls back to encoding the intact source',
|
||||
() async {
|
||||
final source = await input('Song.m4a');
|
||||
var encoded = false;
|
||||
final result = await FFmpegService.convertM4aToFlac(
|
||||
source.path,
|
||||
sourceCodec: 'flac',
|
||||
execute: (arguments) async {
|
||||
expect(await source.readAsString(), 'source');
|
||||
if (arguments.contains('copy')) {
|
||||
await File(arguments[arguments.length - 2]).writeAsBytes([
|
||||
if (failure != 'header') ...[0x66, 0x4c, 0x61, 0x43],
|
||||
...List<int>.filled(64, 0),
|
||||
]);
|
||||
return FFmpegResult(
|
||||
success: failure != 'copy',
|
||||
returnCode: failure == 'copy' ? 1 : 0,
|
||||
output: '',
|
||||
);
|
||||
}
|
||||
if (arguments.last == '-') {
|
||||
return FFmpegResult(success: false, returnCode: 1, output: 'bad');
|
||||
}
|
||||
encoded = true;
|
||||
expect(arguments, containsAllInOrder(['-c:a', 'flac']));
|
||||
return succeed(arguments);
|
||||
},
|
||||
);
|
||||
expect(encoded, isTrue);
|
||||
expect(await File(result!).readAsString(), 'converted');
|
||||
expect(await source.exists(), isFalse);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
test(
|
||||
'native FLAC rename preserves sibling files and adds missing suffix',
|
||||
() async {
|
||||
|
||||
Reference in New Issue
Block a user