Compare commits

..
Author SHA1 Message Date
JMR-devandClaude Opus 5 6992f0e783 Reattach to a conversion that is still running (#230)
Reattachment.rank gives RUNNING the highest rank of all -- "live work
outranks a finished result because a running job is holding a foreground
service" -- and no test on either source set had ever produced one.
ReattachOnLaunchTest covers a job that finished, one whose staged file is
gone, an ambiguous pair, one still queued, and one the user cancelled.
ReattachmentTest exercises the ranking as a pure function over fabricated
snapshots. What was missing is a ViewModel meeting a real running job,
which is also the likeliest reattachment there is: the user starts a
conversion, leaves, and comes back while it is still going.

The engine is a fake, deliberately. The job has to still be running when
the ViewModel is built, and every real conversion in this suite finishes in
about a second -- racing that is what made the cancellation tests flaky
enough to need retries. A SoftwareTranscoder that blocks until released
removes the race outright. Nothing about reattachment depends on which
engine is transcoding: the tag query, Reattachment.choose over live
WorkManager state, and observe's mapping to Converting all run identically
whatever is doing the work.

This is what #230 can actually deliver, and the ticket asked for the answer
either way. Process death itself stays device-manual. D3/D13 already record
that am kill refuses a process holding a foreground service, and there is a
more basic obstacle underneath it: instrumentation runs in the app's own
process, so any route that really killed it would take the test runner with
it and leave nothing to assert with. Observing a relaunch needs two
instrumentation runs, which the runner does not provide. So the closest
observable analogue is a fresh ViewModel, with no memory of the work,
meeting a job that is genuinely mid-flight.

The teardown now resets ConversionDependencies. The suite runs without
Android Test Orchestrator, so a BlockingTranscoder left in place would hang
the next class that converts anything.

Verified on a local API 34 emulator: 67 tests, 0 failures, 3 skipped; and
making RUNNING unreattachable in Reattachment.rank fails this test and
nothing else -- which is also the evidence that the JVM ranking test was
not already covering it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 01:58:24 -05:00
Jason Ross bb920b5bd0 Merge pull request #239 from JMR-dev/test/content-uri-reaches-ffmpeg
Let a join read the files the user actually picked
2026-09-06 01:51:10 -05:00
JMR-devandClaude Opus 5 802997439d Let a join read the files the user actually picked (#238, #225)
Joining files picked through the system picker failed outright whenever
the strategy was stream copy -- the matched-files case the UI advertises
as "joined without re-encoding, no quality loss".

  [ffkitsaf @ ...] Protocol 'ffkitsaf' not on whitelist 'file,crypto,data'!
  Error opening input file .../joined_from_content.concat_list.txt

JoinScreen picks with OpenMultipleDocuments, so real inputs are always
content://. ConcatEngine maps each through getSafParameterForRead and
FFmpegConcatCommand writes the resulting ffkitsaf: paths into the concat
list file. The demuxer applies its own protocol whitelist, defaulting to
file,crypto,data, and -safe 0 does not touch it: that permits absolute
paths, this permits the scheme they carry. Two separate gates, and only
one was open.

Nothing caught it because the two halves of the bug never met. Only
STREAM_COPY feeds the demuxer a list file -- REENCODE passes each input
with its own -i, where the whitelist does not apply -- so joining over SAF
worked for mismatched clips. And every join test passed Uri.fromFile,
which takes ConcatEngine's uri.path arm instead of the bridge, so
matchingClipsAreJoinedByStreamCopy exercised stream copy with a file:
path and passed. The one broken combination was the one no test produced
and the only one a user can reach.

That is #225's gap: FFmpegKitConfig.getSafParameterForRead is on every
real conversion and join, and was on no passing test -- only on
UnopenableUriTest's failure side, which proves the error message rather
than the bridge. ContentUriInputTest now drives both the convert and join
paths from a real content:// URI.

It uses a plain ContentProvider, because the documents provider cannot be
reached. Measured three ways: a DOCUMENTS_PROVIDER without MANAGE_DOCUMENTS
is refused at install, instrumentation runs in the target app's process so
Instrumentation.getContext() still carries the app's uid and is denied, and
adoptShellPermissionIdentity(MANAGE_DOCUMENTS) is denied identically -- the
denial naming ACTION_OPEN_DOCUMENT as the only way in. The bridge needs no
documents provider: it opens a descriptor through the resolver, so any
readable content:// URI exercises it, and an ordinary provider may be
exported unprotected. The whole class stays headless. Recorded on #226,
which that also settles: its cheap half does not exist.

Verified on a local API 34 emulator: 66 tests, 0 failures, 3 skipped; and
removing the -protocol_whitelist pair reproduces the production failure
verbatim in the join test and nothing else. FFmpegConcatCommandTest pins
the flag on the JVM.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 01:42:03 -05:00
Jason Ross 495eaa4ab8 Merge pull request #241 from JMR-dev/fix/launcher-wiring-waits-for-the-pick
Wait for the pick the launcher test is about
2026-09-06 01:41:55 -05:00
JMR-devandClaude Opus 5 bc8e67888e Wait for the pick the launcher test is about (#220)
onInputPicked does not reach Ready on the calling thread. It hops twice --
withContext(pickDispatcher) { InputQuery.describe(...) } and then the probe
-- and pickDispatcher defaults to Dispatchers.IO, a real background thread
Compose's idling knows nothing about. So deliver() returned with the state
still Idle, and asserting immediately was a race the test usually won.

It lost five times on CI in one day, on PRs whose diffs were instrumented
tests and documentation and could not reach it. Two of those failures came
alongside #125's deadlock and could be argued as fallout; three did not.

waitUntil polls through waitForIdle, draining the main looper each time, so
it sees the recomposition the IO hop eventually posts back.

Injecting the dispatcher would be better and is not available here.
pickDispatcher is a constructor parameter precisely so a test can pin it,
but this test composes the real ConverterScreen, which resolves its own
ViewModel through viewModel() -- the seam is one layer below the launcher
edge this class exists to cover, and reaching for it would mean not testing
that edge.

The evidence is the mutation rather than the repetition count, per the note
#218 left: transposing the two launcher callbacks at ConverterScreen.kt:70
and :83 -- the exact defect this test guards -- still fails it, so the wait
did not make it vacuous. A transposed callback leaves the screen in Idle
forever and it fails on the timeout with the meaning it had before.
Supporting evidence, eight consecutive green runs of the class.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 01:34:17 -05:00
Jason Ross 706eea8709 Merge pull request #240 from JMR-dev/fix/cancel-tests-need-a-slower-encode
Stop the cancel tests losing their race on a loaded runner
2026-09-06 01:20:57 -05:00
JMR-devandClaude Opus 5 163ce54b77 Stop the cancel tests losing their race on a loaded runner
Both cancellation tests I added in ad2a75d and d293646 wait for
SessionState.RUNNING and then cancel. That is not enough. The conversion
one passed four consecutive local runs and all five CI legs, then failed
the API 34 and 35 legs of the next PR with state=COMPLETED rc=0, on a diff
that could not reach it. On a loaded runner the thread that observed
RUNNING can be descheduled long enough for a short encode to finish before
it calls cancel. A longer timeout does not help: the wait already
succeeded.

Two changes, because neither is sufficient alone.

A slower encode. The conversion test now targets WEBM_VP9 at BEST, the
slowest thing FFmpegCommandBuilder emits -- libvpx-vp9 -crf 31 -b:v 0,
with -deadline realtime added only on FAST. Probed on an API 34 emulator:
that session is still RUNNING at 1 s and finished by 2 s, against well
under a second for x265 -preset medium.

A bounded retry. An attempt whose session finished before the cancel
landed has not tested anything, so it is a miss rather than a failure and
is retried; only exhausting five attempts fails, and the message reports
every attempt's state and return code so a real breakage is distinguishable
from a slow machine.

The retry does not soften the test. With FFmpegKit.cancel removed from
both engines, every attempt ends COMPLETED, so both still fail -- verified,
each listing five [state=COMPLETED rc=0] outcomes. Two clean runs
beforehand at 64/0/0/3.

The join test gets the same treatment. It has not flaked yet, but it is
the same mechanism and the same fragility, and finding out on CI again is
not worth the round trip.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 01:04:03 -05:00
Jason Ross d293646f69 Merge pull request #237 from JMR-dev/test/cancelling-a-running-join
Cancel a running join session too
2026-09-06 00:11:33 -05:00
JMR-devandClaude Opus 5 5416788274 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>
2026-09-06 00:02:52 -05:00
Jason Ross ad2a75d9a0 Merge pull request #236 from JMR-dev/test/cancelling-a-running-session
Cancel a running FFmpeg session, which nothing had ever done
2026-09-05 23:53:13 -05:00
Jason Ross 2b921fafe4 Merge pull request #235 from JMR-dev/test/notification-cancel-action
Press the Cancel button in the notification
2026-09-05 23:45:53 -05:00
JMR-devandClaude Opus 5 98c0e4dba2 Cancel a running FFmpeg session, which nothing had ever done (#224)
Every cancel in app/src/androidTest is WorkManager.cancelWorkById against
work that is queued or already finished: ReattachOnLaunchTest cancels a job
carrying a one-hour initial delay, and another immediately after enqueue.
On the JVM, WorkerCancellationTest and HardwareFallbackTest's cancellation
case drive a SoftwareTranscoder double that records the call. No test on
any source set had asked a real native session to stop. That 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.

Two things were measured rather than assumed, and both changed the test.

The output file cannot be the assertion. invokeOnCancellation deletes the
path, and on POSIX unlinking a file ffmpeg still holds open leaves ffmpeg
writing to the unlinked inode -- so the path stays gone whether or not the
cancel reached the session, and removing FFmpegKit.cancel passes that check
every time. The session's own verdict is what separates them: a cancelled
session ends with the cancel return code, a completed one does not.

Cancelling from the first progress callback loses the race. It was tried
first and failed with state=COMPLETED rc=0: every committed fixture is
2-3 s at 320x240, and the encode finishes before the first statistics
callback is delivered and acted on. FFmpegKit.listSessions shows the
session RUNNING far earlier, so that is what the test waits for.
QualityTier.BEST is deliberate for the same reason -- preset medium leaves
more of the encode ahead of the cancel.

Verified on a local API 34 emulator. Four consecutive runs green at
62/0/0/3, and removing FFmpegKit.cancel while keeping output.delete fails
this test and nothing else, with state=COMPLETED rc=1.

ConcatEngine and Media3Engine carry the same shape and are not covered
here; #224 stays open for them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-05 23:45:34 -05:00
Jason Ross cf540f1ecc Merge pull request #234 from JMR-dev/test/ffmpeg-progress-is-observed
Read the progress percentage FFmpeg has always been computing
2026-09-05 23:21:54 -05:00
JMR-devandClaude Opus 5 e0412329ff Press the Cancel button in the notification (#227)
ConversionNotifications.build attaches one action, wired to
WorkManager.createCancelPendingIntent(id). Until now
createCancelPendingIntent had no references anywhere outside its own
declaration -- no JVM test, no instrumented test.

That is worth more than an ordinary uncovered line. A conversion runs in a
foreground service and the user is invited to leave the app; once they do,
this action is the only way to stop it. If the PendingIntent carries the
wrong id the button does nothing, the notification stays, and the job runs
to completion, with no error, no log and no screen to look at.

The obvious version of this test reads NotificationManager's active
notifications for id 1001 and taps what it finds. Rejected: the
instrumented suite grants no runtime permissions, so POST_NOTIFICATIONS is
denied throughout, and whether a suppressed foreground-service notification
is returned there is a platform detail that varies. The test would be
asserting something about notification visibility rather than about
cancellation. The PendingIntent is the subject and where it is read from is
incidental, so this builds the notification for a real live work id and
fires its action -- a real dispatch reaching real WorkManager, the same way
on every API level.

The job carries an initial delay so it stays ENQUEUED. A conversion of the
3 s fixture finishes in well under a second on an emulator, so racing a
cancel against a running job would be flaky in the direction that fails,
and cancelWorkById acts on ENQUEUED identically. What is under test is
whether firing the action reaches WorkManager with the right id.

Verified both ways on a local API 34 emulator: 61 tests, 0 failures, 3
skipped as written; and building the PendingIntent from a random UUID
instead of the request's id fails the new test and nothing else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-05 23:20:49 -05:00
10 changed files with 817 additions and 1 deletions
+22
View File
@@ -42,6 +42,28 @@
<action android:name="android.content.action.DOCUMENTS_PROVIDER" />
</intent-filter>
</provider>
<!--
A PLAIN provider, for the ffkitsaf bridge on the success path.
FFmpegKitConfig.getSafParameterForRead is on every real user conversion and was on no
passing test: they all pass Uri.fromFile, which takes the other arm. Only its failure
side was covered, by UnopenableUriTest naming an authority that does not exist.
The documents provider above cannot serve this. Any DOCUMENTS_PROVIDER must hold
MANAGE_DOCUMENTS or the platform refuses to install it, instrumentation runs in the
target app's process and so carries the app's uid, and the resulting denial says what
is actually required: access obtained through ACTION_OPEN_DOCUMENT. That means a picker,
and the flake it brings. See issue #226.
The bridge does not need a documents provider. It opens a descriptor through the
resolver and hands FFmpeg a saf: path, so any readable content:// URI exercises it, and
an ordinary provider is allowed to be exported without a permission.
-->
<provider
android:name="org.libremediaconverter.saf.FixtureContentProvider"
android:authorities="org.libremediaconverter.test.content"
android:exported="true" />
</application>
</manifest>
@@ -13,6 +13,7 @@ import androidx.work.WorkManager
import androidx.work.Worker
import androidx.work.WorkerParameters
import androidx.work.workDataOf
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
@@ -27,7 +28,10 @@ import org.junit.runner.RunWith
import org.libremediaconverter.join.JoinState
import org.libremediaconverter.join.JoinViewModel
import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.model.ConversionRequest
import org.libremediaconverter.model.Engine
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.QualityTier
import org.libremediaconverter.work.ConcatWorker
import org.libremediaconverter.work.ConversionWorker
import org.libremediaconverter.work.JobTags
@@ -64,6 +68,26 @@ class EchoWorker(context: Context, params: WorkerParameters) : Worker(context, p
* path, foreground service included — into a synchronous test double, depending on class order.
*/
@UnstableApi
/**
* A [SoftwareTranscoder] that holds the worker in [WorkInfo.State.RUNNING] until released.
*
* Declared here rather than in `FakeFailures` because it is the only test that needs a job to stay
* live on demand, and the shape is specific to that: the others fake a *failure*, this fakes
* *duration*.
*/
private class BlockingTranscoder(private val released: CompletableDeferred<Unit>) : SoftwareTranscoder {
override suspend fun run(
request: ConversionRequest,
inputPath: String,
output: File,
durationMs: Long,
onProgress: (Int) -> Unit,
) {
released.await()
output.writeBytes(ByteArray(1_024))
}
}
@RunWith(AndroidJUnit4::class)
class ReattachOnLaunchTest {
@@ -75,7 +99,13 @@ class ReattachOnLaunchTest {
fun clearTheQueue() = emptyQueueAndStaging()
@After
fun leaveNothingBehind() = emptyQueueAndStaging()
fun leaveNothingBehind() {
// The suite runs without Android Test Orchestrator, so every class shares one process and
// a swapped seam outlives the class that set it. Only one test here swaps one, but a
// BlockingTranscoder left in place would hang the next class that converts anything.
ConversionDependencies.reset()
emptyQueueAndStaging()
}
/**
* The claim the whole fix rests on, checked against the production request builder rather
@@ -261,6 +291,69 @@ class ReattachOnLaunchTest {
return request.id
}
/**
* Reattaching to a conversion that is **running right now**, which nothing had ever driven.
*
* This class covers a job that finished, one whose staged file is gone, an ambiguous pair, one
* still queued, and one the user cancelled. [Reattachment.rank] gives
* [WorkInfo.State.RUNNING] the **highest** rank of all — "live work outranks a finished result
* because a running job is holding a foreground service" — and no test on either source set
* ever produced one. `ReattachmentTest` exercises the ranking as a pure function over
* fabricated snapshots; what was missing is a ViewModel meeting a real running job.
*
* It is also the likeliest reattachment there is: the user starts a conversion, leaves, and
* comes back while it is still going.
*
* ## Why the engine is a fake here, and why that is not a weakening
*
* The job has to still be running when the ViewModel is built, and every real conversion in
* this suite finishes in about a second — racing that is what made the cancellation tests flaky
* enough to need retries (#224). A [SoftwareTranscoder] that blocks until released removes the
* race outright: the job is `RUNNING` for exactly as long as the test wants.
*
* Nothing about reattachment depends on which engine is transcoding. What is under test is the
* tag query, [Reattachment.choose] over live WorkManager state, and `observe` mapping it to
* [ConversionState.Converting] — all of which run identically whatever is doing the work.
*
* ## What this does not do, and cannot (#230)
*
* It does not kill the process. `docs/defect-audit.md` D3/D13 record that `am kill` refuses a
* process holding a foreground service, and there is a more basic obstacle: **instrumentation
* runs in the app's own process**, so any route that really killed it would take the test
* runner with it and there would be nothing left to assert with. A relaunch-and-observe test
* needs two instrumentation runs, which the runner does not provide.
*
* So process death stays device-manual, and this is the closest observable analogue: a fresh
* ViewModel, with no memory of the work, meeting a job that is genuinely mid-flight.
*/
@Test
fun reattachesToAConversionThatIsStillRunning(): Unit = runBlocking {
val released = CompletableDeferred<Unit>()
ConversionDependencies.software = { BlockingTranscoder(released) }
val request = ConversionWorker.request(
inputUri = Uri.fromFile(stage("running_input.mp3")),
displayName = RUNNING_NAME,
sizeBytes = RUNNING_SIZE,
spec = OutputFormat.MP3.spec,
quality = QualityTier.FAST,
)
workManager.enqueue(request).result.get()
// Deterministic: the worker cannot finish until this test lets it.
withTimeout(TIMEOUT_MS) {
workManager.getWorkInfoByIdFlow(request.id).first { it?.state == WorkInfo.State.RUNNING }
}
val reattached = awaitConversion<ConversionState.Converting>()
assertEquals(RUNNING_NAME, reattached.input.displayName)
assertEquals(RUNNING_SIZE, reattached.input.sizeBytes)
released.complete(Unit)
workManager.cancelWorkById(request.id).result.get()
}
/**
* Enqueues a job that stays [WorkInfo.State.ENQUEUED]. The delay is what holds it there: it
* is long enough that nothing can run it during a test, and it is cancelled either way.
@@ -326,5 +419,9 @@ class ReattachOnLaunchTest {
* against WorkManager's database, so this is generous rather than tuned.
*/
const val SETTLE_MS = 5_000L
/** Read back off the job's tags by the reattaching ViewModel, so both have to survive. */
const val RUNNING_NAME = "still_running.mp3"
const val RUNNING_SIZE = 4_242L
}
}
@@ -4,10 +4,20 @@ 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
import org.junit.Assert.fail
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
@@ -191,6 +201,108 @@ class FFmpegEngineTest {
)
}
/**
* 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. 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 proves the session is running, but arrives too late to interrupt anything.
* `FFmpegKit.listSessions` shows the session [SessionState.RUNNING] far earlier.
*
* ## Why it retries, which is the part that took two attempts to get right
*
* Waiting for `RUNNING` is not on its own enough. With `MP4_H265` at [QualityTier.BEST] this
* passed four consecutive local runs and all five CI legs, then failed on the API 34 and 35 legs
* of the next PR with `state=COMPLETED rc=0`. Nothing had changed: on a loaded runner the thread
* that observed `RUNNING` can be descheduled long enough for a short encode to finish before it
* calls `cancel`. A longer timeout does not help — the wait already succeeded.
*
* Two changes together, because neither is sufficient:
*
* - **A slower encode.** `WEBM_VP9` at `BEST` is the slowest thing this builder emits:
* `libvpx-vp9 -crf 31 -b:v 0`, with `-deadline realtime` added **only** on
* [QualityTier.FAST]. Probed on an API 34 emulator, that session is still `RUNNING` at 1 s
* and finished by 2 s, against well under a second for x265 `-preset medium`.
* - **Retrying the attempt.** An attempt whose session finished before the cancel landed has
* not tested anything, so it is not a failure — it is a miss, and it is retried. Only
* exhausting [CANCEL_ATTEMPTS] is a failure, and its message says which case it hit.
*
* That keeps the mutation honest: with `FFmpegKit.cancel` removed **every** attempt ends
* `COMPLETED`, so the test still fails — it just takes [CANCEL_ATTEMPTS] tries to say so.
*
* The session is identified by diffing against the ids present before each attempt, because
* this class has already produced eight of them by the time this executes.
*/
@Test
fun cancellingARunningConversionCancelsTheNativeSession(): Unit = runBlocking {
val outcomes = mutableListOf<String>()
repeat(CANCEL_ATTEMPTS) { attempt ->
val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
val out = outputFor("out_cancelled_$attempt.webm")
val job = launch(Dispatchers.IO) {
engine.run(
// The slowest target this builder emits -- see the KDoc. Not decoration:
// with a faster one this loses the race on a loaded CI runner.
request = ConversionRequest(spec = OutputFormat.WEBM_VP9.spec, quality = QualityTier.BEST),
inputPath = input.absolutePath,
output = out,
durationMs = 3_000,
)
}
val ours = withTimeout(TIMEOUT_MS) {
var found: FFmpegSession? = null
while (found == null) {
found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
if (found == null) delay(POLL_MS)
}
found
}
job.cancelAndJoin()
withTimeout(TIMEOUT_MS) {
while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
}
if (ReturnCode.isCancel(ours.getReturnCode())) return@runBlocking
// The encode beat us to it. That attempt proved nothing either way, so try again.
outcomes += "state=${ours.getState()} rc=${ours.getReturnCode()}"
}
fail(
"never interrupted a running session in $CANCEL_ATTEMPTS attempts, so either every " +
"encode finished first or cancellation does not reach it: $outcomes",
)
}
// --- the quality tier the GPL licence was taken for --------------------
@Test
@@ -228,4 +340,19 @@ 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
/**
* How many times to try to catch the session mid-encode.
*
* Each miss costs about the length of one VP9 encode -- a second or two -- and a miss is
* the loaded-runner case rather than a defect. Five is enough that exhausting them means
* cancellation is not reaching the session, which is what the failure message says.
*/
const val CANCEL_ATTEMPTS = 5
}
}
@@ -5,10 +5,20 @@ 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
import org.junit.Assert.fail
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
@@ -16,6 +26,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 +61,82 @@ 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.
*
* **And the attempt is retried**, for the reason the conversion side measured the hard way: on
* a loaded runner the thread that observed `RUNNING` can be descheduled long enough for a short
* encode to finish before it calls `cancel`, which failed two CI legs there. An attempt whose
* session finished first has tested nothing, so it is a miss rather than a failure; only
* exhausting [CANCEL_ATTEMPTS] fails, and with `FFmpegKit.cancel` removed every attempt misses,
* so the mutation still bites.
*
* 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 outcomes = mutableListOf<String>()
repeat(CANCEL_ATTEMPTS) { attempt ->
val before = FFmpegKit.listSessions().map { it.getSessionId() }.toSet()
val out = output("cancelled_join_$attempt.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 == null) {
found = FFmpegKit.listSessions().firstOrNull { it.getSessionId() !in before }
if (found == null) delay(POLL_MS)
}
found
}
job.cancelAndJoin()
withTimeout(TIMEOUT_MS) {
while (ours.getState() == SessionState.RUNNING) delay(POLL_MS)
}
if (ReturnCode.isCancel(ours.getReturnCode())) return@runBlocking
outcomes += "state=${ours.getState()} rc=${ours.getReturnCode()}"
}
fail(
"never interrupted a running join in $CANCEL_ATTEMPTS attempts, so either every " +
"encode finished first or cancellation does not reach it: $outcomes",
)
}
private fun copyAsset(name: String): File {
val out = File(context.cacheDir, name)
InstrumentationRegistry.getInstrumentation().context.assets
@@ -180,4 +267,13 @@ 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
/** See the conversion side: a miss is the loaded-runner case, not a defect. */
const val CANCEL_ATTEMPTS = 5
}
}
@@ -0,0 +1,123 @@
package org.libremediaconverter.saf
import androidx.media3.common.util.UnstableApi
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import androidx.work.WorkInfo
import androidx.work.WorkManager
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.ffmpeg.ConcatEngine
import org.libremediaconverter.model.Engine
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.QualityTier
import org.libremediaconverter.work.ConversionWorker
import java.io.File
/**
* A `content://` input reaching FFmpeg successfully, which nothing had ever driven (#225).
*
* `FFmpegKitConfig.getSafParameterForRead` stands between a SAF grant and the native process, and
* it is on **every real user conversion**. Every passing convert and join test in this suite hands
* the worker a `Uri.fromFile(...)`, which takes the `uri.path` arm instead — so the bridge was
* exercised only on its failure side, by `UnopenableUriTest` naming an authority that does not
* exist. That proves the error message, not the bridge.
*
* ## Why a plain provider rather than the documents one
*
* [FixtureDocumentsProvider] cannot be reached from the app, measured three ways on an API 34
* emulator (#226): a `DOCUMENTS_PROVIDER` declared without `MANAGE_DOCUMENTS` is refused at install
* — *"Provider must be protected by MANAGE_DOCUMENTS"*; instrumentation runs in the **target app's
* process**, so `Instrumentation.getContext()` still carries the app's uid and is denied; and
* `adoptShellPermissionIdentity(MANAGE_DOCUMENTS)` is denied identically. The denial names the only
* way in: *"you obtain access using ACTION_OPEN_DOCUMENT or related APIs"*.
*
* The bridge does not need one. It opens a descriptor through the resolver and hands FFmpeg a
* `saf:` path, so any readable `content://` URI exercises it — and [FixtureContentProvider] is an
* ordinary provider, which may be exported without a permission. The whole class is headless: no
* DocumentsUI, and none of the flake #190 records.
*
* ## Why MP3
*
* The bridge lives on the FFmpeg arm, and MP3 is the format the router sends there unconditionally
* — no platform encoder exists at any API level, so `ConversionWorkerTest.routesAnMp3JobToFfmpeg…`
* relies on the same fact. Choosing a video target would make the engine depend on the device's
* codecs, and #223 is what that costs.
*
* *Mutation:* make `getSafParameterForRead` return `uri.toString()`. FFmpeg cannot open it and both
* tests fail; nothing else in either suite notices.
*/
@UnstableApi
@RunWith(AndroidJUnit4::class)
class ContentUriInputTest {
private val context = InstrumentationRegistry.getInstrumentation().targetContext
private val workManager = WorkManager.getInstance(context)
@After
fun tearDown() {
File(context.cacheDir, "conversions").listFiles()?.forEach { it.delete() }
}
@Test
fun aContentUriInputConvertsThroughTheSafBridge(): Unit = runBlocking {
val input = FixtureContentProvider.uriFor(SAMPLE)
val request = ConversionWorker.request(
inputUri = input,
displayName = SAMPLE,
sizeBytes = 0L,
spec = OutputFormat.MP3.spec,
quality = QualityTier.FAST,
)
workManager.enqueue(request).result.get()
val terminal = withTimeout(TIMEOUT_MS) {
workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
}
val error = terminal?.outputData?.getString(ConversionWorker.KEY_ERROR)
assertEquals(
"a content:// input must convert, but failed with: $error",
WorkInfo.State.SUCCEEDED,
terminal?.state,
)
// The bridge is on the FFmpeg arm only, so this is part of the claim rather than colour.
assertEquals(Engine.FFMPEG.name, terminal?.outputData?.getString(ConversionWorker.KEY_ENGINE_USED))
val out = File(terminal!!.outputData.getString(ConversionWorker.KEY_OUTPUT_PATH)!!)
assertTrue("no output produced from a content:// input", out.exists() && out.length() > 0)
out.delete()
}
/**
* The same bridge on the join path, which has its own copy of the call (`ConcatEngine:36`).
*
* Driven through the engine rather than `ConcatWorker` because the engine is where the branch
* is; the worker adds a foreground service and nothing else this is about.
*/
@Test
fun contentUriInputsJoinThroughTheSafBridge(): Unit = runBlocking {
val out = File(context.cacheDir, "joined_from_content.mp4").apply { delete() }
val result = ConcatEngine(context).join(
listOf(FixtureContentProvider.uriFor(CLIP_A), FixtureContentProvider.uriFor(CLIP_B)),
out,
OutputFormat.MP4_H264,
)
assertTrue("no output produced from content:// inputs", result.output.length() > 0)
out.delete()
}
private companion object {
const val SAMPLE = "sample_h264.mp4"
const val CLIP_A = "clip_a.mp4"
const val CLIP_B = "clip_b.mp4"
const val TIMEOUT_MS = 300_000L
}
}
@@ -0,0 +1,135 @@
package org.libremediaconverter.saf;
import android.content.ContentProvider;
import android.content.ContentValues;
import android.database.Cursor;
import android.database.MatrixCursor;
import android.net.Uri;
import android.os.ParcelFileDescriptor;
import android.provider.OpenableColumns;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
/**
* A plain {@link ContentProvider} serving the committed media fixtures over {@code content://}.
*
* <p><b>Why this exists alongside {@link FixtureDocumentsProvider}.</b> Every passing convert and
* join test hands the worker a {@code Uri.fromFile(...)}, which takes the {@code uri.path} arm and
* never touches {@code FFmpegKitConfig.getSafParameterForRead}. That bridge is on 100% of real user
* conversions and was on 0% of tested ones; only its failure side was covered, by
* {@code UnopenableUriTest} pointing at an authority that does not exist.
*
* <p><b>Why not the documents provider.</b> It cannot be reached. Measured three ways on an API 34
* emulator: a {@code DOCUMENTS_PROVIDER} declared without {@code MANAGE_DOCUMENTS} is refused at
* install ("Provider must be protected by MANAGE_DOCUMENTS"); instrumentation runs in the target
* app's process, so {@code Instrumentation.getContext()} still carries the app's uid and is denied;
* and {@code adoptShellPermissionIdentity(MANAGE_DOCUMENTS)} is denied identically. The denial says
* what is required — <i>"you obtain access using ACTION_OPEN_DOCUMENT or related APIs"</i> — so a
* documents provider is reachable only through a picker-issued grant. See issue #226.
*
* <p>The bridge does not need one. {@code getSafParameterForRead} opens a file descriptor through
* the resolver and hands FFmpeg a {@code saf:} path; any readable {@code content://} URI exercises
* it. An ordinary provider may be exported without a permission, so this one is, and the whole test
* stays headless — no DocumentsUI, and none of the flake #190 records.
*
* <p>Unlike {@link FixtureDocumentsProvider} this may use {@code androidx} and Kotlin freely — it is
* loaded into the app process like any other provider, not into the bare test process. It is kept
* in Java anyway, next to its sibling, so the two read alike.
*/
public final class FixtureContentProvider extends ContentProvider {
/** Authority. Distinct from the documents provider's, and from anything the app declares. */
public static final String AUTHORITY = "org.libremediaconverter.test.content";
/** Builds a URI for one of this source set's committed assets, e.g. {@code sample_h264.mp4}. */
public static Uri uriFor(String assetName) {
return new Uri.Builder().scheme("content").authority(AUTHORITY).appendPath(assetName).build();
}
@Override
public boolean onCreate() {
return true;
}
@Override
public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException {
if (!"r".equals(mode)) {
throw new FileNotFoundException("this provider is read-only: " + mode);
}
return ParcelFileDescriptor.open(unpack(assetOf(uri)), ParcelFileDescriptor.MODE_READ_ONLY);
}
/**
* Enough of {@link OpenableColumns} for {@code InputQuery.describe} to name and size the input.
*
* <p>Without these the app reaches the "Size unknown" screen, which is a different test.
*/
@Override
public Cursor query(Uri uri, String[] projection, String selection, String[] args, String sort) {
String asset = assetOf(uri);
File file;
try {
file = unpack(asset);
} catch (FileNotFoundException e) {
return null;
}
MatrixCursor cursor = new MatrixCursor(
new String[] {OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE});
cursor.newRow().add(OpenableColumns.DISPLAY_NAME, asset).add(OpenableColumns.SIZE, file.length());
return cursor;
}
@Override
public String getType(Uri uri) {
return assetOf(uri).endsWith(".m4a") ? "audio/mp4" : "video/mp4";
}
@Override
public Uri insert(Uri uri, ContentValues values) {
throw new UnsupportedOperationException("read-only fixture provider");
}
@Override
public int delete(Uri uri, String selection, String[] args) {
throw new UnsupportedOperationException("read-only fixture provider");
}
@Override
public int update(Uri uri, ContentValues values, String selection, String[] args) {
throw new UnsupportedOperationException("read-only fixture provider");
}
private static String assetOf(Uri uri) {
String asset = uri.getLastPathSegment();
return asset == null ? "" : asset;
}
/**
* The asset on disk, unpacked the first time anything asks.
*
* <p>Reported as {@link FileNotFoundException} rather than swallowed: a provider answering with
* a zero-byte file would fail the conversion for a reason nothing states.
*/
private File unpack(String asset) throws FileNotFoundException {
File file = new File(getContext().getCacheDir(), "provided_" + asset);
if (file.length() > 0L) {
return file;
}
try (InputStream source = getContext().getAssets().open(asset);
OutputStream sink = new FileOutputStream(file)) {
byte[] buffer = new byte[8192];
int read;
while ((read = source.read(buffer)) != -1) {
sink.write(buffer, 0, read);
}
} catch (IOException e) {
throw new FileNotFoundException("could not unpack " + asset + ": " + e);
}
return file;
}
}
@@ -0,0 +1,129 @@
package org.libremediaconverter.work
import android.net.Uri
import androidx.media3.common.util.UnstableApi
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import androidx.work.OneTimeWorkRequestBuilder
import androidx.work.WorkInfo
import androidx.work.WorkManager
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withTimeout
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.OutputFormat
import org.libremediaconverter.model.QualityTier
import java.io.File
import java.util.concurrent.TimeUnit
/**
* The Cancel button in the notification shade actually cancels the job.
*
* `ConversionNotifications.build` attaches one action, wired to
* `WorkManager.createCancelPendingIntent(id)`. Before this test `createCancelPendingIntent` had
* **no references anywhere outside its own declaration** — no JVM test, no instrumented test
* (#227).
*
* That matters more than an ordinary uncovered line. A conversion runs in a foreground service and
* the user is invited to leave the app; once they do, this action is the only way to stop it. If
* the `PendingIntent` carries the wrong id, the button does nothing, the notification stays, and
* the job runs to completion — with no error, no log, and no screen to look at.
*
* ## Why this fires the intent rather than reading the shade
*
* The obvious version asks `NotificationManager.getActiveNotifications()` for id 1001 and taps what
* it finds. That was rejected: the instrumented suite grants no runtime permissions, so
* `POST_NOTIFICATIONS` is denied throughout, and whether a suppressed foreground-service
* notification is returned there is a platform detail that varies — the test would be asserting
* something about notification *visibility* rather than about cancellation.
*
* The `PendingIntent` is the subject; where it is read from is incidental. Building the
* notification for a real, live work id and firing its action exercises exactly the thing that can
* be wrong — a real `PendingIntent` dispatch reaching real `WorkManager` — and does it the same way
* on every API level.
*
* ## Why the job is delayed rather than running
*
* A conversion of the committed 3 s fixture finishes in well under a second on an emulator
* (`HardwareFallbackTest` completed one in 448 ms), so racing a cancel against a running job would
* be flaky in the direction that fails. An initial delay keeps the job reliably `ENQUEUED`, which
* is a state `cancelWorkById` acts on identically — what is under test is whether firing the action
* reaches WorkManager with the right id, not which state it interrupts.
*
* *Mutation:* build the `PendingIntent` from `UUID.randomUUID()` instead of the request's id. The
* notification looks identical and the job is never cancelled.
*/
@UnstableApi
@RunWith(AndroidJUnit4::class)
class NotificationCancelActionTest {
private val context = InstrumentationRegistry.getInstrumentation().targetContext
private val workManager = WorkManager.getInstance(context)
private lateinit var input: File
@Before
fun setUp() {
input = File(context.cacheDir, "cancel_action_sample.mp4")
InstrumentationRegistry.getInstrumentation().context.assets
.open("sample_h264.mp4")
.use { asset -> input.outputStream().use { asset.copyTo(it) } }
}
@After
fun tearDown() {
input.delete()
File(context.cacheDir, "conversions").listFiles()?.forEach { it.delete() }
}
@Test
fun theNotificationsCancelActionCancelsThatJob(): Unit = runBlocking {
val request = ConversionWorker.request(
inputUri = Uri.fromFile(input),
displayName = input.name,
sizeBytes = input.length(),
spec = OutputFormat.MP4_H264.spec,
quality = QualityTier.FAST,
).let { base ->
// Rebuild with a delay so the job stays ENQUEUED for the whole test. See the KDoc.
OneTimeWorkRequestBuilder<ConversionWorker>()
.setInputData(base.workSpec.input)
.setInitialDelay(1, TimeUnit.HOURS)
.build()
}
workManager.enqueue(request).result.get()
// The job is queued and waiting, which is the state the cancel has to interrupt.
assertEquals(
WorkInfo.State.ENQUEUED,
withTimeout(TIMEOUT_MS) {
workManager.getWorkInfoByIdFlow(request.id).first { it != null }
}?.state,
)
val notification = ConversionNotifications(context)
.build(request.id, title = input.name, percent = 0, indeterminate = true)
val action = notification.actions?.firstOrNull()
assertNotNull("the progress notification carries no action to cancel with", action)
// The whole point: fire it the way the shade would, and see the job stop.
action!!.actionIntent.send()
val terminal = withTimeout(TIMEOUT_MS) {
workManager.getWorkInfoByIdFlow(request.id).first { it != null && it.state.isFinished }
}
assertEquals(
"firing the notification's Cancel action must cancel the job it was built for",
WorkInfo.State.CANCELLED,
terminal?.state,
)
}
private companion object {
const val TIMEOUT_MS = 30_000L
}
}
@@ -35,6 +35,21 @@ object FFmpegConcatCommand {
add("concat")
add("-safe")
add("0")
// And -protocol_whitelist permits the *scheme* those paths carry, which is a
// separate gate (#238). Every input the user actually picks is a content:// URI --
// JoinScreen uses OpenMultipleDocuments -- so ConcatEngine maps it through
// FFmpegKitConfig.getSafParameterForRead and writes an `ffkitsaf:` path into the
// list file. The concat demuxer applies its own whitelist, defaulting to
// "file,crypto,data", and refused every one of them:
//
// [ffkitsaf @ ...] Protocol 'ffkitsaf' not on whitelist 'file,crypto,data'!
//
// This only widens that default. It is on the stream-copy branch alone because it
// is the only one that feeds the demuxer a list file -- REENCODE passes each input
// with its own -i, where the whitelist does not apply, which is why joining over SAF
// worked for mismatched clips and failed for matching ones.
add("-protocol_whitelist")
add(PROTOCOL_WHITELIST)
add("-i")
add(listFile.absolutePath)
add("-c")
@@ -84,4 +99,12 @@ object FFmpegConcatCommand {
add(output.absolutePath)
}
}
/**
* The concat demuxer's protocol whitelist: FFmpeg's own default, plus ffmpeg-kit's SAF scheme.
*
* Spelled out rather than appended to an unknown default, because the default is FFmpeg's and
* could change under us; naming all four keeps the command self-describing. See #238.
*/
private const val PROTOCOL_WHITELIST = "file,crypto,data,ffkitsaf"
}
@@ -6,6 +6,7 @@ import android.net.Uri
import androidx.activity.ComponentActivity
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
import androidx.compose.ui.test.onAllNodesWithTag
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.performClick
import androidx.media3.common.util.UnstableApi
@@ -77,6 +78,28 @@ class LauncherWiringTest {
* The transposition guard. A picked file has to reach `onInputPicked`, which is observable as
* the screen arriving at `Ready` with the file card showing — `save()` from `Idle` returns at
* its own guard and leaves nothing behind.
*
* ## Why this waits rather than asserting straight away (#220)
*
* `onInputPicked` does not reach `Ready` on the calling thread. It hops twice —
* `withContext(pickDispatcher) { InputQuery.describe(...) }` and then the probe — and
* `pickDispatcher` defaults to `Dispatchers.IO`, a real background thread that Compose's
* idling does not know about. `deliver` therefore returns with the state still `Idle` more
* often than not, and asserting immediately was a race the test usually won.
*
* It lost five times on CI in one day, on PRs whose diffs were instrumented tests and
* documentation, which is what #220 was filed for. `waitUntil` polls through
* `waitForIdle`, so it drains the main looper each time round and sees the recomposition that
* the IO hop eventually posts back.
*
* **Injecting the dispatcher would be better and is not available here.** `pickDispatcher` is
* a constructor parameter precisely so a test can pin it, but this test composes the real
* `ConverterScreen`, which resolves its own ViewModel through `viewModel()` — the seam exists
* one layer below the thing under test. Pinning it would mean not testing the launcher edge,
* which is the whole point of this class.
*
* The wait does not weaken the assertion: transposing the two callbacks leaves the screen in
* `Idle` forever, so it fails on the timeout with the same meaning it failed with before.
*/
@Test
fun `a picked document is loaded as input rather than saved to`() {
@@ -85,6 +108,11 @@ class LauncherWiringTest {
composeRule.onNodeWithTag(TestTags.Converter.CHOOSE_FILE).performClick()
deliver(Uri.parse("content://test/holiday.mkv"))
composeRule.waitUntil(PICK_TIMEOUT_MS) {
composeRule.onAllNodesWithTag(TestTags.Converter.FILE_CARD_NAME)
.fetchSemanticsNodes()
.isNotEmpty()
}
composeRule.onNodeWithTag(TestTags.Converter.FILE_CARD_NAME).assertIsDisplayed()
}
@@ -138,4 +166,13 @@ class LauncherWiringTest {
)
composeRule.waitForIdle()
}
private companion object {
/**
* Long enough that a slow CI runner is not the reason this fails, short enough that a
* genuinely transposed callback does not stall the suite. The pick normally lands in
* single-digit milliseconds.
*/
const val PICK_TIMEOUT_MS = 10_000L
}
}
@@ -56,6 +56,33 @@ class FFmpegConcatCommandTest {
assertEquals("0", args[args.indexOf("-safe") + 1])
}
/**
* The gate that `-safe 0` does not open, and the one every real join needs (#238).
*
* `-safe 0` permits absolute *paths*; the concat demuxer separately whitelists the *protocol*,
* defaulting to `file,crypto,data`. `JoinScreen` picks with `OpenMultipleDocuments`, so real
* inputs are `content://` and `ConcatEngine` writes `ffkitsaf:` paths into the list file — which
* the demuxer refused outright, failing every stream-copy join a user could actually start.
*
* The re-encode strategy has no equivalent assertion because it needs none: it passes each
* input with its own `-i` and never feeds the demuxer a list file. That asymmetry is exactly
* why the defect survived — joining mismatched clips over SAF worked.
*/
@Test
fun `stream copy whitelists the protocol its list file entries actually use`() {
val args = FFmpegConcatCommand.build(
ConcatStrategy.STREAM_COPY,
inputs,
listFile,
output,
OutputFormat.MP4_H264,
)
val whitelist = args[args.indexOf("-protocol_whitelist") + 1].split(",")
assertTrue("ffmpeg-kit's SAF scheme must be permitted, got $whitelist", "ffkitsaf" in whitelist)
// The defaults have to survive too: the list file itself is opened over `file`.
assertTrue("the demuxer still reads the list file itself, got $whitelist", "file" in whitelist)
}
@Test
fun `re-encode passes every input separately and builds a filter graph`() {
val args = FFmpegConcatCommand.build(