Reject --astreams numbers that don't match an audio track, and show setting errors on the CLI without a traceback

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.
@@ -538,15 +550,17 @@ 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))
+        with exit_on_setting_error():
+            result = twopass.run(settings, dry_run=True) if dry_run else twopass_loop(twopass, settings, approx)
+
+        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__":

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))
 

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))
@@ -152,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))
@@ -355,7 +377,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

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"]