Skip to content

--recode-audio=copy cannot be distinguished from the default, so an impossible copy is silently re-encoded instead of refused #645

Description

@crippledgeek

Found by research after #637 shipped (PR #644). #644's behaviour is correct for the default case and arguably wrong for an explicit request; the blocker is that rdlp cannot tell them apart.

What upstream actually does

Verified from source, not paraphrase:

  • FFmpegVideoConvertorPP._options calls stream_copy_opts(False)-c copy is never emitted. yt-dlp's recode verb has no copy mode at all; it always re-encodes when the extension differs.
  • FFmpegVideoRemuxerPP._options is stream_copy_opts() (copy=True) with no compatibility check; when ffmpeg rejects the copy, real_run_ffmpeg raises FFmpegPostProcessorError with ffmpeg's own last stderr line. The README is explicit: "If the target container does not support the video/audio codec, remuxing will fail."
  • ffmpeg never auto-falls back from -c copy to encoding.

So the two verbs are deliberately separated: copy-or-fail lives under remux, always-re-encode lives under recode, with no cross-contamination. Neither tool has a "copy, and quietly substitute an encoder if that doesn't work" mode — which is what rdlp does today.

Source: https://github.com/yt-dlp/yt-dlp/blob/master/yt_dlp/postprocessor/ffmpeg.py, https://github.com/yt-dlp/yt-dlp/blob/master/README.md

The problem

RecodeAudioMode::Copy is #[default] (crates/rdlp-types/src/recode_audio_mode.rs) and PostProcess::recode_audio is a non-optional RecodeAudioMode (postprocess.rs:174). By the time RecodeStage sees it, these are the same value:

  • the user typed --recode-audio=copy — an explicit demand
  • the user typed nothing — rdlp's own optimisation ("copy the audio if you can")

The information exists at the CLI boundary (Args::recode_audio is Option<String>) and is discarded during merge.

Consequence: for a container that provably cannot carry the codec (webm/mpg/ogg + aac), #644 re-encodes to the container's default and warns. Right for the default; wrong for the explicit request, where yt-dlp's contract says fail.

Hard-failing unconditionally is not the fix — it would break the default configuration, since --recode-video=webm with no audio flag would start erroring on any AAC source.

Proposed

Preserve the distinction, then branch on it:

  • PostProcess::recode_audio: Option<RecodeAudioMode>None = "not specified", matching the Option-as-inherit convention the desktop overlay already uses (see Desktop: should settings.json materialise defaults and be authoritative over config.toml? #610's verdict; do NOT materialise the default into settings.json).
  • Not specified + copy impossible → re-encode with the current warn! (today's behaviour, matches yt-dlp recode).
  • Explicitly copy + copy impossible → refuse, naming the container/codec pair and suggesting --recode-audio=auto or a different container.

Touches CLI merge, the desktop IPC contract, and config.toml parity (#540/#583 are the precedent for that kind of change).

Acceptance

Refs #637, #644

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions