From 98961badea22241285bee5224bc622b41e7edb98 Mon Sep 17 00:00:00 2001 From: Zach Fleeman Date: Thu, 22 Oct 2026 10:46:26 -0600 Subject: [PATCH 1/3] Reject --astreams numbers that don't match an audio track --- ffmpeg4discord/twopass.py | 20 +++++++++++++++++++- tests/test_main.py | 2 +- tests/test_twopass.py | 27 ++++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/ffmpeg4discord/twopass.py b/ffmpeg4discord/twopass.py index d5e9579..51a2784 100644 --- a/ffmpeg4discord/twopass.py +++ b/ffmpeg4discord/twopass.py @@ -573,6 +573,23 @@ def _check_video_size(self, settings: EncodeSettings) -> None: "Use an even crop width and height, or set an even Resolution (-r)." ) + def _check_audio_tracks(self, settings: EncodeSettings) -> None: + """Raise a ValueError for an --astreams number that doesn't match an audio track in the file.""" + + # --no-audio drops every track, so the numbers don't matter. + if settings.no_audio or not settings.astreams: + return + count = len(self.audio_streams) + for i in settings.astreams: + if i >= count: + if count == 0: + tracks = "This file has no audio tracks." + elif count == 1: + tracks = "This file has 1 audio track, numbered 0." + else: + tracks = f"This file has {count} audio tracks, numbered 0 to {count - 1}." + raise ValueError(f"Audio track {i} doesn't exist. {tracks}") + def _output_filename(self, settings: EncodeSettings) -> str: """ Returns the file to write. A folder gets a generated name, and a file name's extension is fixed to match the codec. @@ -649,7 +666,7 @@ def _apply_audio_filters(self, ffinput, settings: EncodeSettings): if settings.astreams is None: selected_positions = list(range(len(self.audio_streams))) else: - selected_positions = [i for i in settings.astreams if 0 <= i < len(self.audio_streams)] + selected_positions = settings.astreams # run() already checked that each one is a real track if not selected_positions: return None @@ -691,6 +708,7 @@ def run( start, end = self.clip_times(settings) self._check_video_size(settings) + self._check_audio_tracks(settings) output_filename = self._output_filename(settings) params = self._generate_params(settings, self._bitrate_targets(settings, end - start)) diff --git a/tests/test_main.py b/tests/test_main.py index 3acd756..796f5ad 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -355,7 +355,7 @@ def test_encode_amix_mode(web, amix_mode, amix, normalize): assert (settings.amix, settings.amix_normalize) == (amix, normalize) -@pytest.mark.parametrize("astreams, expected", [(["0", "2"], [0, 2]), ([], None)]) +@pytest.mark.parametrize("astreams, expected", [(["0"], [0]), ([], None)]) def test_encode_audio_stream_selection(web, astreams, expected): web.client.post("/encode", data=encode_form(astreams=astreams)) assert posted_settings(web).astreams == expected diff --git a/tests/test_twopass.py b/tests/test_twopass.py index 38833fc..8e32027 100644 --- a/tests/test_twopass.py +++ b/tests/test_twopass.py @@ -322,7 +322,7 @@ def test_crop_does_not_change_the_next_encodes_aspect_ratio(make_twopass, caplog assert tp.ratio == pytest.approx(1920 / 1080) -@pytest.mark.parametrize("astreams, expected", [(None, "a:0"), ([1, 0], "a:1"), ([5, 1], "a:1")]) +@pytest.mark.parametrize("astreams, expected", [(None, "a:0"), ([1, 0], "a:1")]) def test_apply_audio_filters_without_mixing_keeps_one_track(probe, make_twopass, astreams, expected): # two audio tracks, like ShadowPlay with a separate mic track; only one fits the bitrate budget probe["streams"].append({"codec_type": "audio", "bit_rate": "128000", "index": 2}) @@ -331,10 +331,10 @@ def test_apply_audio_filters_without_mixing_keeps_one_track(probe, make_twopass, def test_apply_audio_filters_mixes_selected_tracks(probe, make_twopass): - # three audio tracks; 5 is out of range and gets dropped + # three audio tracks, mixing the first and last for index in (2, 3): probe["streams"].append({"codec_type": "audio", "bit_rate": "128000", "index": index}) - settings = make_settings(amix=True, astreams=[0, 2, 5]) + settings = make_settings(amix=True, astreams=[0, 2]) audio = make_twopass()._apply_audio_filters(ffmpeg.input("test.mp4"), settings) assert audio.node.name == "amix" assert audio.node.kwargs == {"normalize": 0, "inputs": 2} @@ -384,6 +384,27 @@ def test_run_no_audio_flag_omits_audio(make_twopass, passes): assert "0:a:0" not in passes[-1].args +@pytest.mark.parametrize("astreams", [[3], [0, 3]]) +def test_run_rejects_audio_tracks_past_the_end(probe, make_twopass, passes, astreams): + for index in (2, 3): + probe["streams"].append({"codec_type": "audio", "bit_rate": "128000", "index": index}) + message = "Audio track 3 doesn't exist. This file has 3 audio tracks, numbered 0 to 2." + with pytest.raises(ValueError, match=message): + make_twopass().run(make_settings(amix=True, astreams=astreams)) + assert passes == [] + + +def test_run_rejects_audio_track_when_the_file_has_none(probe, make_twopass, passes): + probe["streams"].pop() + with pytest.raises(ValueError, match="Audio track 0 doesn't exist. This file has no audio tracks."): + make_twopass().run(make_settings(astreams=[0])) + + +def test_run_ignores_audio_track_numbers_with_no_audio(make_twopass, passes): + make_twopass().run(make_settings(no_audio=True, astreams=[7])) + assert "0:a:0" not in passes[-1].args + + def test_run_hardware_codec_skips_first_pass(make_twopass, passes): make_twopass().run(make_settings(codec="hevc_videotoolbox")) assert [p.name for p in passes] == ["single"] From 9571b92eb427a8fb7f71557285f987e962c487b6 Mon Sep 17 00:00:00 2001 From: Zach Fleeman Date: Thu, 22 Oct 2026 10:49:14 -0600 Subject: [PATCH 2/3] Show setting errors on the CLI without a traceback --- ffmpeg4discord/__main__.py | 22 ++++++++++++++-------- tests/test_main.py | 14 +++++++++++++- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/ffmpeg4discord/__main__.py b/ffmpeg4discord/__main__.py index b3d164c..44c2bcf 100644 --- a/ffmpeg4discord/__main__.py +++ b/ffmpeg4discord/__main__.py @@ -538,15 +538,21 @@ def form_twopass(): threading.Thread(target=open_browser, args=[port], name="Open Browser").start() # Only this computer can reach the server. The form can write files anywhere, so keep it off the network. app.run("127.0.0.1", port=port) - elif dry_run: - result = twopass.run(settings, dry_run=True) - print(format_commands(result.commands)) - if not approx: - print("\nIf the output lands over the target size, ff4d re-encodes it at a lower bitrate.") else: - result = twopass_loop(twopass, settings, approx) - print(f"\n{format_commands(result.commands)}\n") - print(success_message(result)) + try: + result = twopass.run(settings, dry_run=True) if dry_run else twopass_loop(twopass, settings, approx) + except ValueError as exc: + # TwoPass raises ValueError for settings that can't work. The message says what to change. + logging.error(exc) + raise SystemExit(1) from None + + if dry_run: + print(format_commands(result.commands)) + if not approx: + print("\nIf the output lands over the target size, ff4d re-encodes it at a lower bitrate.") + else: + print(f"\n{format_commands(result.commands)}\n") + print(success_message(result)) if __name__ == "__main__": diff --git a/tests/test_main.py b/tests/test_main.py index 796f5ad..41d0a68 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -36,6 +36,8 @@ def no_cleanup(monkeypatch): @pytest.fixture def run_main(monkeypatch, twopass): """Run main() with the given command line args, without checking PyPI or encoding anything.""" + # Set here, not in _run, so a test can replace twopass_loop before calling run_main. + monkeypatch.setattr(mainmod, "twopass_loop", MagicMock(return_value=result(5))) def _run(**args): cli_args = { @@ -50,7 +52,6 @@ def _run(**args): monkeypatch.setattr(mainmod, "check_for_update", lambda **_: VersionInfo("0.1.9", "0.1.9")) monkeypatch.setattr(mainmod.arguments, "get_args", lambda: cli_args) monkeypatch.setattr(mainmod, "TwoPass", MagicMock(return_value=twopass)) - monkeypatch.setattr(mainmod, "twopass_loop", MagicMock(return_value=result(5))) mainmod.main() return mainmod.twopass_loop @@ -129,6 +130,17 @@ def test_main_dry_run_prints_commands_without_encoding(run_main, twopass, capsys assert ("re-encodes it at a lower bitrate" in out) is not approx +@pytest.mark.parametrize("dry_run", [False, True]) +def test_main_cli_shows_setting_errors_without_a_traceback(run_main, monkeypatch, twopass, caplog, dry_run): + error = ValueError("Audio track 7 doesn't exist.") + twopass.run.side_effect = error + monkeypatch.setattr(mainmod, "twopass_loop", MagicMock(side_effect=error)) + with pytest.raises(SystemExit) as exc_info: + run_main(dry_run=dry_run) + assert exc_info.value.code == 1 + assert caplog.messages == ["Audio track 7 doesn't exist."] + + def test_main_web_ignores_dry_run(run_main, monkeypatch, caplog): app = MagicMock() monkeypatch.setattr(mainmod, "Flask", MagicMock(return_value=app)) From c53c118aed014d04f6fefd10c505952b90f3b93d Mon Sep 17 00:00:00 2001 From: Zach Fleeman Date: Thu, 22 Oct 2026 11:02:40 -0600 Subject: [PATCH 3/3] Show bad --from/--to errors in web mode without a traceback --- ffmpeg4discord/__main__.py | 20 ++++++++++++++------ tests/test_main.py | 10 ++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/ffmpeg4discord/__main__.py b/ffmpeg4discord/__main__.py index 44c2bcf..a5ff5d1 100644 --- a/ffmpeg4discord/__main__.py +++ b/ffmpeg4discord/__main__.py @@ -25,6 +25,7 @@ import threading import time import webbrowser +from contextlib import contextmanager from dataclasses import dataclass, field, replace from pathlib import Path from textwrap import dedent @@ -407,6 +408,16 @@ def parse_encode_form(form, twopass: TwoPass, codecs: list[str]) -> tuple[Encode return settings, [] +@contextmanager +def exit_on_setting_error(): + """TwoPass raises ValueError for settings that can't work. Log its message, which says what to change, and exit.""" + try: + yield + except ValueError as exc: + logging.error(exc) + raise SystemExit(1) from None # from None leaves the ValueError's traceback out + + def main() -> None: """ The main function that parses command-line arguments and starts the encoding process or web server. @@ -450,7 +461,8 @@ def main() -> None: if web: # --from and --to fill in the form's starting trim, so check them before starting the server - twopass.clip_times(settings) + with exit_on_setting_error(): + twopass.clip_times(settings) codecs = available_codecs() source = source_facts(twopass) # No static folder: serving the video's folder would expose every file next to it. @@ -539,12 +551,8 @@ def form_twopass(): # Only this computer can reach the server. The form can write files anywhere, so keep it off the network. app.run("127.0.0.1", port=port) else: - try: + with exit_on_setting_error(): result = twopass.run(settings, dry_run=True) if dry_run else twopass_loop(twopass, settings, approx) - except ValueError as exc: - # TwoPass raises ValueError for settings that can't work. The message says what to change. - logging.error(exc) - raise SystemExit(1) from None if dry_run: print(format_commands(result.commands)) diff --git a/tests/test_main.py b/tests/test_main.py index 41d0a68..8d693fb 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -164,6 +164,16 @@ def test_format_commands_for_one_pass(): ) +def test_main_web_shows_bad_times_without_a_traceback(run_main, monkeypatch, twopass, caplog): + twopass.clip_times.side_effect = ValueError("Time Paradox?") + monkeypatch.setattr(mainmod, "Flask", MagicMock()) + with pytest.raises(SystemExit) as exc_info: + run_main(web=True, port=5000) + assert exc_info.value.code == 1 + assert caplog.messages == ["Time Paradox?"] + mainmod.Flask.assert_not_called() + + def test_main_web_starts_flask_on_port(run_main, monkeypatch): app = MagicMock() monkeypatch.setattr(mainmod, "Flask", MagicMock(return_value=app))