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

Copy-video-from-a-file-with-no-video suggests a spec that is itself invalid #114

New Issue
Closed
opened 2026-08-26 02:00:23 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-08-26 02:00:23 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Found while fixing #14, and deliberately left out of that PR: it is the same
family (a Validation.Invalid whose suggestion list is built by hand instead of
going through suggestions(), so nothing filters it by validate(...).isValid)
but a different reachable case.

Measured on the JVM, on main and still on the #14 branch:

validate(OutputSpec(MP4, COPY, COPY), InputProbe(audioCodec = "vorbis", hasVideo = false))
  -> Invalid(message=This file has no video track to copy.,
             suggestions=[OutputSpec(container=MP4, videoCodec=NONE, audioCodec=COPY)])

validate(OutputSpec(MP4, NONE, COPY), same probe)
  -> Invalid(message=MP4 cannot hold Vorbis audio.,
             suggestions=[OutputSpec(container=MP4, videoCodec=NONE, audioCodec=AAC)])

So the Advanced picker offers a one-tap fix that is itself refused. The user taps
it and gets a second error rather than a working combination — the dead end the
class KDoc says cannot happen ("every entry is itself valid, which
ContainerCapabilitiesTest asserts"). That assertion only ever covered one spec;
#14 widened it to five, and this case is outside all of them.

Where: ContainerCapabilities.validateVideo, the !probe.hasVideo branch:

return Validation.Invalid(
    "This file has no video track to copy.",
    listOf(spec.copy(videoCodec = VideoCodec.NONE)),
)

spec.copy(videoCodec = NONE) is valid whenever the audio axis happens to be
fine — which is why it went unnoticed — and invalid whenever it is not. It is
reachable with any audio the target container cannot carry: a Vorbis or PCM
source into MP4, an MP3 source into WebM.

Likely fix: route it through suggestions(spec.copy(videoCodec = NONE), probe, exclude = spec) like every other branch. Measured, that yields
MP4/NONE/AAC for the case above, which validates.

Why it was not folded into #14: that PR is about a spec that produces an
empty file and crashes the process. This one produces a bad suggestion, never
an invalid job — ConversionWorker re-validates, so tapping the dead-end
suggestion fails the job with a reason rather than doing anything unsafe. Keeping
the diffs separate keeps the crash fix reviewable.

Test to write with it: extend ContainerCapabilitiesTest.every suggestion is itself valid with a Vorbis audio-only probe. It is a table-driven test now, so
that is one more row.

Found while fixing #14, and deliberately left out of that PR: it is the same family (a `Validation.Invalid` whose suggestion list is built by hand instead of going through `suggestions()`, so nothing filters it by `validate(...).isValid`) but a different reachable case. **Measured** on the JVM, on `main` and still on the #14 branch: ``` validate(OutputSpec(MP4, COPY, COPY), InputProbe(audioCodec = "vorbis", hasVideo = false)) -> Invalid(message=This file has no video track to copy., suggestions=[OutputSpec(container=MP4, videoCodec=NONE, audioCodec=COPY)]) validate(OutputSpec(MP4, NONE, COPY), same probe) -> Invalid(message=MP4 cannot hold Vorbis audio., suggestions=[OutputSpec(container=MP4, videoCodec=NONE, audioCodec=AAC)]) ``` So the Advanced picker offers a one-tap fix that is itself refused. The user taps it and gets a second error rather than a working combination — the dead end the class KDoc says cannot happen ("every entry is itself valid, which `ContainerCapabilitiesTest` asserts"). That assertion only ever covered one spec; #14 widened it to five, and this case is outside all of them. **Where:** `ContainerCapabilities.validateVideo`, the `!probe.hasVideo` branch: ```kotlin return Validation.Invalid( "This file has no video track to copy.", listOf(spec.copy(videoCodec = VideoCodec.NONE)), ) ``` `spec.copy(videoCodec = NONE)` is valid whenever the audio axis happens to be fine — which is why it went unnoticed — and invalid whenever it is not. It is reachable with any audio the target container cannot carry: a Vorbis or PCM source into MP4, an MP3 source into WebM. **Likely fix:** route it through `suggestions(spec.copy(videoCodec = NONE), probe, exclude = spec)` like every other branch. Measured, that yields `MP4/NONE/AAC` for the case above, which validates. **Why it was not folded into #14:** that PR is about a spec that produces an *empty* file and crashes the process. This one produces a bad suggestion, never an invalid job — `ConversionWorker` re-validates, so tapping the dead-end suggestion fails the job with a reason rather than doing anything unsafe. Keeping the diffs separate keeps the crash fix reviewable. **Test to write with it:** extend `ContainerCapabilitiesTest.every suggestion is itself valid` with a Vorbis audio-only probe. It is a table-driven test now, so that is one more row.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels bug confirmed sev:low
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#114