Cancel a running join session too (#224)

The FFmpegEngine half landed in ad2a75d; this is the same gap in
ConcatEngine. Between them, a real native session being asked to stop is
now covered on both FFmpeg paths.

The assertion is the session's return code again, and here that is not
merely the better choice but close to the only one: ConcatEngine does not
delete its output on cancellation at all. Its invokeOnCancellation is
FFmpegKit.cancel and nothing else, where FFmpegEngine's also deletes the
partial. Whether that asymmetry is deliberate is a separate question, so
this asserts what is true of both engines rather than depending on it.

The cancel triggers on SessionState.RUNNING rather than on progress.
ConcatWorker publishes no progress at all, so there is no callback to hang
it on even in principle -- and the conversion side already measured the
deeper reason, that the committed clips outrun a callback-triggered cancel.

The inputs are the mismatched pair on purpose, so ConcatPlanner chooses
REENCODE. A stream copy of two 2 s clips is close to instantaneous and
would leave nothing to interrupt; re-encoding is also the case where a user
would actually reach for Cancel.

Verified on a local API 34 emulator: three runs green at 64/0/0/3, and
replacing invokeOnCancellation's body with an empty block fails this test
and nothing else, with state=COMPLETED rc=0.

Media3Engine's transformer.cancel() is still uncovered and #224 stays open
for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

app/src/androidTest/java/org/libremediaconverter/ffmpeg/FFmpegEngineTest.kt

@@ -4,7 +4,16 @@ import android.media.MediaExtractor
 import android.media.MediaFormat
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
+import com.arthenica.ffmpegkit.FFmpegKit
+import com.arthenica.ffmpegkit.FFmpegSession
+import com.arthenica.ffmpegkit.ReturnCode
+import com.arthenica.ffmpegkit.SessionState
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.cancelAndJoin
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -139,6 +148,136 @@ class FFmpegEngineTest {
         assertEquals("OggS", magic)
     }
 
+    /**
+     * The percentage itself, which every other test in this class computes and none of them reads.
+     *
+     * `FFmpegEngine` derives progress as `stats.time / durationMs * 100`, and the statistics
+     * callback runs on every conversion here — but every call site omits `onProgress`, so until
+     * this test nothing on any source set had ever looked at the number (#229). #196 covered the
+     * *worker's* progress lambda, and did it with a fake engine that reports whatever the test
+     * tells it to; `ProgressNotificationTest` covers throttling the same way. The arithmetic was
+     * the one part with no reader.
+     *
+     * ## Why the duration is deliberately wrong
+     *
+     * `sample_h264.mp4` is exactly 3.000 s, and this passes **30 s** as the duration. So the
+     * conversion still encodes the whole clip, `stats.time` still climbs to about 3000 ms, and the
+     * reported percentage tops out around **10** rather than 100.
+     *
+     * That is what makes the assertion bite. A range check alone is worthless here: replacing
+     * `percent` with a constant `0` satisfies "every value is in 0..100" and "the values never go
+     * backwards", and so does a list of `[0, 100]`. Pinning the *band* rejects every constant, and
+     * — because the band is a tenth of the way up — it also rejects an implementation that ignores
+     * `durationMs`, which would report ~100 for the same run.
+     *
+     * The bound is deliberately loose (5..25 for an expected 10). The last statistics callback can
+     * land slightly before the final frame, so the peak is "about 3000 ms of a claimed 30 000",
+     * not exactly it.
+     */
+    @Test
+    fun progressIsReportedAsAFractionOfTheDurationItWasGiven() {
+        val seen = mutableListOf<Int>()
+        val out = outputFor("out_progress.mp4")
+        runBlocking {
+            engine.run(
+                request = ConversionRequest(spec = OutputFormat.MP4_H264.spec, quality = QualityTier.BEST),
+                inputPath = input.absolutePath,
+                output = out,
+                // Ten times the fixture's real 3 s. See the KDoc.
+                durationMs = 30_000,
+                onProgress = { percent -> seen += percent },
+            )
+        }
+
+        assertTrue("the statistics callback never reported progress", seen.isNotEmpty())
+        assertTrue("progress out of range: $seen", seen.all { it in 0..100 })
+        assertEquals("progress went backwards: $seen", seen.sorted(), seen)
+        // The band. Rejects any constant, and rejects ignoring durationMs (which would read ~100).
+        val peak = seen.max()
+        assertTrue(
+            "3 s of media against a claimed 30 s should peak near 10%, got $peak from $seen",
+            peak in 5..25,
+        )
+    }
+
+    /**
+     * Cancelling a *running* conversion actually stops the native session.
+     *
+     * Nothing on any source set did this before (#224). Every `cancel` in `app/src/androidTest` is
+     * `WorkManager.cancelWorkById` against work that is **queued or already finished** — the two in
+     * `ReattachOnLaunchTest` cancel a job carrying a one-hour initial delay, and one immediately
+     * after enqueue. On the JVM, `WorkerCancellationTest` and `HardwareFallbackTest`'s cancellation
+     * case drive a `SoftwareTranscoder` double that records the call. No test had ever asked a real
+     * native session to stop. This is `docs/defect-audit.md` **D10**'s forcing condition.
+     *
+     * It is the one path where cancelling wrong is silently expensive rather than loudly broken: a
+     * missed `FFmpegKit.cancel` leaves the native process encoding to completion while the UI says
+     * the job is cancelled, and nothing reports the battery and thermal cost.
+     *
+     * ## Why the assertion is the session's return code, not the output file
+     *
+     * The obvious assertion — the partial output is gone — **cannot fail**, so it would have been a
+     * vacuous test. `invokeOnCancellation` deletes the path, and on POSIX unlinking a file ffmpeg
+     * still holds open leaves ffmpeg writing to the unlinked inode; the path stays gone whether or
+     * not the cancel ever reached the session. Deleting `FFmpegKit.cancel` and keeping
+     * `output.delete()` passes that check every time.
+     *
+     * What distinguishes them is the session's own verdict: a cancelled session ends with the
+     * cancel return code, a completed one ends successfully. That is a fact about the session
+     * rather than about timing, so it is read *after* waiting for the session to leave
+     * [SessionState.RUNNING] rather than at a fixed delay.
+     *
+     * ## Why it cancels on RUNNING rather than on the first progress callback
+     *
+     * Measured, and this is the part worth keeping. Cancelling from the first `onProgress` was
+     * tried first and **failed on a local API 34 emulator with `state=COMPLETED rc=0`** — every
+     * committed fixture is 2-3 s at 320x240, and the encode finishes before the first statistics
+     * callback has been delivered and acted on. The progress callback is proof the session is
+     * running, but it arrives too late to interrupt anything.
+     *
+     * `FFmpegKit.listSessions` shows the session as [SessionState.RUNNING] far earlier, so that is
+     * what is waited on. `QualityTier.BEST` is deliberate for the same reason: `-preset medium`
+     * leaves more of the encode ahead of the cancel than `veryfast` would.
+     *
+     * The session is identified by diffing against the ids present before the run, because this
+     * class has already produced eight of them by the time this executes.
+     */
+    @Test
+    fun cancellingARunningConversionCancelsTheNativeSession(): Unit = runBlocking {
+        val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
+        val out = outputFor("out_cancelled.mp4")
+
+        val job = launch(Dispatchers.IO) {
+            engine.run(
+                request = ConversionRequest(spec = OutputFormat.MP4_H265.spec, quality = QualityTier.BEST),
+                inputPath = input.absolutePath,
+                output = out,
+                durationMs = 3_000,
+            )
+        }
+
+        // Interrupt as early as the session can be observed at all. See the KDoc: waiting for
+        // progress instead lost the race outright.
+        val ours = withTimeout(TIMEOUT_MS) {
+            var found: FFmpegSession? = null
+            while (found?.getState() != SessionState.RUNNING) {
+                found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
+                if (found?.getState() != SessionState.RUNNING) delay(POLL_MS)
+            }
+            found
+        }
+        job.cancelAndJoin()
+
+        withTimeout(TIMEOUT_MS) {
+            while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
+        }
+
+        assertTrue(
+            "the native session was not cancelled: state=${ours.getState()} rc=${ours.getReturnCode()}",
+            ReturnCode.isCancel(ours.getReturnCode()),
+        )
+    }
+
     // --- the quality tier the GPL licence was taken for --------------------
 
     @Test
@@ -176,4 +315,10 @@ class FFmpegEngineTest {
         }.exceptionOrNull()
         assertTrue("expected an FFmpegException, got $failure", failure is FFmpegEngine.FFmpegException)
     }
+
+    private companion object {
+        /** Generous: it bounds a hang, and every wait here normally settles in well under a second. */
+        const val TIMEOUT_MS = 30_000L
+        const val POLL_MS = 50L
+    }
 }

app/src/androidTest/java/org/libremediaconverter/join/ConcatEngineTest.kt

@@ -5,7 +5,16 @@ import android.media.MediaFormat
 import android.net.Uri
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
+import com.arthenica.ffmpegkit.FFmpegKit
+import com.arthenica.ffmpegkit.FFmpegSession
+import com.arthenica.ffmpegkit.ReturnCode
+import com.arthenica.ffmpegkit.SessionState
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.cancelAndJoin
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -16,6 +25,7 @@ import org.libremediaconverter.convert.MediaProbe
 import org.libremediaconverter.convert.StagingNames
 import org.libremediaconverter.ffmpeg.ConcatEngine
 import org.libremediaconverter.model.ConcatStrategy
+import org.libremediaconverter.work.ConcatWorker
 import java.io.File
 
 /**
@@ -50,6 +60,65 @@ class ConcatEngineTest {
         (staged + listOf(clipA, clipB, clipMismatched)).forEach { it.delete() }
     }
 
+    /**
+     * Cancelling a *running* join actually stops the native session.
+     *
+     * The `FFmpegEngine` half of #224 landed first (PR #236); this is the same gap in
+     * [ConcatEngine]. Before these two, no test on any source set had ever asked a real native
+     * session to stop — every `cancel` in `app/src/androidTest` targets WorkManager entries that
+     * are queued or already finished.
+     *
+     * ## Two things carried over from the conversion side, both measured there
+     *
+     * **The assertion is the session's return code.** A cancelled session ends with the cancel
+     * code, a completed one does not. The alternative — checking the output file — is even less
+     * available here than it was for conversions: [ConcatEngine] does not delete its output on
+     * cancellation at all. Its `invokeOnCancellation` is `FFmpegKit.cancel(...)` and nothing else,
+     * where [org.libremediaconverter.ffmpeg.FFmpegEngine]'s also deletes the partial. Whether that
+     * asymmetry is deliberate is a separate question from this test, which is why this asserts the
+     * thing that is true of both.
+     *
+     * **The cancel is triggered on [SessionState.RUNNING], not on progress.** `ConcatWorker`
+     * publishes no progress at all, so there is no callback to hang it on even in principle — but
+     * the conversion side established the deeper reason: the committed clips are 2 s at 320x240 and
+     * the encode outruns a callback-triggered cancel.
+     *
+     * The inputs are deliberately the **mismatched** pair, so [ConcatStrategy.REENCODE] is chosen.
+     * A stream copy of two short clips is close to instantaneous and would leave nothing to
+     * interrupt; re-encoding is the case where a user would actually reach for Cancel.
+     *
+     * *Mutation:* drop `FFmpegKit.cancel(session.getSessionId())` from `ConcatEngine`'s
+     * `invokeOnCancellation` — the session runs to completion and this fails.
+     */
+    @Test
+    fun cancellingARunningJoinCancelsTheNativeSession(): Unit = runBlocking {
+        val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
+        val out = output("cancelled_join.mp4")
+
+        val job = launch(Dispatchers.IO) {
+            engine.join(listOf(Uri.fromFile(clipA), Uri.fromFile(clipMismatched)), out, ConcatWorker.DEFAULT_FORMAT)
+        }
+
+        val ours = withTimeout(TIMEOUT_MS) {
+            var found: FFmpegSession? = null
+            while (found?.getState() != SessionState.RUNNING) {
+                found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
+                if (found?.getState() != SessionState.RUNNING) delay(POLL_MS)
+            }
+            found
+        }
+        job.cancelAndJoin()
+
+        withTimeout(TIMEOUT_MS) {
+            while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
+        }
+
+        assertTrue(
+            "the native join session was not cancelled: state=${ours.getState()} rc=${ours.getReturnCode()}",
+            ReturnCode.isCancel(ours.getReturnCode()),
+        )
+    }
+
     private fun copyAsset(name: String): File {
         val out = File(context.cacheDir, name)
         InstrumentationRegistry.getInstrumentation().context.assets
@@ -180,4 +249,10 @@ class ConcatEngineTest {
             a.width != mismatched.width || a.height != mismatched.height,
         )
     }
+
+    private companion object {
+        /** Generous: it bounds a hang, and both waits here normally settle in well under a second. */
+        const val TIMEOUT_MS = 30_000L
+        const val POLL_MS = 50L
+    }
 }