Name a finished file after the job that made it

Six places decided what a converted or joined file should be called, and not one of them asked
the job. `JoinViewModel` reported `JoinState.Saved("joined.mp4")` whatever the format;
`JoinScreen` opened `CreateDocument("video/mp4")` and launched it with `"joined.mp4"`. On the
convert side `save()` and `suggestedOutputName()` built the name from `_settings.value.spec`, and
the screen took the MIME type from the same place -- the picker as it stands *now*, which is not
the spec the job ran with.

All six are right today, and all six are right by accident. The join screen has no format picker,
so the three MP4 literals agree with `ConcatWorker.request`'s default. The conversion pickers are
drawn only in the `Ready` state, so the settings cannot move between enqueue and save. Neither
accident is load-bearing anywhere it is written down.

One of them has already stopped holding, quietly. A job picked up by `reattach()` ran with a spec
that was never in this ViewModel's settings, because those settings belong to a process that no
longer exists -- so a reattached MP3 conversion is offered `.mp4` and `video/mp4` today. The
previous commit made that path more reachable rather than less: `Reattachment` exists precisely
to find work this ViewModel did not start.

The fix is to ask the only thing that knows. The spec travels to the worker as input `Data` and
`WorkInfo` hands input `Data` back to nobody, so the worker is the single point at which the
input's name and the spec that ran are both in scope. Both workers now report the two derived
strings -- the name to suggest and the type to open the dialog with -- in their output `Data`,
and they ride on `ConversionState.Converted` and `JoinState.Joined` from there. `save()` reports
what it saved rather than recomputing it, and both screens read the name and the MIME type off
the state they already collect, remembering their `CreateDocument` contract against that type
instead of a literal. The MIME type is not cosmetic: some providers rewrite a document's
extension to match it, so an MP3 offered as `video/webm` can arrive with the wrong one.

`suggestedOutputName()` is deleted rather than repaired. With the answer on the state there is no
caller left for it, and an accessor recomputing the same string would only be a second place for
it to be wrong -- which is what it was.

`ConcatWorker` gains `DEFAULT_FORMAT` and `outputNameFor(format)`. Three copies of "MP4" is the
shape this entry is about, and the input-Data default, `request`'s parameter default and the
ViewModel's fallback for a job that predates this change are exactly three copies.

Work already in the queue carries neither string, and WorkManager keeps finished work for about a
week, so that is the ordinary case for a few days rather than a corner. Those fall back to the
old derivation, which is a guess -- but it is the same guess the app was already making, it is
confined to jobs enqueued before this commit, and for such a job there is genuinely nothing
better to hand. New work never reaches it. The join's fallback is not even a guess: the format
`ConcatWorker.request` has always defaulted to is the format such a job really used.

`reattach()`'s KDoc carried this as a known wart it was deliberately leaving alone. That
paragraph is now a description of the fix rather than of a defect.

Tested at both ends, since either alone would pass while the other was wrong.
`WorkerOutputNamingTest` runs the real worker on an MP3 job -- nothing like the default preset, so
a name built from the picker is visibly wrong rather than accidentally right -- and compares the
whole success `Result`, which is what makes a missing key fail rather than go unnoticed. The two
ViewModel tests drive a finished job and then move the picker, which is what a reattached job
amounts to from the ViewModel's point of view.

Restoring just the two name sources -- `save()`'s `outputNameFor(displayName, _settings.value.spec)`
and `JoinState.Saved("joined.mp4")` -- turns them red with
`expected:<holiday_converted.mp3> but was:<input_converted.webm>` and
`expected:<joined.mkv> but was:<joined.mp4>`. The convert side loses the extension *and* the name:
`_settings.value` had been moved to WebM, and the display name a reattached job cannot supply had
already fallen back to the placeholder.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-22 20:15:08 -05:00
co-authored by Claude Opus 5
parent 2a68f03134
commit 159320dfa0
12 changed files with 520 additions and 60 deletions
@@ -81,6 +81,15 @@ sealed interface ConversionState {
val staged: File,
val engineUsed: String = "",
val routeReason: String = "",
/**
* What to call the file, and what type to open the save dialog with.
*
* Carried on the state rather than derived when the Save button is tapped, because the
* only thing that knows them is the job — see `ConversionWorker.KEY_SUGGESTED_NAME`. The
* staged file's own name says nothing: it is the job's id.
*/
val suggestedName: String = "",
val mimeType: String = "",
) : ConversionState
data class Saved(val displayName: String) : ConversionState
data class Failed(val message: String) : ConversionState
@@ -160,11 +169,10 @@ class ConversionViewModel @JvmOverloads constructor(
* every request with on its own, so it finds work enqueued by an earlier run of the app —
* and by an earlier *version* of it — which an id saved in a `SavedStateHandle` would not.
*
* One known wart, not fixed here because it is a different defect: the save dialog's
* suggested name and MIME type come from the current picker rather than from the job that
* ran, so a reattached job converting to something other than the default format is offered
* the default extension. That derivation is wrong on its own terms and is left to the change
* that fixes it properly.
* The save dialog's suggested name and MIME type used to be built from the current picker,
* which made a reattached job the worst case: its spec was never in these settings at all, so
* a job that converted to MP3 was offered `.mp4`. Both now travel in the job's own output
* `Data` — see [ConversionState.Converted].
*/
private fun reattach() {
viewModelScope.launch {
@@ -309,6 +317,24 @@ class ConversionViewModel @JvmOverloads constructor(
.getString(ConversionWorker.KEY_ENGINE_USED).orEmpty(),
routeReason = info.outputData
.getString(ConversionWorker.KEY_ROUTE_REASON).orEmpty(),
suggestedName = info.outputData
.getString(ConversionWorker.KEY_SUGGESTED_NAME)
?.takeIf { it.isNotBlank() }
// Work enqueued before the worker reported this carries
// nothing, and WorkManager keeps finished work for about a
// week -- so this branch is ordinary for a few days rather
// than a corner. It is the old derivation, kept because it is
// the same guess the app already made and there is genuinely
// nothing better available for such a job. New work never
// reaches it.
?: ConversionWorker.outputNameFor(
input.displayName,
_settings.value.spec,
),
mimeType = info.outputData
.getString(ConversionWorker.KEY_MIME_TYPE)
?.takeIf { it.isNotBlank() }
?: _settings.value.spec.mimeType,
)
}
}
@@ -345,12 +371,7 @@ class ConversionViewModel @JvmOverloads constructor(
}.onSuccess {
// publish() already deleted it; nothing left to clean up.
pendingStaged = null
_state.value = ConversionState.Saved(
ConversionWorker.outputNameFor(
converted.input.displayName,
_settings.value.spec,
),
)
_state.value = ConversionState.Saved(converted.suggestedName)
}.onFailure { e ->
// Deliberately NOT cleared. A failed save may mean the staged file is the
// only copy of an hour of transcoding, and the user's destination did not
@@ -384,11 +405,6 @@ class ConversionViewModel @JvmOverloads constructor(
_state.value = ConversionState.Idle
}
fun suggestedOutputName(): String = ConversionWorker.outputNameFor(
currentInput()?.displayName ?: "output",
_settings.value.spec,
)
private fun ConversionState.probe(): InputProbe? = when (this) {
is ConversionState.Ready -> input.probe
is ConversionState.Converting -> input.probe
@@ -28,6 +28,7 @@ import androidx.compose.material3.TextButton
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
@@ -66,8 +67,14 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
ActivityResultContracts.OpenDocument(),
) { uri -> uri?.let(viewModel::onInputPicked) }
// The contract's MIME type comes from the finished job rather than from the picker as it
// stands: some providers rewrite a document's extension to match it, so an MP3 offered as
// video/webm can arrive with the wrong one. Read straight off the collected state, so this
// recomposes because it depends on that rather than because an unrelated line happens to.
// Remembered against the type so the launcher re-registers only when it actually changes.
val destinationMime = (state as? ConversionState.Converted)?.mimeType ?: settings.spec.mimeType
val chooseDestination = rememberLauncherForActivityResult(
ActivityResultContracts.CreateDocument(settings.spec.mimeType),
remember(destinationMime) { ActivityResultContracts.CreateDocument(destinationMime) },
) { uri -> uri?.let(viewModel::save) }
// Requested at the point of use rather than on first launch, so the ask carries its
@@ -194,7 +201,7 @@ fun ConverterScreen(modifier: Modifier = Modifier, viewModel: ConversionViewMode
AssistChip(onClick = {}, label = { Text(s.routeReason) })
}
Button(
onClick = { chooseDestination.launch(viewModel.suggestedOutputName()) },
onClick = { chooseDestination.launch(s.suggestedName) },
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
) { Text("Save file") }
OutlinedButton(
@@ -18,6 +18,7 @@ import androidx.compose.material3.OutlinedButton
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.style.TextAlign
@@ -30,6 +31,7 @@ import org.libremediaconverter.model.ConcatStrategy
import org.libremediaconverter.ui.PrimaryButtonHeight
import org.libremediaconverter.ui.ScreenPaddingHorizontal
import org.libremediaconverter.ui.ScreenPaddingVertical
import org.libremediaconverter.work.ConcatWorker
@UnstableApi
@Composable
@@ -40,8 +42,13 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
ActivityResultContracts.OpenMultipleDocuments(),
) { uris -> if (uris.isNotEmpty()) viewModel.onInputsPicked(uris) }
// The contract's MIME type comes from the finished job rather than from a literal: some
// providers rewrite a document's extension to match it, so naming MP4 for a join that is not
// one can hand the user a file the extension lies about. Remembered against that type so the
// launcher re-registers only when it actually changes.
val destinationMime = (state as? JoinState.Joined)?.mimeType ?: ConcatWorker.DEFAULT_FORMAT.mimeType
val chooseDestination = rememberLauncherForActivityResult(
ActivityResultContracts.CreateDocument("video/mp4"),
remember(destinationMime) { ActivityResultContracts.CreateDocument(destinationMime) },
) { uri -> uri?.let(viewModel::save) }
Column(
@@ -138,7 +145,7 @@ fun JoinScreen(modifier: Modifier = Modifier, viewModel: JoinViewModel = viewMod
style = MaterialTheme.typography.bodySmall,
)
Button(
onClick = { chooseDestination.launch("joined.mp4") },
onClick = { chooseDestination.launch(s.suggestedName) },
modifier = Modifier.fillMaxWidth().height(PrimaryButtonHeight),
) { Text("Save file") }
OutlinedButton(
@@ -31,7 +31,17 @@ sealed interface JoinState {
data class Ready(val inputs: List<InputFile>) : JoinState
data class Joining(val inputs: List<InputFile>) : JoinState
data class Waiting(val inputs: List<InputFile>) : JoinState
data class Joined(val staged: File, val strategy: ConcatStrategy) : JoinState
data class Joined(
val staged: File,
val strategy: ConcatStrategy,
/**
* What to call the file, and what type to open the save dialog with.
*
* From the job, not from a literal. See `ConcatWorker.KEY_SUGGESTED_NAME`.
*/
val suggestedName: String,
val mimeType: String,
) : JoinState
data class Saved(val displayName: String) : JoinState
data class Failed(val message: String) : JoinState
}
@@ -168,7 +178,22 @@ class JoinViewModel @JvmOverloads constructor(
// Take responsibility for the file at the same moment the state
// starts referring to it, so the two cannot disagree.
pendingStaged = staged
JoinState.Joined(staged, strategy)
JoinState.Joined(
staged = staged,
strategy = strategy,
// A join enqueued before the worker reported these carries
// neither, and the fallback is the format such a job really
// used -- ConcatWorker.request has always defaulted to it, and
// the join screen has never offered a choice.
suggestedName = info.outputData
.getString(ConcatWorker.KEY_SUGGESTED_NAME)
?.takeIf { it.isNotBlank() }
?: ConcatWorker.outputNameFor(ConcatWorker.DEFAULT_FORMAT),
mimeType = info.outputData
.getString(ConcatWorker.KEY_MIME_TYPE)
?.takeIf { it.isNotBlank() }
?: ConcatWorker.DEFAULT_FORMAT.mimeType,
)
}
}
@@ -202,7 +227,7 @@ class JoinViewModel @JvmOverloads constructor(
}.onSuccess {
// publish() already deleted it; nothing left to clean up.
pendingStaged = null
_state.value = JoinState.Saved("joined.mp4")
_state.value = JoinState.Saved(joined.suggestedName)
}.onFailure { e ->
// Deliberately NOT cleared -- see the same branch in ConversionViewModel.
// A failed save can leave the staged file as the only copy of the work, so
@@ -41,7 +41,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
}
val totalBytes = inputData.getLong(KEY_TOTAL_BYTES, 0L)
val format = OutputFormat.valueOf(
inputData.getString(KEY_FORMAT) ?: OutputFormat.MP4_H264.name,
inputData.getString(KEY_FORMAT) ?: DEFAULT_FORMAT.name,
)
if (!publisher.hasSpaceFor(totalBytes)) {
@@ -73,6 +73,11 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
workDataOf(
KEY_OUTPUT_PATH to staged.absolutePath,
KEY_STRATEGY to result.strategy.name,
// See the same two in ConversionWorker. The join screen has no format picker
// today, so `joined.mp4` was right by accident everywhere it was written out;
// reporting them means the accident is not what holds it up.
KEY_SUGGESTED_NAME to outputNameFor(format),
KEY_MIME_TYPE to format.mimeType,
),
)
} catch (e: CancellationException) {
@@ -111,8 +116,31 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
const val KEY_FORMAT = "format"
const val KEY_OUTPUT_PATH = "output_path"
const val KEY_STRATEGY = "strategy"
/** The name to offer in the save dialog, and the type to open it with. */
const val KEY_SUGGESTED_NAME = "suggested_name"
const val KEY_MIME_TYPE = "mime_type"
const val KEY_ERROR = "error"
/**
* What a join produces when nothing says otherwise.
*
* Named once rather than repeated at the three places that need it -- the input-Data
* default, [request]'s parameter default, and the fallback a ViewModel uses for a job
* enqueued before this worker reported its format. Those three disagreeing is the shape
* this whole entry is about.
*/
val DEFAULT_FORMAT: OutputFormat = OutputFormat.MP4_H264
/**
* The name to suggest in the save dialog for a join of this [format].
*
* A function rather than the literal `joined.mp4` it replaces: that literal appeared in
* the ViewModel and twice in the screen, and all three were correct only because the join
* screen has no format picker yet.
*/
fun outputNameFor(format: OutputFormat): String = "joined.${format.extension}"
private const val NOTIFICATION_ID = 1002
private const val TAG = "ConcatWorker"
@@ -122,7 +150,7 @@ 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 = OutputFormat.MP4_H264) =
fun request(inputs: List<Uri>, totalBytes: Long, format: OutputFormat = DEFAULT_FORMAT) =
OneTimeWorkRequestBuilder<ConcatWorker>()
.addTag(JobTags.inputCount(inputs.size))
.setInputData(
@@ -124,6 +124,14 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
KEY_OUTPUT_PATH to staged.absolutePath,
KEY_ENGINE_USED to decision.engine.name,
KEY_ROUTE_REASON to decision.reason.explanation,
// Reported rather than left to be recomputed. The spec arrives here as input
// Data, and WorkInfo never hands input Data back -- so this is the only moment
// at which anything knows both the input's name and the spec that ran. A
// ViewModel deriving it later has only its own picker, which is not the same
// thing and is not the same thing in two different ways: a reattached job's
// spec was never in those settings, and a live picker can move mid-job.
KEY_SUGGESTED_NAME to outputNameFor(displayName, spec),
KEY_MIME_TYPE to spec.mimeType,
),
)
} catch (e: CancellationException) {
@@ -277,6 +285,10 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
const val KEY_OUTPUT_PATH = "output_path"
const val KEY_ENGINE_USED = "engine_used"
const val KEY_ROUTE_REASON = "route_reason"
/** The name to offer in the save dialog, and the type to open it with. */
const val KEY_SUGGESTED_NAME = "suggested_name"
const val KEY_MIME_TYPE = "mime_type"
const val KEY_ERROR = "error"
private const val NOTIFICATION_ID = 1001