Tell an unknown input size apart from an empty file
`queryFile` started at `var size = 0L` and only moved off it when a provider answered the
`OpenableColumns.SIZE` column, which the platform documents providers *may* omit. So "this file
is empty" and "nobody would tell me how big it is" reached `OutputPublisher.hasSpaceFor` as the
same number, and `hasSpaceFor(0)` is not a space check -- it is "is there 128 MB free", which any
phone with a working camera passes.
The reachable value is not an inference. On a Pixel 10 Pro XL, `contentResolver.query` on a
`file://` URI returns null outright, so the cursor block never runs at all and the default
survives untouched: `queryFile gave displayName='input' sizeBytes=0`. Robolectric reproduces that
exactly -- null query, and a descriptor that reports 4321 bytes for the same file -- which is why
every test here is a JVM test rather than a device one.
`InputQuery` replaces the two copies of `queryFile`, which were byte for byte identical in
`ConversionViewModel` and `JoinViewModel`, so a fix to either would have been a fix to half the
app. It asks for the size twice: what the provider says, and then what the file itself says
through `openFileDescriptor(uri, "r").statSize`. The second needs no cooperation beyond the input
being openable, which a conversion is about to require anyway, and it is what answers the device
case above. Only when both decline is the answer null, and `InputFile.sizeBytes` is `Long?` so
that null cannot be spelled the same way as zero again.
**What an unknown size does was the decision, and it is deliberately not a refusal.**
`OutputPublisher.hasSpaceForUnknownSize()` produces the same number the defect produced by
accident -- with no size to reserve for, the headroom is all there is left to check -- and that is
worth saying plainly rather than dressing up. What changed is that it is now the answer to a
question that was asked. `hasSpaceFor` means "there is room for this many bytes" and nothing else
claims it.
Refusing was the obvious alternative and would have been worse than the bug: it turns "no
provider answered the SIZE column" into "this file cannot be converted", for a user who can do
nothing about either. A fixed floor was the other, and there is no honest number for it -- a
1 GB floor refuses a 10 MB conversion on a device with 500 MB free, which is the same failure in
a costume. `SpaceCheckTest` pins the choice from both sides: an unmeasurable input must not end
the job, and a full disk must still refuse it.
That second half is why the default answers *through* `hasSpaceFor`. `FakeFailures.FullDisk` in
the instrumented suite overrides `hasSpaceFor` and nothing else, so the delegation is the only
reason it still refuses an unknown-size job. Replacing the delegation with a bare `true` leaves
the full-disk test red with `expected:<Failure {error : Not enough free space to convert.}> but
was:<Failure {error : FFmpegKit failed to start on brand: robolectric...}>` -- the job sailed past
the guard and died at the engine instead.
Both workers get the same shape. The size arrives as input `Data`, which has no null, so the
absence of the key *is* the unknown -- `getLong(key, 0L)` was the other half of the conflation.
When it is absent the worker measures the input itself, which it can do because it holds the URI:
that covers a `request(...)` built by hand and work enqueued before the size became optional, and
it costs an ordinary job nothing because it runs only on the fallback. `ConversionWorker.request`
writes neither the `Data` entry nor the `JobTags.sizeBytes` tag for a size nobody knows, since a
tag reading `size-bytes:0` would come back through `Reattachment` as a confident claim that the
user's file is empty -- and `reattach()`'s `?: 0L` is gone for the same reason.
A join's total is `InputQuery.total`, which is null the moment a *single* input cannot be sized.
Summing the ones that answered was the competing reading and is rejected: a lower bound is
indistinguishable from a real total once it reaches the space check, so the guard would reserve
for half the job and pass. Reverting it to `sumOf { it ?: 0L }` records `[1111]` for a two-file
join whose second input nothing can measure.
Work already in the queue keeps the old conflation, and there is no fixing it. The previous
`request()` always wrote `putLong(KEY_SIZE_BYTES, sizeBytes)`, so a job enqueued before this
commit for a file nothing could size carries the key *present* and set to zero -- which reads
back as a declared size of zero and is trusted, exactly as before. Its `lmc.size-bytes:0` tag
reads back the same way, so `reattach()` shows such a card "0 B" rather than "Size unknown".
Nothing can separate that from a genuinely empty file after the fact, and a rule that treated a
declared zero as suspect would only rebuild the conflation facing the other way. WorkManager
keeps finished work for about a week, so this is a bounded window that clears itself; new work
never enters it.
`hasSpaceFor`'s KDoc claimed peak usage was "roughly input + output at once" while the arithmetic
reserved `input + 128 MB`. The arithmetic is what stays and the doc now says why: `bytes` is the
input's size standing in for the output's, generous for the ordinary conversion (which is asked
for precisely because it shrinks its input) and short for a re-encode to a bulkier codec; the
128 MB absorbs that error and the transient double copy while `publish` runs. Reserving
`input + output` outright would refuse jobs that fit. This is a pre-flight check that stops an
obviously impossible job from spending minutes finding out, not a guarantee -- a conversion that
runs out of space anyway still fails through its engine.
The measurement side of that line is untouched on purpose. `StorageManager.getAllocatableBytes`
is a separate entry with its own device evidence and its own `informational += "UsableSpace"` in
the lint block; this commit is about the number going *in*. `hasSpaceFor(bytes: Long)` keeps its
signature and stays open, so nothing overriding it had to change.
The file card says "Size unknown" rather than `0 B`. Handled at the call site rather than inside
`formatBytes`, because a formatter that invented a number would be the defect on screen; the card
already degrades in words for a file nothing could read.
Five of the seven new tests were red before a line of production code moved, with the numbers
they were about: `expected:<4321> but was:<0>` for a picked file, `expected null, but was:<0>` for
one nothing can measure, `expected:<[1111, 2222]> but was:<[0, 0]>` for the join picker, and
`expected:<[4321]> but was:<[0]>` and `expected:<[3333]> but was:<[0]>` for what the two workers
asked the space check. The tests assert on the *question* rather than the verdict, which matters:
one that only checked whether the job ran would have passed against the defect, since the defect
is that the guard is vacuous rather than that it refuses.
The remaining two needed the new call to exist first, so each was proved by mutation instead.
Answering the unknown with `hasSpaceFor(0L)` inline leaves `expected:<[]> but was:<[0]>` in both
workers; refusing it instead leaves `an unknown size must not end the job; got Failure {error :
Not enough free space to convert.}`. Making the worker always measure rather than trust a declared
size leaves `expected:<[9999]> but was:<[4321]>`.
`join()`'s own use of `InputQuery.total` is tested separately from the function, because
`StagingCleanupSupport` already records what that distinction costs: a tool can be provably right
while nothing calls it. `SucceedingWorkerFactory` now keeps the input `Data` of every request that
reaches a worker -- the only place it is legible, since `WorkInfo` hands back a job's tags and its
output and never the `Data` it was built with -- and the test reads the enqueued total off it.
Restoring `inputs.sumOf { it.sizeBytes ?: 0L }` leaves every other test in the change green and
this one red with `a total that could not be worked out must not be enqueued as a number`.
`ConversionViewModelProbeFailureTest` expected `InputFile(INPUT, "input", 0L)` for an authority no
provider serves. It expects `sizeBytes = null` now, which is the behaviour change stated where a
reader will meet it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2,7 +2,6 @@ package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import android.provider.OpenableColumns
|
||||
import android.util.Log
|
||||
import androidx.lifecycle.AndroidViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
@@ -53,7 +52,15 @@ data class ConversionSettings(
|
||||
data class InputFile(
|
||||
val uri: Uri,
|
||||
val displayName: String,
|
||||
val sizeBytes: Long,
|
||||
/**
|
||||
* How big the file is, or null when nothing could say.
|
||||
*
|
||||
* Nullable rather than `0L`, and that is the point of it. The two were the same value before,
|
||||
* so an unmeasurable file arrived at the space check claiming to be empty. [InputQuery] owns
|
||||
* how the answer is found and what it means; every reader of this has to decide what an
|
||||
* unknown size does, which is exactly the decision the old default made silently.
|
||||
*/
|
||||
val sizeBytes: Long?,
|
||||
/**
|
||||
* What probing found. Null only while the probe is still running.
|
||||
*
|
||||
@@ -206,7 +213,10 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
// card its source details, and re-probing is what there is no URI for.
|
||||
uri = Uri.EMPTY,
|
||||
displayName = JobTags.displayNameOf(tags) ?: UNKNOWN_INPUT_NAME,
|
||||
sizeBytes = JobTags.sizeBytesOf(tags) ?: 0L,
|
||||
// No `?: 0L`. A job tagged before sizes were tagged at all, or one enqueued
|
||||
// for a file nothing could measure, has no size -- and answering that with
|
||||
// zero is the same conflation this whole change is about. See [InputQuery].
|
||||
sizeBytes = JobTags.sizeBytesOf(tags),
|
||||
)
|
||||
activeWorkId = reattachment.job.id
|
||||
// No initial state of our own: the flow's first emission carries the job's real
|
||||
@@ -231,7 +241,7 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
viewModelScope.launch {
|
||||
// Both the metadata query and the probe touch disk, and the probe spawns FFprobe.
|
||||
// Neither belongs on the main thread.
|
||||
val file = withContext(Dispatchers.IO) { queryFile(uri) }
|
||||
val file = withContext(Dispatchers.IO) { InputQuery.describe(getApplication(), uri) }
|
||||
// Show the file as soon as its name and size are known. Probing now runs FFprobe on
|
||||
// every pick, which is a native process spawn, and making the whole screen wait on it
|
||||
// would read as the app having ignored the tap.
|
||||
@@ -451,24 +461,6 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
else -> null
|
||||
}
|
||||
|
||||
private fun queryFile(uri: Uri): InputFile {
|
||||
var name = "input"
|
||||
var size = 0L
|
||||
getApplication<Application>().contentResolver
|
||||
.query(uri, null, null, null, null)
|
||||
?.use { cursor ->
|
||||
if (cursor.moveToFirst()) {
|
||||
cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)
|
||||
.takeIf { it >= 0 }
|
||||
?.let { name = cursor.getString(it) ?: name }
|
||||
cursor.getColumnIndex(OpenableColumns.SIZE)
|
||||
.takeIf { it >= 0 }
|
||||
?.let { size = cursor.getLong(it) }
|
||||
}
|
||||
}
|
||||
return InputFile(uri, name, size)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/**
|
||||
* Shown for a reattached job whose tags predate them — work enqueued by an earlier
|
||||
|
||||
@@ -422,7 +422,13 @@ private fun FileCard(input: InputFile) {
|
||||
Card(modifier = Modifier.fillMaxWidth()) {
|
||||
Column(modifier = Modifier.padding(16.dp)) {
|
||||
Text(input.displayName, style = MaterialTheme.typography.titleMedium)
|
||||
Text(formatBytes(input.sizeBytes), style = MaterialTheme.typography.bodySmall)
|
||||
// The null is handled here rather than inside formatBytes, because "no provider would
|
||||
// say" is not a number and a formatter that invented one -- "0 B" -- is the defect
|
||||
// this card would be showing. It degrades in words, like the codec rows below it.
|
||||
Text(
|
||||
input.sizeBytes?.let(::formatBytes) ?: "Size unknown",
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
)
|
||||
|
||||
val probe = input.probe
|
||||
if (probe == null) {
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.content.Context
|
||||
import android.database.Cursor
|
||||
import android.net.Uri
|
||||
import android.provider.OpenableColumns
|
||||
import android.util.Log
|
||||
|
||||
/**
|
||||
* What the app can find out about a picked file before an engine opens it.
|
||||
*
|
||||
* One place rather than two: `queryFile` existed in `ConversionViewModel` and `JoinViewModel`
|
||||
* byte for byte, so a fix to either was a fix to half the app.
|
||||
*
|
||||
* **An unknown size is null here, never zero.** That distinction is the whole point of this file.
|
||||
* The old code started at `var size = 0L` and only moved off it when a provider answered the
|
||||
* `OpenableColumns.SIZE` column, so "this file is empty" and "nobody told me how big it is"
|
||||
* reached `OutputPublisher.hasSpaceFor` as the same number — and `hasSpaceFor(0)` is only "is
|
||||
* there 128 MB free". Confirmed live on a Pixel 10 Pro XL, where `contentResolver.query` on a
|
||||
* `file://` URI returns null outright and the default survived untouched:
|
||||
* `queryFile gave displayName='input' sizeBytes=0`.
|
||||
*
|
||||
* So the size is asked for twice, in order:
|
||||
*
|
||||
* 1. **What the provider says.** `OpenableColumns.SIZE`, which documents providers *may* omit.
|
||||
* 2. **What the file itself says.** `openFileDescriptor(uri, "r")` and `statSize`, which needs
|
||||
* no cooperation from a provider beyond being openable — and the app is going to have to
|
||||
* open the input anyway, so it is not asking for anything a conversion would not need. This
|
||||
* is what answers the `file://` case above.
|
||||
*
|
||||
* Only when both decline is the answer null, and the callers each say what they do about that.
|
||||
*/
|
||||
object InputQuery {
|
||||
|
||||
/**
|
||||
* Shown when no provider names the file.
|
||||
*
|
||||
* Kept exactly as it was — it is what reaches the save dialog as `input_converted.mp4` for a
|
||||
* job whose input nothing described, and changing it here would rename files for reasons
|
||||
* unrelated to this fix.
|
||||
*/
|
||||
const val FALLBACK_DISPLAY_NAME = "input"
|
||||
|
||||
/** Everything the picker knows about [uri] the moment it is chosen. */
|
||||
fun describe(context: Context, uri: Uri): InputFile = InputFile(
|
||||
uri = uri,
|
||||
displayName = firstRow(context, uri) { it.displayNameOrNull() } ?: FALLBACK_DISPLAY_NAME,
|
||||
sizeBytes = sizeOf(context, uri),
|
||||
)
|
||||
|
||||
/**
|
||||
* How many bytes [uri] holds, or null when nothing can say.
|
||||
*
|
||||
* Public because the workers need it too, and for a reason worth stating: a worker's input
|
||||
* `Data` carries the size the *picker* found, which is missing for work enqueued before this
|
||||
* existed and for a request built by hand. The worker holds the URI, so when the number is
|
||||
* absent it can ask the file rather than assume.
|
||||
*/
|
||||
fun sizeOf(context: Context, uri: Uri): Long? = firstRow(context, uri) { it.sizeOrNull() } ?: measure(context, uri)
|
||||
|
||||
/**
|
||||
* The sum of [sizes], or null if even one of them is unknown.
|
||||
*
|
||||
* A join's total is only as good as its worst-known part. Adding up the ones that answered
|
||||
* would produce a lower bound that reads exactly like a real total, and the space check has
|
||||
* no way to tell the two apart — which is the same conflation this whole file exists to end.
|
||||
*/
|
||||
fun total(sizes: List<Long?>): Long? = sizes.fold(0L as Long?) { running, size ->
|
||||
if (running == null || size == null) null else running + size
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads [read] out of the first row of a metadata query, or null if there is no row.
|
||||
*
|
||||
* Guarded because a resolver call is a call into another app: a provider that has been
|
||||
* uninstalled, revoked its grant, or simply crashes takes the query with it, and a file
|
||||
* picker is not a place to bring the process down from.
|
||||
*/
|
||||
private fun <T> firstRow(context: Context, uri: Uri, read: (Cursor) -> T): T? = runCatching {
|
||||
context.contentResolver.query(uri, null, null, null, null)?.use { cursor ->
|
||||
if (cursor.moveToFirst()) read(cursor) else null
|
||||
}
|
||||
}.onFailure { Log.w(TAG, "Could not read metadata for $uri", it) }.getOrNull()
|
||||
|
||||
/**
|
||||
* The size according to the file descriptor, or null if it cannot be opened.
|
||||
*
|
||||
* `statSize` is `-1` for anything without a fixed length — a pipe, or a provider streaming its
|
||||
* answer — which is a different way of saying "unknown" and is treated as one.
|
||||
*/
|
||||
private fun measure(context: Context, uri: Uri): Long? = runCatching {
|
||||
context.contentResolver.openFileDescriptor(uri, "r")?.use { it.statSize }
|
||||
}.getOrNull()?.takeIf { it >= 0 }
|
||||
|
||||
private fun Cursor.displayNameOrNull(): String? =
|
||||
getColumnIndex(OpenableColumns.DISPLAY_NAME).takeIf { it >= 0 && !isNull(it) }?.let(::getString)
|
||||
|
||||
private fun Cursor.sizeOrNull(): Long? =
|
||||
getColumnIndex(OpenableColumns.SIZE).takeIf { it >= 0 && !isNull(it) }?.let(::getLong)?.takeIf { it >= 0 }
|
||||
|
||||
private const val TAG = "InputQuery"
|
||||
}
|
||||
@@ -29,13 +29,51 @@ open class OutputPublisher(private val context: Context) {
|
||||
open fun createStagingFile(name: String): File = File(stagingDir, name)
|
||||
|
||||
/**
|
||||
* True if there is room for a further [bytes], including headroom.
|
||||
* True if staging can take a further [bytes], with [SPACE_HEADROOM_BYTES] left over.
|
||||
*
|
||||
* Staging means peak usage is roughly input + output at once, so a job that would
|
||||
* just barely fit is rejected rather than failing partway through.
|
||||
* **The doc this replaces claimed peak usage was "roughly input + output at once" while the
|
||||
* arithmetic reserved `input + 128 MB`.** The arithmetic is what stays, and this says why
|
||||
* rather than the two continuing to disagree.
|
||||
*
|
||||
* [bytes] is the *input's* size standing in for the output's, because before an engine has
|
||||
* run there is no other number. It is generous for the ordinary conversion, which is asked
|
||||
* for precisely because it shrinks its input, and short for the ones that do not — a re-encode
|
||||
* to a bulkier codec, or a stream copy into a container with more overhead.
|
||||
*
|
||||
* The 128 MB absorbs that error, and one more besides: [publish] copies the staged file to
|
||||
* the user's destination, so while that runs the bytes exist twice on any destination sharing
|
||||
* this volume. Reserving `input + output` outright would have refused jobs that fit, on a
|
||||
* device where the destination is usually removable or remote.
|
||||
*
|
||||
* So this is a pre-flight check that stops a job which obviously cannot fit from spending
|
||||
* minutes discovering it — not a guarantee. A conversion that runs out of space anyway fails
|
||||
* through its engine, with a message of its own.
|
||||
*
|
||||
* Open so a test can force a full disk; see `FakeFailures` in the instrumented source set.
|
||||
*/
|
||||
open fun hasSpaceFor(bytes: Long): Boolean = stagingDir.usableSpace > bytes + SPACE_HEADROOM_BYTES
|
||||
|
||||
/**
|
||||
* The same check for a job whose input size nobody could determine — see [InputQuery].
|
||||
*
|
||||
* **This deliberately produces the same number the defect produced by accident**, which is
|
||||
* worth stating plainly: with no size to reserve for, all that is left to check is the
|
||||
* headroom. What has changed is that it is now the answer to a question that was asked. The
|
||||
* old code could not tell an unmeasurable file from an empty one, so it silently made this
|
||||
* the answer for *both*; now [hasSpaceFor] means "there is room for this many bytes" and
|
||||
* nothing else claims it.
|
||||
*
|
||||
* Refusing instead was considered and rejected. It would turn "no provider answered the
|
||||
* `SIZE` column" into "this file cannot be converted" — a worse defect than the one being
|
||||
* fixed, and one the user could do nothing about.
|
||||
*
|
||||
* The default answers *through* [hasSpaceFor], which is what keeps a publisher that refuses
|
||||
* on space — `FakeFailures.FullDisk`, which overrides `hasSpaceFor` and nothing else —
|
||||
* refusing this too. `SpaceCheckTest` pins that delegation, because an override here that
|
||||
* stopped delegating would quietly stop honouring a full disk.
|
||||
*/
|
||||
open fun hasSpaceForUnknownSize(): Boolean = hasSpaceFor(0L)
|
||||
|
||||
/**
|
||||
* Copies a finished staging file into a user-chosen SAF destination.
|
||||
*
|
||||
|
||||
@@ -2,7 +2,6 @@ package org.libremediaconverter.join
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import android.provider.OpenableColumns
|
||||
import androidx.lifecycle.AndroidViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
@@ -18,6 +17,7 @@ import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withContext
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.InputFile
|
||||
import org.libremediaconverter.convert.InputQuery
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import org.libremediaconverter.work.JobTags
|
||||
@@ -117,7 +117,7 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
// render these individually and offer to join them. Anything that starts drawing
|
||||
// this list has to carry the names in the tags first.
|
||||
val inputs = List(JobTags.inputCountOf(tags) ?: MIN_JOIN_INPUTS) {
|
||||
InputFile(Uri.EMPTY, "", 0L)
|
||||
InputFile(Uri.EMPTY, "", sizeBytes = null)
|
||||
}
|
||||
activeWorkId = reattachment.job.id
|
||||
observe(reattachment.job.id, inputs, cancelled = JoinState.Idle)
|
||||
@@ -130,7 +130,9 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
return
|
||||
}
|
||||
viewModelScope.launch {
|
||||
val files = withContext(Dispatchers.IO) { uris.map(::queryFile) }
|
||||
val files = withContext(Dispatchers.IO) {
|
||||
uris.map { InputQuery.describe(getApplication(), it) }
|
||||
}
|
||||
_state.value = JoinState.Ready(files)
|
||||
}
|
||||
}
|
||||
@@ -139,7 +141,10 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
val inputs = (_state.value as? JoinState.Ready)?.inputs ?: return
|
||||
val request = ConcatWorker.request(
|
||||
inputs = inputs.map { it.uri },
|
||||
totalBytes = inputs.sumOf { it.sizeBytes },
|
||||
// Not `sumOf`, which cannot express what is being summed any more. A join's
|
||||
// total is only as good as its least-known part, and adding up the inputs that
|
||||
// did answer would hand the space check a lower bound it would read as a total.
|
||||
totalBytes = InputQuery.total(inputs.map { it.sizeBytes }),
|
||||
)
|
||||
activeWorkId = request.id
|
||||
workManager.enqueue(request)
|
||||
@@ -256,22 +261,6 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
_state.value = JoinState.Idle
|
||||
}
|
||||
|
||||
private fun queryFile(uri: Uri): InputFile {
|
||||
var name = "input"
|
||||
var size = 0L
|
||||
getApplication<Application>().contentResolver
|
||||
.query(uri, null, null, null, null)
|
||||
?.use { cursor ->
|
||||
if (cursor.moveToFirst()) {
|
||||
cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME).takeIf { it >= 0 }
|
||||
?.let { name = cursor.getString(it) ?: name }
|
||||
cursor.getColumnIndex(OpenableColumns.SIZE).takeIf { it >= 0 }
|
||||
?.let { size = cursor.getLong(it) }
|
||||
}
|
||||
}
|
||||
return InputFile(uri, name, size)
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/**
|
||||
* Used when a reattached job carries no count tag — work enqueued by an earlier version
|
||||
|
||||
@@ -9,9 +9,11 @@ import androidx.work.Data
|
||||
import androidx.work.ForegroundInfo
|
||||
import androidx.work.OneTimeWorkRequestBuilder
|
||||
import androidx.work.WorkerParameters
|
||||
import androidx.work.hasKeyWithValueOfType
|
||||
import androidx.work.workDataOf
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.InputQuery
|
||||
import org.libremediaconverter.convert.StagingNames
|
||||
import org.libremediaconverter.ffmpeg.ConcatEngine
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
@@ -39,12 +41,16 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
if (uris.size < 2) {
|
||||
return Result.failure(workDataOf(KEY_ERROR to "Pick at least two files to join."))
|
||||
}
|
||||
val totalBytes = inputData.getLong(KEY_TOTAL_BYTES, 0L)
|
||||
// Absent, not zero, when the picker could not size every input -- see the same read in
|
||||
// ConversionWorker and InputQuery for why the two are no longer one number.
|
||||
val declaredTotal = inputData
|
||||
.takeIf { it.hasKeyWithValueOfType<Long>(KEY_TOTAL_BYTES) }
|
||||
?.getLong(KEY_TOTAL_BYTES, 0L)
|
||||
val format = OutputFormat.valueOf(
|
||||
inputData.getString(KEY_FORMAT) ?: DEFAULT_FORMAT.name,
|
||||
)
|
||||
|
||||
if (!publisher.hasSpaceFor(totalBytes)) {
|
||||
if (!hasRoomFor(declaredTotal, uris)) {
|
||||
return Result.failure(workDataOf(KEY_ERROR to "Not enough free space to join these files."))
|
||||
}
|
||||
|
||||
@@ -104,6 +110,23 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether staging can take this join, measuring the inputs when nothing else has.
|
||||
*
|
||||
* The same shape as `ConversionWorker.hasRoomFor` and for the same reasons, with one
|
||||
* difference worth naming: a join's total is [InputQuery.total], which is null the moment a
|
||||
* *single* input cannot be sized. Summing the ones that answered would produce a lower bound
|
||||
* indistinguishable from a real total, which is the conflation this change exists to end.
|
||||
*/
|
||||
private fun hasRoomFor(declared: Long?, uris: List<Uri>): Boolean {
|
||||
val bytes = declared ?: InputQuery.total(uris.map { InputQuery.sizeOf(applicationContext, it) })
|
||||
if (bytes == null) {
|
||||
Log.i(TAG, "Nothing could size every input; checking headroom only.")
|
||||
return publisher.hasSpaceForUnknownSize()
|
||||
}
|
||||
return publisher.hasSpaceFor(bytes)
|
||||
}
|
||||
|
||||
override suspend fun getForegroundInfo(): ForegroundInfo = ForegroundInfo(
|
||||
NOTIFICATION_ID,
|
||||
notifications.build(id, "Joining files", 0, indeterminate = true),
|
||||
@@ -150,13 +173,15 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
* join screen says about a job in flight, and after a restart nothing else can supply
|
||||
* it. See [JobTags].
|
||||
*/
|
||||
fun request(inputs: List<Uri>, totalBytes: Long, format: OutputFormat = DEFAULT_FORMAT) =
|
||||
fun request(inputs: List<Uri>, totalBytes: Long?, format: OutputFormat = DEFAULT_FORMAT) =
|
||||
OneTimeWorkRequestBuilder<ConcatWorker>()
|
||||
.addTag(JobTags.inputCount(inputs.size))
|
||||
.setInputData(
|
||||
Data.Builder()
|
||||
.putStringArray(KEY_INPUT_URIS, inputs.map(Uri::toString).toTypedArray())
|
||||
.putLong(KEY_TOTAL_BYTES, totalBytes)
|
||||
// Omitted rather than zeroed when a total could not be worked out; a
|
||||
// `Data` has no null, so the missing key is the unknown.
|
||||
.apply { totalBytes?.let { putLong(KEY_TOTAL_BYTES, it) } }
|
||||
.putString(KEY_FORMAT, format.name)
|
||||
.build(),
|
||||
)
|
||||
|
||||
@@ -9,10 +9,13 @@ import androidx.work.Data
|
||||
import androidx.work.ForegroundInfo
|
||||
import androidx.work.OneTimeWorkRequestBuilder
|
||||
import androidx.work.WorkerParameters
|
||||
import androidx.work.hasKeyWithValueOfType
|
||||
import androidx.work.workDataOf
|
||||
import com.arthenica.ffmpegkit.FFmpegKitConfig
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.InputQuery
|
||||
import org.libremediaconverter.convert.OutputPublisher
|
||||
import org.libremediaconverter.convert.StagingNames
|
||||
import org.libremediaconverter.model.AudioCodec
|
||||
import org.libremediaconverter.model.Container
|
||||
@@ -57,7 +60,11 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
||||
val inputUri = inputData.getString(KEY_INPUT_URI)?.let(Uri::parse)
|
||||
?: return Result.failure(workDataOf(KEY_ERROR to "No input file."))
|
||||
val displayName = inputData.getString(KEY_DISPLAY_NAME) ?: "input"
|
||||
val sizeBytes = inputData.getLong(KEY_SIZE_BYTES, 0L)
|
||||
// Absent, not zero, when nobody could say -- see InputQuery. `getLong(key, 0L)` is what
|
||||
// made those two the same number, and `hasSpaceFor(0)` is only "is there 128 MB free".
|
||||
val declaredSize = inputData
|
||||
.takeIf { it.hasKeyWithValueOfType<Long>(KEY_SIZE_BYTES) }
|
||||
?.getLong(KEY_SIZE_BYTES, 0L)
|
||||
val spec = readSpec()
|
||||
val quality = QualityTier.valueOf(
|
||||
inputData.getString(KEY_QUALITY) ?: QualityTier.FAST.name,
|
||||
@@ -66,7 +73,7 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
||||
inputData.getString(KEY_ENGINE_PREFERENCE) ?: EnginePreference.AUTO.name,
|
||||
)
|
||||
|
||||
if (!publisher.hasSpaceFor(sizeBytes)) {
|
||||
if (!hasRoomFor(declaredSize, inputUri)) {
|
||||
return Result.failure(workDataOf(KEY_ERROR to "Not enough free space to convert."))
|
||||
}
|
||||
|
||||
@@ -151,6 +158,29 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether staging can take this job, asking the input itself when nothing else has.
|
||||
*
|
||||
* [declared] is what the picker found, carried in this job's `Data`. It is absent for work
|
||||
* enqueued before the size became optional, for a [request] built by hand, and for a file
|
||||
* whose provider would not answer — so the fallback opens the input and asks the descriptor,
|
||||
* which is one syscall on a file the conversion is about to open anyway. It runs only when
|
||||
* [declared] is null, so an ordinary job pays nothing for it.
|
||||
*
|
||||
* When even that cannot answer, the *question* changes rather than a number being invented:
|
||||
* [OutputPublisher.hasSpaceForUnknownSize] is the documented "all that is left to check is
|
||||
* the headroom", and it is not a refusal. Failing every job whose provider is quiet would be
|
||||
* a worse defect than the vacuous check it replaces.
|
||||
*/
|
||||
private fun hasRoomFor(declared: Long?, inputUri: Uri): Boolean {
|
||||
val bytes = declared ?: InputQuery.sizeOf(applicationContext, inputUri)
|
||||
if (bytes == null) {
|
||||
Log.i(TAG, "Nothing could size $inputUri; checking headroom only.")
|
||||
return publisher.hasSpaceForUnknownSize()
|
||||
}
|
||||
return publisher.hasSpaceFor(bytes)
|
||||
}
|
||||
|
||||
/**
|
||||
* The dynamic half of the routing rules.
|
||||
*
|
||||
@@ -318,18 +348,22 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
||||
fun request(
|
||||
inputUri: Uri,
|
||||
displayName: String,
|
||||
sizeBytes: Long,
|
||||
sizeBytes: Long?,
|
||||
spec: OutputSpec = OutputFormat.MP4_H265.spec,
|
||||
quality: QualityTier = QualityTier.FAST,
|
||||
enginePreference: EnginePreference = EnginePreference.AUTO,
|
||||
) = OneTimeWorkRequestBuilder<ConversionWorker>()
|
||||
.addTag(JobTags.displayName(displayName))
|
||||
.addTag(JobTags.sizeBytes(sizeBytes))
|
||||
// Neither the tag nor the Data entry is written for a size nobody knows. A `Data` has
|
||||
// no null, so the absence of the key *is* the unknown — and a tag reading
|
||||
// `size-bytes:0` would come back through Reattachment as a confident claim that the
|
||||
// user's file is empty.
|
||||
.apply { sizeBytes?.let { addTag(JobTags.sizeBytes(it)) } }
|
||||
.setInputData(
|
||||
Data.Builder()
|
||||
.putString(KEY_INPUT_URI, inputUri.toString())
|
||||
.putString(KEY_DISPLAY_NAME, displayName)
|
||||
.putLong(KEY_SIZE_BYTES, sizeBytes)
|
||||
.apply { sizeBytes?.let { putLong(KEY_SIZE_BYTES, it) } }
|
||||
.putString(KEY_CONTAINER, spec.container.name)
|
||||
.putString(KEY_VIDEO_CODEC, spec.videoCodec.name)
|
||||
.putString(KEY_AUDIO_CODEC, spec.audioCodec.name)
|
||||
|
||||
+4
-1
@@ -100,7 +100,10 @@ class ConversionViewModelProbeFailureTest {
|
||||
// on that thread's handler rather than at this call. What must not happen is the card
|
||||
// filling in with an "unreadable" verdict the app would then act on.
|
||||
val settled = settle(viewModel)
|
||||
assertEquals(ConversionState.Ready(InputFile(INPUT, "input", 0L)), settled)
|
||||
// `sizeBytes = null`, not `0L`: no provider is registered for this authority, so the
|
||||
// metadata query returns nothing and the descriptor cannot be opened either. That is the
|
||||
// unknown, and it stopped being spelled the same way as "empty" -- see [InputQuery].
|
||||
assertEquals(ConversionState.Ready(InputFile(INPUT, "input", sizeBytes = null)), settled)
|
||||
assertNull("an OOM must not be reported as a probe result", (settled as ConversionState.Ready).input.probe)
|
||||
}
|
||||
|
||||
|
||||
@@ -59,28 +59,46 @@ open class RecordingPublisher(context: Context) : OutputPublisher(context) {
|
||||
* `SUCCEEDED` `WorkInfo` carrying an output path, and that is exactly what this produces —
|
||||
* through a real `WorkManager`, so the ViewModel's own observer, its `SUCCEEDED` branch and
|
||||
* its cleanup handle are all the production ones.
|
||||
*
|
||||
* It also keeps every [Data] it was handed, which is the only way back to what a ViewModel
|
||||
* actually enqueued: `WorkInfo` returns a job's tags and its output and never the input `Data`
|
||||
* it was built with, so a test that wants to know what `convert()` or `join()` put in a request
|
||||
* has to catch it here, on its way to the worker.
|
||||
*/
|
||||
class SucceedingWorkerFactory(private val outputData: Data) : WorkerFactory() {
|
||||
|
||||
/** The input `Data` of each request that has reached a worker, in order. */
|
||||
val enqueued = mutableListOf<Data>()
|
||||
|
||||
override fun createWorker(
|
||||
appContext: Context,
|
||||
workerClassName: String,
|
||||
workerParameters: WorkerParameters,
|
||||
): ListenableWorker = object : Worker(appContext, workerParameters) {
|
||||
override fun doWork(): Result = Result.success(outputData)
|
||||
): ListenableWorker {
|
||||
enqueued += workerParameters.inputData
|
||||
return object : Worker(appContext, workerParameters) {
|
||||
override fun doWork(): Result = Result.success(outputData)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Installs a synchronous test WorkManager whose workers succeed with [outputData]. */
|
||||
fun installTestWorkManager(context: Context, outputData: Data) {
|
||||
/**
|
||||
* Installs a synchronous test WorkManager whose workers succeed with [outputData].
|
||||
*
|
||||
* @return the factory, so a caller that cares can read back what was enqueued.
|
||||
*/
|
||||
fun installTestWorkManager(context: Context, outputData: Data): SucceedingWorkerFactory {
|
||||
val factory = SucceedingWorkerFactory(outputData)
|
||||
WorkManagerTestInitHelper.initializeTestWorkManager(
|
||||
context,
|
||||
Configuration.Builder()
|
||||
.setMinimumLoggingLevel(Log.ASSERT)
|
||||
.setExecutor(SynchronousExecutor())
|
||||
.setTaskExecutor(SynchronousExecutor())
|
||||
.setWorkerFactory(SucceedingWorkerFactory(outputData))
|
||||
.setWorkerFactory(factory)
|
||||
.build(),
|
||||
)
|
||||
return factory
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,165 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.hasKeyWithValueOfType
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.join.JoinState
|
||||
import org.libremediaconverter.join.JoinViewModel
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* That a size nobody reported is not the same thing as a size of zero.
|
||||
*
|
||||
* `queryFile` started at `var size = 0L` and only moved off it when a provider answered the
|
||||
* `OpenableColumns.SIZE` column, so "the file is empty" and "nobody told me" arrived at the space
|
||||
* check as the same number — and `hasSpaceFor(0)` is only "is there 128 MB free".
|
||||
*
|
||||
* The gap is not hypothetical. On a Pixel 10 Pro XL, `contentResolver.query` on a `file://` URI
|
||||
* returns null outright, so the cursor block never runs and the default survives:
|
||||
* `queryFile gave displayName='input' sizeBytes=0`. Robolectric reproduces that exactly, which is
|
||||
* what makes the first test below a JVM test rather than a device one.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class UnknownInputSizeTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var workers: SucceedingWorkerFactory
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
// FFprobe's loader throws a bare java.lang.Error on the JVM, and nothing here is about
|
||||
// what the probe found.
|
||||
ConversionDependencies.probe = { _, _ -> InputProbe() }
|
||||
// Both ViewModels reach WorkManager.getInstance() while constructing.
|
||||
workers = installTestWorkManager(app, Data.EMPTY)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a picked file no provider describes is measured rather than reported as empty`() {
|
||||
val input = fileOfSize(INPUT_BYTES, "holiday.mp4")
|
||||
|
||||
val ready = pickedInto(Uri.fromFile(input))
|
||||
|
||||
// The resolver answers nothing at all for a file:// URI -- the exact device case -- so the
|
||||
// only way to this number is opening the file and asking the descriptor.
|
||||
assertEquals(INPUT_BYTES.toLong(), ready.input.sizeBytes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a picked file nothing can measure has no size rather than a size of zero`() {
|
||||
// No provider is registered for this authority, so the metadata query returns null and
|
||||
// openFileDescriptor throws FileNotFoundException. Nothing can say how big it is, and
|
||||
// saying "zero" would be a claim rather than an answer.
|
||||
val ready = pickedInto(Uri.parse("content://test/holiday.mp4"))
|
||||
|
||||
assertNull(ready.input.sizeBytes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the same measurement is what the join picker gets`() {
|
||||
// Not a copy of the convert test for its own sake: `queryFile` existed twice, once in each
|
||||
// ViewModel, byte for byte. One of the two being fixed is the shape this would come back in.
|
||||
val first = fileOfSize(FIRST_JOIN_BYTES, "one.mp4")
|
||||
val second = fileOfSize(SECOND_JOIN_BYTES, "two.mp4")
|
||||
val viewModel = JoinViewModel(app)
|
||||
|
||||
viewModel.onInputsPicked(listOf(Uri.fromFile(first), Uri.fromFile(second)))
|
||||
val ready = awaitState(viewModel.state, "Ready") { it is JoinState.Ready } as JoinState.Ready
|
||||
|
||||
assertEquals(
|
||||
listOf(FIRST_JOIN_BYTES.toLong(), SECOND_JOIN_BYTES.toLong()),
|
||||
ready.inputs.map { it.sizeBytes },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join enqueues the total it worked out, and no total at all when it could not`() {
|
||||
// The wiring, which the two tests around it do not reach: `InputQuery.total` being right
|
||||
// says nothing about `join()` calling it, and `ConcatWorker`'s unknown branch is reached
|
||||
// by work built in that test rather than by this ViewModel. Restoring
|
||||
// `inputs.sumOf { it.sizeBytes ?: 0L }` leaves both of those green.
|
||||
//
|
||||
// Read off the request on its way to a worker, because that is the only place it is
|
||||
// legible: `WorkInfo` hands back a job's tags and its output, never the input `Data`.
|
||||
val first = fileOfSize(FIRST_JOIN_BYTES, "one.mp4")
|
||||
val second = fileOfSize(SECOND_JOIN_BYTES, "two.mp4")
|
||||
|
||||
joined(Uri.fromFile(first), Uri.fromFile(second))
|
||||
val known = workers.enqueued.single()
|
||||
assertEquals(
|
||||
(FIRST_JOIN_BYTES + SECOND_JOIN_BYTES).toLong(),
|
||||
known.getLong(ConcatWorker.KEY_TOTAL_BYTES, MISSING),
|
||||
)
|
||||
|
||||
// And with one input nothing can size, the key is absent rather than carrying a short
|
||||
// total -- a `Data` has no null, so absence is the only way to say "unknown" in one.
|
||||
workers = installTestWorkManager(app, Data.EMPTY)
|
||||
joined(Uri.fromFile(first), Uri.parse("content://test/two.mp4"))
|
||||
val unknown = workers.enqueued.single()
|
||||
assertFalse(
|
||||
"a total that could not be worked out must not be enqueued as a number",
|
||||
unknown.hasKeyWithValueOfType<Long>(ConcatWorker.KEY_TOTAL_BYTES),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a total is only as good as its least-known part`() {
|
||||
// The rule the join side needed that the convert side did not. `sumOf` over a list with an
|
||||
// unknown in it produces a number, and a number that is short by one whole file is worse
|
||||
// than no number: the space check cannot tell it from a real total, so it would reserve
|
||||
// for half the job and pass.
|
||||
assertEquals(3_333L, InputQuery.total(listOf(1_111L, 2_222L)))
|
||||
assertNull(InputQuery.total(listOf(1_111L, null)))
|
||||
assertNull(InputQuery.total(listOf(null, 2_222L)))
|
||||
// A join of nothing has a known total of nothing. Both workers refuse fewer than two
|
||||
// inputs long before this, so it is a statement about the fold rather than a real case.
|
||||
assertEquals(0L, InputQuery.total(emptyList()))
|
||||
}
|
||||
|
||||
/** Drives a real [JoinViewModel] from a pick to an enqueued join. */
|
||||
private fun joined(vararg uris: Uri) {
|
||||
val viewModel = JoinViewModel(app)
|
||||
viewModel.onInputsPicked(uris.toList())
|
||||
awaitState(viewModel.state, "Ready") { it is JoinState.Ready }
|
||||
viewModel.join()
|
||||
awaitState(viewModel.state, "past Joining") { it !is JoinState.Ready }
|
||||
}
|
||||
|
||||
private fun pickedInto(uri: Uri): ConversionState.Ready {
|
||||
val viewModel = ConversionViewModel(app)
|
||||
viewModel.onInputPicked(uri)
|
||||
return awaitState(viewModel.state, "Ready") { it is ConversionState.Ready } as ConversionState.Ready
|
||||
}
|
||||
|
||||
private fun fileOfSize(bytes: Int, name: String): File =
|
||||
File(app.cacheDir, name).apply { writeBytes(ByteArray(bytes)) }
|
||||
|
||||
private companion object {
|
||||
const val INPUT_BYTES = 4_321
|
||||
const val FIRST_JOIN_BYTES = 1_111
|
||||
const val SECOND_JOIN_BYTES = 2_222
|
||||
|
||||
/** A `getLong` default no real total could be mistaken for. */
|
||||
const val MISSING = -1L
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,251 @@
|
||||
package org.libremediaconverter.work
|
||||
|
||||
import android.app.Application
|
||||
import android.content.Context
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
import androidx.work.Data
|
||||
import androidx.work.ListenableWorker
|
||||
import androidx.work.testing.TestListenableWorkerBuilder
|
||||
import androidx.work.workDataOf
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.OutputPublisher
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.libremediaconverter.model.DeviceCodecs
|
||||
import org.libremediaconverter.model.EnginePreference
|
||||
import org.libremediaconverter.model.InputProbe
|
||||
import org.libremediaconverter.model.OutputFormat
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import java.io.File
|
||||
import java.util.UUID
|
||||
|
||||
/**
|
||||
* What the space check is actually asked, which is where D5 lived.
|
||||
*
|
||||
* The worker read its input size out of `Data` with `getLong(KEY_SIZE_BYTES, 0L)`, so a job whose
|
||||
* size nobody could report asked "is there room for 0 bytes?" — which the headroom answers yes to
|
||||
* on any device with 128 MB free, whatever the file turns out to weigh.
|
||||
*
|
||||
* These assert on the *question*, not on the verdict. A test that only checked whether the job ran
|
||||
* would pass against the defect: the defect is that the guard is vacuous, not that it refuses.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class SpaceCheckTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var publisher: RecordingSpacePublisher
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
publisher = RecordingSpacePublisher(app)
|
||||
ConversionDependencies.publisher = { publisher }
|
||||
ConversionDependencies.probe = { _, _ -> InputProbe() }
|
||||
ConversionDependencies.deviceCodecs = { DeviceCodecs.PERMISSIVE }
|
||||
// The progress notification builds its cancel action from WorkManager.getInstance().
|
||||
installTestWorkManager(app, Data.EMPTY)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a job with no declared size measures its input rather than asking for room for nothing`() {
|
||||
val input = fileOfSize(INPUT_BYTES, "holiday.mp4")
|
||||
|
||||
runBlocking { conversionWorker(Uri.fromFile(input), declaredSize = null).doWork() }
|
||||
|
||||
assertEquals(listOf(INPUT_BYTES.toLong()), publisher.requested)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a declared size is trusted rather than re-measured`() {
|
||||
// The ordinary path, pinned so the measurement stays a fallback. Opening a descriptor per
|
||||
// job is cheap, but doing it when the picker already answered would be work for nothing --
|
||||
// and the declared number is the one the user was shown.
|
||||
val input = fileOfSize(INPUT_BYTES, "holiday.mp4")
|
||||
|
||||
runBlocking { conversionWorker(Uri.fromFile(input), declaredSize = DECLARED_BYTES).doWork() }
|
||||
|
||||
assertEquals(listOf(DECLARED_BYTES), publisher.requested)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join with no declared total measures its inputs rather than asking for room for nothing`() {
|
||||
val first = fileOfSize(FIRST_JOIN_BYTES, "one.mp4")
|
||||
val second = fileOfSize(SECOND_JOIN_BYTES, "two.mp4")
|
||||
|
||||
runBlocking { concatWorker(listOf(Uri.fromFile(first), Uri.fromFile(second))).doWork() }
|
||||
|
||||
assertEquals(listOf(FIRST_JOIN_BYTES.toLong() + SECOND_JOIN_BYTES), publisher.requested)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a job whose input nothing can size asks the unknown-size question instead of claiming zero`() {
|
||||
// A file:// URI at a path that does not exist: the resolver answers no metadata, and
|
||||
// opening a descriptor throws. Nothing left can say how big it is.
|
||||
runBlocking { conversionWorker(MISSING_INPUT, declaredSize = null).doWork() }
|
||||
|
||||
// The point of the whole change, in one line. `[0L]` -- what this recorded before -- is a
|
||||
// claim that the file is empty; the unknown question is the absence of a claim.
|
||||
assertEquals(emptyList<Long>(), publisher.requested)
|
||||
assertEquals(1, publisher.unknownQuestions)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a join whose inputs are not all measurable has no total, rather than the ones that answered`() {
|
||||
val known = fileOfSize(FIRST_JOIN_BYTES, "one.mp4")
|
||||
|
||||
runBlocking { concatWorker(listOf(Uri.fromFile(known), MISSING_INPUT)).doWork() }
|
||||
|
||||
// Emphatically not `[1111]`. A lower bound is indistinguishable from a total once it
|
||||
// reaches the space check, and the check would then be reserving for half the job.
|
||||
assertEquals(emptyList<Long>(), publisher.requested)
|
||||
assertEquals(1, publisher.unknownQuestions)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a size nothing can determine is not by itself a reason to refuse the conversion`() {
|
||||
// The decision this defect had to make, pinned so it cannot be quietly reversed. Refusing
|
||||
// an unmeasurable input would turn "no provider answered the SIZE column" into "this file
|
||||
// cannot be converted", which is a worse defect than the vacuous guard it replaces -- and
|
||||
// one the user could do nothing at all about.
|
||||
ConversionDependencies.publisher = { AlwaysRoomPublisher(app) }
|
||||
ConversionDependencies.software = { WritingTranscoder }
|
||||
|
||||
val result = runBlocking {
|
||||
conversionWorker(MISSING_INPUT, declaredSize = null, engine = EnginePreference.FORCE_SOFTWARE).doWork()
|
||||
}
|
||||
|
||||
assertTrue("an unknown size must not end the job; got $result", result is ListenableWorker.Result.Success)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a full disk still refuses a job whose size is unknown`() {
|
||||
// The other half of that decision, and what keeps `FakeFailures.FullDisk` -- which
|
||||
// overrides `hasSpaceFor` and nothing else -- still meaning what it says. An independent
|
||||
// implementation of the unknown-size question could stop honouring a full disk without a
|
||||
// single caller changing.
|
||||
ConversionDependencies.publisher = { NoRoomPublisher(app) }
|
||||
|
||||
val result = runBlocking { conversionWorker(MISSING_INPUT, declaredSize = null).doWork() }
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(
|
||||
workDataOf(ConversionWorker.KEY_ERROR to "Not enough free space to convert."),
|
||||
),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A worker whose input `Data` carries a size only when [declaredSize] is given.
|
||||
*
|
||||
* Built entry by entry rather than through `ConversionWorker.request`, because "the key is
|
||||
* simply not there" is the shape being tested and `request` is one of the two things that
|
||||
* produces it.
|
||||
*/
|
||||
private fun conversionWorker(
|
||||
input: Uri,
|
||||
declaredSize: Long?,
|
||||
engine: EnginePreference = EnginePreference.AUTO,
|
||||
): ConversionWorker {
|
||||
val data = mutableMapOf<String, Any>(
|
||||
ConversionWorker.KEY_INPUT_URI to input.toString(),
|
||||
ConversionWorker.KEY_DISPLAY_NAME to "holiday.mp4",
|
||||
ConversionWorker.KEY_CONTAINER to SPEC.container.name,
|
||||
ConversionWorker.KEY_VIDEO_CODEC to SPEC.videoCodec.name,
|
||||
ConversionWorker.KEY_AUDIO_CODEC to SPEC.audioCodec.name,
|
||||
ConversionWorker.KEY_ENGINE_PREFERENCE to engine.name,
|
||||
)
|
||||
declaredSize?.let { data[ConversionWorker.KEY_SIZE_BYTES] = it }
|
||||
return TestListenableWorkerBuilder<ConversionWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(*data.map { it.key to it.value }.toTypedArray()),
|
||||
runAttemptCount = 0,
|
||||
).setId(CONVERSION_ID).build()
|
||||
}
|
||||
|
||||
private fun concatWorker(inputs: List<Uri>): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(
|
||||
ConcatWorker.KEY_INPUT_URIS to inputs.map(Uri::toString).toTypedArray(),
|
||||
ConcatWorker.KEY_FORMAT to OutputFormat.MP4_H264.name,
|
||||
),
|
||||
runAttemptCount = 0,
|
||||
).setId(CONCAT_ID).build()
|
||||
|
||||
private fun fileOfSize(bytes: Int, name: String): File =
|
||||
File(app.cacheDir, name).apply { writeBytes(ByteArray(bytes)) }
|
||||
|
||||
private companion object {
|
||||
const val INPUT_BYTES = 4_321
|
||||
const val DECLARED_BYTES = 9_999L
|
||||
const val FIRST_JOIN_BYTES = 1_111
|
||||
const val SECOND_JOIN_BYTES = 2_222
|
||||
|
||||
/**
|
||||
* An input nothing can size.
|
||||
*
|
||||
* A `file://` path that does not exist, which under Robolectric behaves exactly as the
|
||||
* device pass recorded for a real one: `contentResolver.query` returns null, so no SIZE
|
||||
* column is ever reached, and `openFileDescriptor` throws `FileNotFoundException`. It is
|
||||
* also a scheme the worker handles without the FFmpegKit SAF bridge, which is native and
|
||||
* therefore unavailable here.
|
||||
*/
|
||||
val MISSING_INPUT: Uri = Uri.parse("file:///nonexistent/holiday.mp4")
|
||||
val SPEC = OutputFormat.MP4_H265.spec
|
||||
val CONVERSION_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000011")
|
||||
val CONCAT_ID: UUID = UUID.fromString("00000000-0000-4000-8000-000000000012")
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Records *which* space question was asked, and refuses either way.
|
||||
*
|
||||
* Both overrides, and neither calls `super`. That is what makes the two questions tell apart at
|
||||
* all: the production default answers `hasSpaceForUnknownSize()` by delegating to
|
||||
* `hasSpaceFor(0L)`, so a recorder that delegated would log an unknown size as a request for zero
|
||||
* bytes — the exact conflation being tested. The delegation itself is pinned separately, by
|
||||
* [NoRoomPublisher] and the full-disk test.
|
||||
*
|
||||
* Refusing keeps the worker to the one line under test: the check runs before anything is staged
|
||||
* or any engine is reached, so `false` ends `doWork` immediately and no native library is asked to
|
||||
* load.
|
||||
*/
|
||||
private class RecordingSpacePublisher(context: Context) : OutputPublisher(context) {
|
||||
val requested = mutableListOf<Long>()
|
||||
var unknownQuestions = 0
|
||||
private set
|
||||
|
||||
override fun hasSpaceFor(bytes: Long): Boolean {
|
||||
requested += bytes
|
||||
return false
|
||||
}
|
||||
|
||||
override fun hasSpaceForUnknownSize(): Boolean {
|
||||
unknownQuestions++
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A full disk expressed the only way `FakeFailures.FullDisk` expresses it.
|
||||
*
|
||||
* `hasSpaceFor` and nothing else, so a job refused here is a job refused *through* the
|
||||
* delegation rather than by an override of its own.
|
||||
*/
|
||||
private class NoRoomPublisher(context: Context) : OutputPublisher(context) {
|
||||
override fun hasSpaceFor(bytes: Long): Boolean = false
|
||||
}
|
||||
Reference in New Issue
Block a user