JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

Tell an unknown input size apart from an empty file

Browse Source 
`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:
JMR-dev
2026-08-22 20:54:46 -05:00
co-authored by Claude Opus 5
parent 49535998b5
commit b86df47c43
11 changed files with 684 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/java/org/libremediaconverter/convert/ConversionViewModel.kt
+14 -22
View File
@@ -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
app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt
+7 -1
View File
@@ -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) {
app/src/main/java/org/libremediaconverter/convert/InputQuery.kt
+102
View File
@@ -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"
}
app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt
+41 -3
View File
@@ -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.
*
app/src/main/java/org/libremediaconverter/join/JoinViewModel.kt
+9 -20
View File
@@ -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
app/src/main/java/org/libremediaconverter/work/ConcatWorker.kt
+29 -4
View File
@@ -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(),
)
app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt
+39 -5
View File
@@ -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)