From c3f2fe0ef7ea389182f525bd7309a4bd2fc31eec Mon Sep 17 00:00:00 2001 From: Zach Fleeman Date: Thu, 5 Nov 2026 18:28:41 -0700 Subject: [PATCH 1/2] Check config files for unknown keys and invalid choices --- README.md | 6 +++--- ffmpeg4discord/arguments.py | 20 ++++++++++++++++++++ tests/test_arguments.py | 30 +++++++++++++++++++++++++----- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 3aea1cd..66f298c 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ The package provides a convenient interface for FFmpeg, including a 2-pass encod ## Installation -This package requires Python >= 3.9. For Windows users, ensure that Python is added to your system's PATH. I highly recommend that you use the [python.org](https://python.org) Python installation. +This package requires Python >= 3.10. For Windows users, ensure that Python is added to your system's PATH. I highly recommend that you use the [python.org](https://python.org) Python installation. Install this package with the following command. @@ -75,7 +75,7 @@ I've had a good time using this command with a Batch file on Windows. Refer to t | `--astreams` | all | `--astreams "0,1"` | Comma-separated list of **0-based audio stream positions** to include. When used with `--amix`, those streams will be mixed; otherwise, only the first selected stream is kept. | | `-v`
`--verbose` | false | `--verbose` | Enable verbose FFmpeg logging (useful for debugging). | | `--config` | off | `--config config.json` | Path to a JSON file containing a saved configuration. When a setting is provided by both JSON and CLI flags, the CLI flag wins. See [JSON Configuration](#json-configuration). | -| `--no-config` | off | `no---config` | Disable loading any default configuration files. See [JSON Configuration](#json-configuration). | +| `--no-config` | off | `--no-config` | Disable loading any default configuration files. See [JSON Configuration](#json-configuration). | | `--web` | false | `--web` | Launch the Web UI for this job. See [Web UI](#web-ui). | | `-p`
`--port` | random (when `--web`) | `-p 5333` | Run the Web UI on a specific port. If omitted, a random free port between 5000–6000 is chosen. | @@ -115,7 +115,7 @@ Create a new plain text JSON file (`my-config.json`) structured like this: { "target_filesize": 10.0, "resolution": "1280x720", - "codec": "libvpx-vp9", + "codec": "vp9", "to": "00:00:40" } ``` diff --git a/ffmpeg4discord/arguments.py b/ffmpeg4discord/arguments.py index d14e2d8..12c1812 100644 --- a/ffmpeg4discord/arguments.py +++ b/ffmpeg4discord/arguments.py @@ -1,5 +1,6 @@ """Reads ff4d's command-line flags and config files.""" +import difflib import json import logging import socket @@ -185,6 +186,24 @@ def _search_for_default_config(args: dict) -> dict: return args +def _check_config(config: dict, parser: ArgumentParser, file_path: Path) -> None: + """Reject config keys that match no flag and values outside a flag's choices, like argparse does on the command line.""" + actions = {action.dest: action for action in parser._actions if action.dest != "help"} + for key, value in config.items(): + if key not in actions: + message = f"{file_path} has an unknown setting {key!r}." + match = difflib.get_close_matches(key, actions, n=1) + if match: + message += f" Did you mean {match[0]!r}?" + parser.error(message) + choices = actions[key].choices + if choices is not None and value not in choices: + flags = "/".join(actions[key].option_strings) + parser.error( + f"{file_path}: argument {flags}: invalid choice: {value!r} (choose from {', '.join(map(repr, choices))})" + ) + + def _merge_config_args(args: dict, parser: ArgumentParser, argv: list[str] | None = None) -> dict: """ Make the config file's values the parser's defaults, then parse the command line again. @@ -198,6 +217,7 @@ def _merge_config_args(args: dict, parser: ArgumentParser, argv: list[str] | Non if file_path not in configs: configs.add(file_path) config = load_config(file_path) + _check_config(config, parser, file_path) args["config"] = config.get("config") defaults = {**config, **defaults} # a config file beats the one it points to if defaults: diff --git a/tests/test_arguments.py b/tests/test_arguments.py index 4723c1e..3644ffc 100644 --- a/tests/test_arguments.py +++ b/tests/test_arguments.py @@ -170,7 +170,7 @@ def test_merge_config_args_applies_config_values(parser, write_config): config = { "output": "mydir", "audio_br": 128, - "codec": "vp9-speed", + "codec": "vp9", "filename_times": True, "from": "00:00:10", "framerate": 60, @@ -205,10 +205,30 @@ def test_merge_config_args_command_line_beats_config(parser, write_config, confi assert result[key] == expected -def test_merge_config_args_keeps_keys_that_are_not_flags(parser, write_config): - argv = ["file.mp4", "--config", str(write_config({"something_else": 1}))] - result = _merge_config_args(vars(parser.parse_args(argv)), parser, argv) - assert result["something_else"] == 1 +def test_merge_config_args_rejects_unknown_key(parser, write_config, capsys): + path = write_config({"target-filesize": 20}) + argv = ["file.mp4", "--config", str(path)] + with pytest.raises(SystemExit): + _merge_config_args(vars(parser.parse_args(argv)), parser, argv) + err = capsys.readouterr().err + assert f"{path.resolve()} has an unknown setting 'target-filesize'. Did you mean 'target_filesize'?" in err + + +def test_merge_config_args_rejects_invalid_codec(parser, write_config, capsys): + path = write_config({"codec": "libvpx-vp9"}) + argv = ["file.mp4", "--config", str(path)] + with pytest.raises(SystemExit): + _merge_config_args(vars(parser.parse_args(argv)), parser, argv) + err = capsys.readouterr().err + assert f"{path.resolve()}: argument -c/--codec: invalid choice: 'libvpx-vp9'" in err + + +def test_merge_config_args_names_the_nested_file_with_the_bad_value(parser, write_config, capsys): + inner = write_config({"codec": "bogus"}, name="inner.json") + argv = ["file.mp4", "--config", str(write_config({"config": str(inner)}))] + with pytest.raises(SystemExit): + _merge_config_args(vars(parser.parse_args(argv)), parser, argv) + assert f"{inner.resolve()}: argument -c/--codec" in capsys.readouterr().err def test_merge_config_args_follows_config_key(parser, write_config): From d128a9caa052977244a6b0ecc8aec5bee96eeae2 Mon Sep 17 00:00:00 2001 From: Zach Fleeman Date: Thu, 5 Nov 2026 18:43:02 -0700 Subject: [PATCH 2/2] Show a one-line error when ffprobe can't read the input file --- ffmpeg4discord/__main__.py | 9 ++++++++- tests/test_main.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/ffmpeg4discord/__main__.py b/ffmpeg4discord/__main__.py index cccaef6..f487905 100644 --- a/ffmpeg4discord/__main__.py +++ b/ffmpeg4discord/__main__.py @@ -14,6 +14,7 @@ from textwrap import dedent from typing import Callable, Optional +import ffmpeg from flask import Flask, redirect, render_template, request, send_file, url_for from ffmpeg4discord import arguments @@ -445,7 +446,13 @@ def main() -> None: args["output"] = Path(args["output"]) # the rest of the args are the encode settings settings = EncodeSettings(**args) - twopass = TwoPass(path) + try: + twopass = TwoPass(path) + except ffmpeg.Error as exc: + # ffprobe's last stderr line names the file and the problem, e.g. "clip.mp4: No such file or directory" + reason = exc.stderr.decode(errors="replace").strip().rsplit("\n", 1)[-1] + logging.error(reason or f"ffprobe couldn't read {path}") + raise SystemExit(1) from None if web: # --from and --to fill in the form's starting trim, so check them before starting the server diff --git a/tests/test_main.py b/tests/test_main.py index 55f8f24..8ae0e21 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -5,6 +5,7 @@ from types import SimpleNamespace from unittest.mock import ANY, MagicMock +import ffmpeg import pytest from flask import Flask @@ -151,6 +152,26 @@ def test_main_shows_bad_flags_without_a_traceback(monkeypatch, caplog): assert "Invalid --astreams value '0,x'" in caplog.text +@pytest.mark.parametrize( + ("stderr", "message"), + [ + ( + b"ffprobe version 8.0\n/clips/nope.mp4: No such file or directory\n", + "/clips/nope.mp4: No such file or directory", + ), + (b"", "ffprobe couldn't read"), + ], +) +def test_main_shows_unreadable_input_without_a_traceback(monkeypatch, caplog, stderr, message): + monkeypatch.setattr(mainmod, "check_for_update", lambda **_: VersionInfo("0.1.9", "0.1.9")) + monkeypatch.setattr(sys, "argv", ["ff4d", "nope.mp4", "--no-config"]) + monkeypatch.setattr(mainmod, "TwoPass", MagicMock(side_effect=ffmpeg.Error("ffprobe", b"", stderr))) + with pytest.raises(SystemExit) as exc_info: + mainmod.main() + assert exc_info.value.code == 1 + assert message in caplog.text + + def test_main_web_ignores_dry_run(run_main, monkeypatch, caplog): app = MagicMock() monkeypatch.setattr(mainmod, "Flask", MagicMock(return_value=app))