Compare commits
10
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
104d02de03 | ||
|
|
90814222b7 | ||
|
|
2d4898ad44 | ||
|
|
83ac7eff2c | ||
|
|
b2c11bbfe0 | ||
|
|
3a5210ec5d | ||
|
|
5f3eda9c40 | ||
|
|
79cca0eb47 | ||
|
|
a84b24ba27 | ||
|
|
b2790e13d9 |
@@ -130,8 +130,9 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
|
||||
decision layer, where one branch is one documented user-visible outcome and the metric counts
|
||||
answers rather than complexity. Every other rule still applies there.
|
||||
- **Coverage is reported, not gated** — **84.9% of lines (1971/2321), 63.8% of branches**,
|
||||
measured 2026-08-26 with `./gradlew :app:jacocoTestReport`, against 454 JVM tests in 67 classes.
|
||||
- **Coverage is reported, not gated** — **87.1% of lines (2025/2324), 69.1% of branches
|
||||
(974/1410)**, measured 2026-08-27 with `./gradlew :app:jacocoTestReport`, against 502 JVM tests
|
||||
in 71 classes.
|
||||
|
||||
**Every figure this file carried before 2026-08-24 was an artifact, roughly half the real one.**
|
||||
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
|
||||
@@ -147,12 +148,19 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
disproportionately Robolectric, so each one added denominator and no numerator — the measurement
|
||||
was punishing exactly the tests that were hardest to write.
|
||||
|
||||
Two things still hold. A floor needs a baseline that has settled, and this one has not: it moved
|
||||
39 points in a single build change on 2026-08-24, then another 16 as the #52 test push and the
|
||||
Two things still hold. A floor needs a baseline that has settled, and this one has not. It moved
|
||||
39 points in a single build change on 2026-08-24; then another 16 as the #52 test push and the
|
||||
fixes it turned up landed — 69.2% -> 84.9% line, 53.2% -> 63.8% branch — while the denominator
|
||||
grew 2194 -> 2321, because that work added production code of its own. And **re-measure before
|
||||
quoting**: this entry was written quoting 81.4%, measured four hours earlier, and was already
|
||||
three points stale by the time it was ready to merge.
|
||||
grew 2194 -> 2321, because that work added production code of its own; then again on 2026-08-27
|
||||
as #132 and #133's ten children landed — 84.9% -> 87.1% line, 63.8% -> **69.1%** branch, 454 ->
|
||||
502 tests. **Branch moved four times as far as line that last time**, and that is the shape to
|
||||
expect from this kind of work rather than a surprise: those children targeted decision code —
|
||||
enum fallbacks, refusal arms, cursor shapes, a `when` over container rules — where one test
|
||||
chooses a branch the suite had never taken. Line coverage barely notices; branch coverage is the
|
||||
whole point.
|
||||
|
||||
And **re-measure before quoting**: this entry was once written quoting 81.4%, measured four hours
|
||||
earlier, and was already three points stale by the time it was ready to merge.
|
||||
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
|
||||
tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
|
||||
a change that is both needs both.
|
||||
|
||||
@@ -505,7 +505,7 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
WorkInfo.State.FAILED -> ConversionState.Failed(
|
||||
info.outputData.getString(ConversionWorker.KEY_ERROR)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: "Conversion failed.",
|
||||
?: ConversionWorker.GENERIC_FAILURE_MESSAGE,
|
||||
)
|
||||
|
||||
WorkInfo.State.CANCELLED -> cancelled
|
||||
@@ -565,7 +565,7 @@ class ConversionViewModel @JvmOverloads constructor(
|
||||
// than a fresh handle for the second failure's sake: a retry that fails again
|
||||
// lands back here still carrying the file, not on a bare Failed that would take
|
||||
// the offer away.
|
||||
_state.value = ConversionState.Failed(e.message ?: "Could not save the file.", pending)
|
||||
_state.value = ConversionState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -92,7 +92,12 @@ object MediaProbe {
|
||||
else -> InputKind.UNPARSEABLE
|
||||
}
|
||||
|
||||
private class Extracted(
|
||||
/**
|
||||
* `internal` rather than `private` so [extractedFrom] can be named from a test. The JVM test
|
||||
* source set is a friend of `main`, so this stays invisible outside the module — the precedent
|
||||
* is `MainActivity`'s `Destination`, and [containerFrom] beside it.
|
||||
*/
|
||||
internal class Extracted(
|
||||
val videoCodec: String?,
|
||||
val audioCodec: String?,
|
||||
val durationMs: Long,
|
||||
@@ -100,33 +105,55 @@ object MediaProbe {
|
||||
val height: Int,
|
||||
)
|
||||
|
||||
/**
|
||||
* What a set of track formats says about a file.
|
||||
*
|
||||
* Split out of [probeWithExtractor] so the rules below can be tested against tracks a test
|
||||
* *chooses*, rather than against whatever the committed fixtures happen to contain. The device
|
||||
* tests exercise this through real files; none of them can construct a two-video-track input,
|
||||
* a track that omits its duration, or an audio-before-video ordering on purpose.
|
||||
*
|
||||
* Three rules live here, and each is a decision rather than plumbing:
|
||||
*
|
||||
* - **First track of a type wins.** `video == null` is the whole guard. A file with two video
|
||||
* tracks must report the first, because that is the one an engine will transcode.
|
||||
* - **Duration is the maximum across tracks**, not the first one found or the last. A file
|
||||
* whose audio outlasts its video is ordinary, and reporting the video's length would cut the
|
||||
* progress bar short.
|
||||
* - **A track that omits `KEY_DURATION` contributes nothing** rather than zero. `MediaExtractor`
|
||||
* omits it for plenty of real tracks — see `MediaProbeTrackFieldsTest` — and `maxOf` against a
|
||||
* fabricated 0 would still be correct here, but reading a key that is absent is not.
|
||||
*/
|
||||
internal fun extractedFrom(formats: List<MediaFormat>): Extracted {
|
||||
var video: String? = null
|
||||
var audio: String? = null
|
||||
var durationUs = 0L
|
||||
var width = 0
|
||||
var height = 0
|
||||
|
||||
for (format in formats) {
|
||||
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
|
||||
if (format.containsKey(MediaFormat.KEY_DURATION)) {
|
||||
durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION))
|
||||
}
|
||||
when {
|
||||
mime.startsWith("video/") && video == null -> {
|
||||
video = shortName(mime)
|
||||
width = format.intOr(MediaFormat.KEY_WIDTH)
|
||||
height = format.intOr(MediaFormat.KEY_HEIGHT)
|
||||
}
|
||||
|
||||
mime.startsWith("audio/") && audio == null -> audio = shortName(mime)
|
||||
}
|
||||
}
|
||||
return Extracted(video, audio, durationUs / US_PER_MS, width, height)
|
||||
}
|
||||
|
||||
private fun probeWithExtractor(context: Context, uri: Uri): Extracted? {
|
||||
val extractor = MediaExtractor()
|
||||
return try {
|
||||
extractor.setDataSource(context, uri, null)
|
||||
var video: String? = null
|
||||
var audio: String? = null
|
||||
var durationUs = 0L
|
||||
var width = 0
|
||||
var height = 0
|
||||
|
||||
for (i in 0 until extractor.trackCount) {
|
||||
val format = extractor.getTrackFormat(i)
|
||||
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
|
||||
if (format.containsKey(MediaFormat.KEY_DURATION)) {
|
||||
durationUs = maxOf(durationUs, format.getLong(MediaFormat.KEY_DURATION))
|
||||
}
|
||||
when {
|
||||
mime.startsWith("video/") && video == null -> {
|
||||
video = shortName(mime)
|
||||
width = format.intOr(MediaFormat.KEY_WIDTH)
|
||||
height = format.intOr(MediaFormat.KEY_HEIGHT)
|
||||
}
|
||||
|
||||
mime.startsWith("audio/") && audio == null -> audio = shortName(mime)
|
||||
}
|
||||
}
|
||||
Extracted(video, audio, durationUs / US_PER_MS, width, height)
|
||||
extractedFrom(extractor.trackFormats())
|
||||
} catch (e: Exception) {
|
||||
Log.i(TAG, "Platform extractor could not read $uri.", e)
|
||||
null
|
||||
@@ -269,25 +296,7 @@ object MediaProbe {
|
||||
val extractor = MediaExtractor()
|
||||
return try {
|
||||
extractor.setDataSource(context, uri, null)
|
||||
var video: String? = null
|
||||
var audio: String? = null
|
||||
var width = 0
|
||||
var height = 0
|
||||
var fps = 0
|
||||
|
||||
for (i in 0 until extractor.trackCount) {
|
||||
val format = extractor.getTrackFormat(i)
|
||||
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
|
||||
if (mime.startsWith("video/") && video == null) {
|
||||
video = shortName(mime)
|
||||
width = format.intOr(MediaFormat.KEY_WIDTH)
|
||||
height = format.intOr(MediaFormat.KEY_HEIGHT)
|
||||
fps = format.intOr(MediaFormat.KEY_FRAME_RATE)
|
||||
} else if (mime.startsWith("audio/") && audio == null) {
|
||||
audio = shortName(mime)
|
||||
}
|
||||
}
|
||||
ConcatInput(video, audio, width, height, fps)
|
||||
concatInputFrom(extractor.trackFormats())
|
||||
} catch (e: Exception) {
|
||||
Log.i(TAG, "Could not probe $uri for concat; will re-encode.", e)
|
||||
ConcatInput(null, null, 0, 0, 0)
|
||||
@@ -296,6 +305,45 @@ object MediaProbe {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The join flow's read of the same track formats. See [extractedFrom] for why this is separate
|
||||
* from the extractor.
|
||||
*
|
||||
* Deliberately **not** folded into [extractedFrom] despite the overlap. This one reads frame
|
||||
* rate and does not read duration; that one reads duration and does not read frame rate. A
|
||||
* merged version would have to compute both for every caller, and `ConcatPlanner` treats an
|
||||
* unknown frame rate as "cannot prove a match" — so a field this flow does not need must not
|
||||
* start arriving as a number.
|
||||
*/
|
||||
internal fun concatInputFrom(formats: List<MediaFormat>): ConcatInput {
|
||||
var video: String? = null
|
||||
var audio: String? = null
|
||||
var width = 0
|
||||
var height = 0
|
||||
var fps = 0
|
||||
|
||||
for (format in formats) {
|
||||
val mime = format.getString(MediaFormat.KEY_MIME).orEmpty()
|
||||
if (mime.startsWith("video/") && video == null) {
|
||||
video = shortName(mime)
|
||||
width = format.intOr(MediaFormat.KEY_WIDTH)
|
||||
height = format.intOr(MediaFormat.KEY_HEIGHT)
|
||||
fps = format.intOr(MediaFormat.KEY_FRAME_RATE)
|
||||
} else if (mime.startsWith("audio/") && audio == null) {
|
||||
audio = shortName(mime)
|
||||
}
|
||||
}
|
||||
return ConcatInput(video, audio, width, height, fps)
|
||||
}
|
||||
|
||||
/**
|
||||
* Every track format this extractor holds, read once.
|
||||
*
|
||||
* The thin edge the two pure functions above leave behind: a `trackCount` and a
|
||||
* `getTrackFormat` per index, which is the whole of what needs a real `MediaExtractor`.
|
||||
*/
|
||||
private fun MediaExtractor.trackFormats(): List<MediaFormat> = (0 until trackCount).map(::getTrackFormat)
|
||||
|
||||
/**
|
||||
* One track property as an Int, or [fallback] when the format has no Int to give.
|
||||
*
|
||||
|
||||
@@ -24,6 +24,20 @@ const val STAGED_FILE_GONE_MESSAGE: String =
|
||||
"The finished file is no longer in the cache, so there is nothing left to save. " +
|
||||
"Start over to make it again."
|
||||
|
||||
/**
|
||||
* What to tell the user when the copy into their chosen destination did not finish.
|
||||
*
|
||||
* A fallback, not the usual message: `publish` throws with a real reason most of the time — the
|
||||
* volume filled, the provider revoked the grant — and that reason is better than this. This is for
|
||||
* the exception that arrives with nothing to say, which would otherwise reach the screen as an
|
||||
* empty failure.
|
||||
*
|
||||
* Kept next to [STAGED_FILE_GONE_MESSAGE] for exactly the reason that one names: **both ViewModels
|
||||
* need it**, and saving is what it is about. It was written out twice before — `ConversionViewModel`
|
||||
* and `JoinViewModel` each carried their own copy of the literal, agreeing by coincidence.
|
||||
*/
|
||||
const val SAVE_FAILED_MESSAGE: String = "Could not save the file."
|
||||
|
||||
/**
|
||||
* A staged file that is still there to be saved, and everything the save dialog needs to offer it.
|
||||
*
|
||||
|
||||
@@ -19,6 +19,7 @@ import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.InputFile
|
||||
import org.libremediaconverter.convert.InputQuery
|
||||
import org.libremediaconverter.convert.PendingSave
|
||||
import org.libremediaconverter.convert.SAVE_FAILED_MESSAGE
|
||||
import org.libremediaconverter.convert.STAGED_FILE_GONE_MESSAGE
|
||||
import org.libremediaconverter.convert.ScreenOwnership
|
||||
import org.libremediaconverter.model.ConcatStrategy
|
||||
@@ -194,7 +195,7 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
fun onInputsPicked(uris: List<Uri>) {
|
||||
val token = ownership.claim()
|
||||
if (uris.size < 2) {
|
||||
_state.value = JoinState.Failed("Pick at least two files to join.")
|
||||
_state.value = JoinState.Failed(ConcatWorker.TOO_FEW_INPUTS_MESSAGE)
|
||||
return
|
||||
}
|
||||
viewModelScope.launch {
|
||||
@@ -295,7 +296,7 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
WorkInfo.State.FAILED -> JoinState.Failed(
|
||||
info.outputData.getString(ConcatWorker.KEY_ERROR)
|
||||
?.takeIf { it.isNotBlank() }
|
||||
?: "Joining failed.",
|
||||
?: ConcatWorker.GENERIC_FAILURE_MESSAGE,
|
||||
)
|
||||
|
||||
WorkInfo.State.CANCELLED -> cancelled
|
||||
@@ -346,7 +347,7 @@ class JoinViewModel @JvmOverloads constructor(
|
||||
// than leaving "Start over" -- which deletes it -- as the only thing on offer.
|
||||
// Passing `pending` rather than rebuilding it is what keeps a retry that fails
|
||||
// again on a carrying Failed instead of a bare one.
|
||||
_state.value = JoinState.Failed(e.message ?: "Could not save the file.", pending)
|
||||
_state.value = JoinState.Failed(e.message ?: SAVE_FAILED_MESSAGE, pending)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -39,7 +39,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
val uris = inputData.getStringArray(KEY_INPUT_URIS)?.map(Uri::parse)
|
||||
?: return Result.failure(workDataOf(KEY_ERROR to "No input files."))
|
||||
if (uris.size < 2) {
|
||||
return Result.failure(workDataOf(KEY_ERROR to "Pick at least two files to join."))
|
||||
return Result.failure(workDataOf(KEY_ERROR to TOO_FEW_INPUTS_MESSAGE))
|
||||
}
|
||||
// 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.
|
||||
@@ -107,7 +107,7 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
}
|
||||
FailureOutcome.FAIL -> {
|
||||
Log.e(TAG, "Joining failed.", e)
|
||||
Result.failure(workDataOf(KEY_ERROR to (e.message ?: "Joining failed.")))
|
||||
Result.failure(workDataOf(KEY_ERROR to (e.message ?: GENERIC_FAILURE_MESSAGE)))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -137,6 +137,34 @@ class ConcatWorker(context: Context, params: WorkerParameters) : CoroutineWorker
|
||||
)
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* What the user is told when a join arrives with fewer than two inputs.
|
||||
*
|
||||
* Shared with `JoinViewModel`, which refuses the same condition one layer up so the picker
|
||||
* can answer without enqueueing anything. Two copies of this sentence existed before, and
|
||||
* only the one here was pinned by a test (#139) — so the wording could drift on the screen
|
||||
* without a single test noticing, for one message the user sees from one condition.
|
||||
*
|
||||
* Here rather than in the ViewModel because the rule is the worker's: `request(...)` takes
|
||||
* a `List<Uri>` and checks nothing about its length, so this is the guard that always runs.
|
||||
*/
|
||||
const val TOO_FEW_INPUTS_MESSAGE: String = "Pick at least two files to join."
|
||||
|
||||
/**
|
||||
* The last resort when a join fails and the exception says nothing.
|
||||
*
|
||||
* Shared with `JoinViewModel`, whose `FAILED` arm falls back to the same sentence when the
|
||||
* output `Data` carries no error at all — a worker killed before it could write one. The two
|
||||
* are a chain rather than a coincidence: this is what the worker puts *in* `KEY_ERROR`, and
|
||||
* that is what the ViewModel says when `KEY_ERROR` never arrived. The user cannot tell the
|
||||
* two apart and should not have to, so they are one sentence.
|
||||
*
|
||||
* The `Log.e` above deliberately keeps its own literal. A log line has a different audience
|
||||
* and carries the exception with it; coupling it to the user-facing wording would mean
|
||||
* rewording the screen to change a log.
|
||||
*/
|
||||
const val GENERIC_FAILURE_MESSAGE: String = "Joining failed."
|
||||
|
||||
const val KEY_INPUT_URIS = "input_uris"
|
||||
const val KEY_TOTAL_BYTES = "total_bytes"
|
||||
const val KEY_FORMAT = "format"
|
||||
|
||||
@@ -313,7 +313,7 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
||||
}
|
||||
FailureOutcome.FAIL -> {
|
||||
Log.e(TAG, "Conversion failed.", cause)
|
||||
Result.failure(workDataOf(KEY_ERROR to (cause.message ?: "Conversion failed.")))
|
||||
Result.failure(workDataOf(KEY_ERROR to (cause.message ?: GENERIC_FAILURE_MESSAGE)))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -352,6 +352,20 @@ class ConversionWorker(context: Context, params: WorkerParameters) : CoroutineWo
|
||||
)
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* The last resort when a conversion fails and the exception says nothing.
|
||||
*
|
||||
* Shared with `ConversionViewModel`, whose `FAILED` arm falls back to the same sentence when
|
||||
* the output `Data` carries no error — a worker killed before it could write one, which a
|
||||
* refused foreground start after a process restart produces. The two are a chain rather than
|
||||
* a coincidence: this is what goes *into* `KEY_ERROR`, and that is what is said when
|
||||
* `KEY_ERROR` never arrived. The user cannot tell those apart and should not have to.
|
||||
*
|
||||
* See [ConcatWorker.GENERIC_FAILURE_MESSAGE] for the join-side twin, and the note there
|
||||
* about why the neighbouring `Log.e` keeps its own literal.
|
||||
*/
|
||||
const val GENERIC_FAILURE_MESSAGE: String = "Conversion failed."
|
||||
|
||||
const val KEY_INPUT_URI = "input_uri"
|
||||
const val KEY_DISPLAY_NAME = "display_name"
|
||||
const val KEY_SIZE_BYTES = "size_bytes"
|
||||
|
||||
@@ -0,0 +1,221 @@
|
||||
package org.libremediaconverter.convert
|
||||
|
||||
import android.media.MediaFormat
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
/**
|
||||
* The rules `MediaProbe` applies to a set of track formats.
|
||||
*
|
||||
* ## Why this exists, and what it revises
|
||||
*
|
||||
* Issue #84 classified `probeWithExtractor` and `probeForConcat` as device-bound and explicitly not
|
||||
* a gap:
|
||||
*
|
||||
* > These are exercised by `RemuxTest`, `ConcatEngineTest` and `RealMediaBenchmark` in
|
||||
* > `androidTest` … **Do not read their 0% as untested.**
|
||||
*
|
||||
* That was right about the measurement boundary and right about FFprobe. It was not right that
|
||||
* these are only orchestration. The track walk is a **branch matrix**, and `androidTest` reaches it
|
||||
* only through whatever the committed fixtures happen to contain — so none of the rules below is
|
||||
* *chosen* by any test there. A fixture with two video tracks, a track that omits its duration, or
|
||||
* an audio-before-video ordering is not something a device test would produce on purpose.
|
||||
*
|
||||
* The seam is the answer #133 preferred over driving `ShadowMediaExtractor`: the walk is a pure
|
||||
* function over `List<MediaFormat>`, and what is left needing a device — `setDataSource`,
|
||||
* `getTrackFormat`, `release` — is the thin edge `androidTest` should be covering. This is the
|
||||
* `work/FailureOutcome.kt` pattern `CLAUDE.md` names.
|
||||
*
|
||||
* `MediaFormat` is a real one throughout, not a stub. `MediaProbeTrackFieldsTest` records why that
|
||||
* matters: it is a heterogeneous map whose getters throw rather than coerce, and a hand-rolled
|
||||
* double would not reproduce that.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class MediaProbeTrackWalkTest {
|
||||
|
||||
// --- extractedFrom: the conversion flow's read ---------------------------
|
||||
|
||||
@Test
|
||||
fun `the first video track wins when a file carries two`() {
|
||||
// `video == null` is the entire guard. A file with two video tracks must report the first,
|
||||
// because that is the one an engine will transcode -- and the width and height must come
|
||||
// from the same track, not be mixed across them.
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080),
|
||||
video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals("h264", extracted.videoCodec)
|
||||
assertEquals(1920, extracted.width)
|
||||
assertEquals(1080, extracted.height)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the first audio track wins when a file carries two`() {
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_OPUS),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals("aac", extracted.audioCodec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `duration is the longest track, not the first or the last`() {
|
||||
// A file whose audio outlasts its video is ordinary. Taking the video's length would cut
|
||||
// the progress bar short; taking the last track's would be right only by accident of order.
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, durationUs = 10_000_000),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 12_500_000),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_OPUS, durationUs = 1_000_000),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals(12_500L, extracted.durationMs)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a track that does not declare its duration contributes nothing to it`() {
|
||||
// MediaExtractor omits KEY_DURATION for plenty of real tracks -- MediaProbeTrackFieldsTest
|
||||
// records the same for KEY_FRAME_RATE. Reading a key that is absent is what containsKey
|
||||
// stands between us and.
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC, durationUs = 7_000_000),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals(7_000L, extracted.durationMs)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `declaring audio before video changes nothing`() {
|
||||
// Track order is a property of the container, not of the content. Both orderings have to
|
||||
// reach the same answer or the same file remuxed twice would probe differently.
|
||||
val videoFirst = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
|
||||
),
|
||||
)
|
||||
val audioFirst = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1280, height = 720),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals(videoFirst.videoCodec, audioFirst.videoCodec)
|
||||
assertEquals(videoFirst.audioCodec, audioFirst.audioCodec)
|
||||
assertEquals(videoFirst.width, audioFirst.width)
|
||||
assertEquals(videoFirst.height, audioFirst.height)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a track that is neither audio nor video is ignored`() {
|
||||
// Subtitle and timed-metadata tracks are common in MKV and MP4. Neither prefix matches, so
|
||||
// neither slot is filled -- and, importantly, a subtitle track must not be mistaken for the
|
||||
// absence of an audio track by some later `else`.
|
||||
val extracted = MediaProbe.extractedFrom(
|
||||
listOf(
|
||||
MediaFormat().apply { setString(MediaFormat.KEY_MIME, "text/vtt") },
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals("h264", extracted.videoCodec)
|
||||
assertNull(extracted.audioCodec)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file with no tracks reports nothing rather than zero-width video`() {
|
||||
val extracted = MediaProbe.extractedFrom(emptyList())
|
||||
|
||||
assertNull(extracted.videoCodec)
|
||||
assertNull(extracted.audioCodec)
|
||||
assertEquals(0L, extracted.durationMs)
|
||||
assertEquals(0, extracted.width)
|
||||
assertEquals(0, extracted.height)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an audio-only file reports no video codec at all`() {
|
||||
// The distinction MediaProbe.classify turns into InputKind.AUDIO_ONLY, and the reason
|
||||
// `hasVideo` exists: an audio file and a corrupt file must not look alike.
|
||||
val extracted = MediaProbe.extractedFrom(listOf(audio(MediaFormat.MIMETYPE_AUDIO_AAC)))
|
||||
|
||||
assertNull(extracted.videoCodec)
|
||||
assertEquals("aac", extracted.audioCodec)
|
||||
assertEquals(0, extracted.width)
|
||||
}
|
||||
|
||||
// --- concatInputFrom: the join flow's read -------------------------------
|
||||
|
||||
@Test
|
||||
fun `the join read takes frame rate from the first video track`() {
|
||||
val input = MediaProbe.concatInputFrom(
|
||||
listOf(
|
||||
video(MediaFormat.MIMETYPE_VIDEO_AVC, width = 1920, height = 1080, frameRate = 30),
|
||||
video(MediaFormat.MIMETYPE_VIDEO_HEVC, width = 640, height = 480, frameRate = 60),
|
||||
audio(MediaFormat.MIMETYPE_AUDIO_AAC),
|
||||
),
|
||||
)
|
||||
|
||||
assertEquals("h264", input.videoCodec)
|
||||
assertEquals("aac", input.audioCodec)
|
||||
assertEquals(1920, input.width)
|
||||
assertEquals(1080, input.height)
|
||||
assertEquals(30, input.frameRate)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a video track with no declared frame rate reports zero rather than guessing`() {
|
||||
// ConcatPlanner treats 0 as "cannot prove a match" and re-encodes. A guessed 30 would read
|
||||
// as agreement and produce a stream copy of clips that do not actually match -- the failure
|
||||
// its KDoc says the whole flow is arranged to avoid.
|
||||
val input = MediaProbe.concatInputFrom(listOf(video(MediaFormat.MIMETYPE_VIDEO_AVC)))
|
||||
|
||||
assertEquals(0, input.frameRate)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file with no tracks joins as entirely unknown`() {
|
||||
val input = MediaProbe.concatInputFrom(emptyList())
|
||||
|
||||
assertNull(input.videoCodec)
|
||||
assertNull(input.audioCodec)
|
||||
assertEquals(0, input.width)
|
||||
assertEquals(0, input.height)
|
||||
assertEquals(0, input.frameRate)
|
||||
}
|
||||
|
||||
private fun video(
|
||||
mime: String,
|
||||
width: Int = 1920,
|
||||
height: Int = 1080,
|
||||
durationUs: Long? = null,
|
||||
frameRate: Int? = null,
|
||||
): MediaFormat = MediaFormat.createVideoFormat(mime, width, height).apply {
|
||||
durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) }
|
||||
frameRate?.let { setInteger(MediaFormat.KEY_FRAME_RATE, it) }
|
||||
}
|
||||
|
||||
private fun audio(mime: String, durationUs: Long? = null): MediaFormat =
|
||||
MediaFormat.createAudioFormat(mime, SAMPLE_RATE, CHANNELS).apply {
|
||||
durationUs?.let { setLong(MediaFormat.KEY_DURATION, it) }
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val SAMPLE_RATE = 48_000
|
||||
const val CHANNELS = 2
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,102 @@
|
||||
package org.libremediaconverter.join
|
||||
|
||||
import android.app.Application
|
||||
import android.net.Uri
|
||||
import androidx.media3.common.util.UnstableApi
|
||||
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.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.libremediaconverter.convert.ConversionDependencies
|
||||
import org.libremediaconverter.convert.RecordingPublisher
|
||||
import org.libremediaconverter.convert.installTestWorkManager
|
||||
import org.libremediaconverter.work.ConcatWorker
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
|
||||
/**
|
||||
* The two layers that refuse a short join, refusing it with one sentence.
|
||||
*
|
||||
* ## Why this is not "assert a constant equals itself"
|
||||
*
|
||||
* `ConcatWorker` and `JoinViewModel` both reject a join of fewer than two files, and before #158
|
||||
* each carried **its own copy of the literal**. Only the worker's was pinned — by `RefusedJobTest`,
|
||||
* added in #139 — so the wording on the screen could drift away from the wording in the job with no
|
||||
* test saying anything, for one message the user sees from one condition.
|
||||
*
|
||||
* Sharing a constant makes them agree by construction. What it does *not* do is prove that both
|
||||
* layers still reach it: a refactor that stops `JoinViewModel` refusing at all, or that gives it a
|
||||
* different message, passes any test that only reads `TOO_FEW_INPUTS_MESSAGE`. So each layer is
|
||||
* driven for real here — the ViewModel through `onInputsPicked`, the worker through `doWork` — and
|
||||
* the assertion is that the two answers are **the same string**, taken from two running layers
|
||||
* rather than from one declaration.
|
||||
*
|
||||
* That is the shape `CLAUDE.md` asks for: revert the sharing and this goes red, because the two
|
||||
* sites drift the moment they are allowed to.
|
||||
*
|
||||
* ## Scope
|
||||
*
|
||||
* The arity guard's own behaviour on the ViewModel side — that it refuses one file, that it accepts
|
||||
* two, that it claims ownership first — is #155's, and this deliberately does not duplicate it.
|
||||
* This file is about the *agreement between layers*, which is what #158 changed.
|
||||
*/
|
||||
@UnstableApi
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class SharedFailureMessagesTest {
|
||||
|
||||
private lateinit var app: Application
|
||||
private lateinit var viewModel: JoinViewModel
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
app = RuntimeEnvironment.getApplication()
|
||||
ConversionDependencies.publisher = { RecordingPublisher(app) }
|
||||
installTestWorkManager(app, workDataOf(ConcatWorker.KEY_OUTPUT_PATH to "/dev/null"))
|
||||
viewModel = JoinViewModel(app)
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
ConversionDependencies.reset()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `both layers refuse a one-file join with the same sentence`() {
|
||||
// The ViewModel, refusing before anything is enqueued.
|
||||
viewModel.onInputsPicked(listOf(ONE_FILE))
|
||||
val fromScreen = (viewModel.state.value as JoinState.Failed).message
|
||||
|
||||
// The worker, refusing a job that reached the queue anyway -- which it can, because
|
||||
// ConcatWorker.request(...) takes a List<Uri> and checks nothing about its length.
|
||||
val result = runBlocking { worker(ONE_FILE).doWork() }
|
||||
val fromJob = (result as ListenableWorker.Result.Failure)
|
||||
.outputData.getString(ConcatWorker.KEY_ERROR)
|
||||
|
||||
assertEquals(
|
||||
"the screen and the job must say the same thing about the same refusal",
|
||||
fromScreen,
|
||||
fromJob,
|
||||
)
|
||||
// And that the shared sentence is the one either layer would have written on its own,
|
||||
// rather than both having drifted together to something else.
|
||||
assertEquals(ConcatWorker.TOO_FEW_INPUTS_MESSAGE, fromScreen)
|
||||
}
|
||||
|
||||
private fun worker(vararg inputs: Uri): ConcatWorker = TestListenableWorkerBuilder<ConcatWorker>(
|
||||
context = app,
|
||||
inputData = workDataOf(
|
||||
ConcatWorker.KEY_INPUT_URIS to inputs.map(Uri::toString).toTypedArray(),
|
||||
ConcatWorker.KEY_TOTAL_BYTES to 1024L,
|
||||
),
|
||||
runAttemptCount = 0,
|
||||
).build()
|
||||
|
||||
private companion object {
|
||||
val ONE_FILE: Uri = Uri.parse("content://test/holiday.mp4")
|
||||
}
|
||||
}
|
||||
@@ -146,7 +146,7 @@ class RefusedJobTest {
|
||||
|
||||
assertEquals(
|
||||
ListenableWorker.Result.failure(
|
||||
workDataOf(ConcatWorker.KEY_ERROR to "Pick at least two files to join."),
|
||||
workDataOf(ConcatWorker.KEY_ERROR to ConcatWorker.TOO_FEW_INPUTS_MESSAGE),
|
||||
),
|
||||
result,
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user