Repository navigation
Reject --astreams numbers that don't match an audio track, and show setting errors on the CLI without a traceback - #122
Merged
Merged
Conversation
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #113. Closes #121.
Changes
--astreamsnumbers must match an audio track (Reject --astreams numbers that don't match an audio track #113).TwoPass.run()now calls a new_check_audio_tracks()right after_check_video_size(), before ffmpeg runs. Any--astreamsnumber past the last audio track raises aValueError, e.g. "Audio track 7 doesn't exist. This file has 2 audio tracks, numbered 0 to 1." A file with no audio says "This file has no audio tracks." Before this, bad numbers were silently dropped, so--astreams 7gave a video with no audio but still spent part of the bitrate budget on audio._apply_audio_filters()no longer filters the list. It usessettings.astreamsas given, becauserun()has already checked it.--no-audio, the check is skipped. No audio goes into the output anyway, so a leftoverastreamsvalue in a config file shouldn't stop the encode._parse_astreams()inarguments.pyalready drops them with a warning.Setting errors on the CLI without a traceback (Show TwoPass setting errors on the CLI without a traceback #121). A new context manager in
__main__.py,exit_on_setting_error(), catches aValueError, logs only its message, and exits with status 1.main()uses it in two places:--dry-runcall totwopass.run()and the normaltwopass_loop()call. This covers everyValueErrorthatTwoPassraises: the new audio track check, the crop and even-size checks, the target-size check, bad--from/--totimes, and the "no bitrate left for video" error on a retry.twopass.clip_times()check that runs before the web server starts, so bad--from/--totimes with--webdon't show a traceback either.Errors from the web page's own form were already shown on the page, and they're unchanged.
Things to look at closely
raise SystemExit(1) from Noneinexit_on_setting_error()hides theValueErrorbehind it. Withoutfrom None, Python would still print the original traceback above the exit.clip_times()now prints without a traceback, but the message itself still has its old color codes and extra indentation. That formatting was already there before this PR, so I didn't change it.test_apply_audio_filters_without_mixing_keeps_one_trackdropped its[5, 1]case, andtest_apply_audio_filters_mixes_selected_tracksnow mixes[0, 2]instead of[0, 2, 5]. Both were testing the old drop-bad-numbers behavior. Intest_main.py,test_encode_audio_stream_selectionposts track0instead of0and2, because thewebfixture's file has one audio track and the web UI's dry run now rejects track 2 with a 400.run_mainfixture. It now replacestwopass_loopwhen the fixture is set up, not inside_run(). That lets the new test swap in atwopass_loopthat raises before callingrun_main(). The other tests get the same mock as before.Testing
uv run pytest --cov: 285 passed, 99% coverage.ruff formatandruff checkare clean.test_twopass.py: a track number past the end raises (alone, and mixed with a valid one),--astreams 0on a file with no audio raises, and--no-audiowith a bad number still encodes.test_main.py: aValueErrorfromtwopass.run()(with--dry-run) ortwopass_loop()(without it) exits with status 1 and logs only the message. A bad--from/--towith--webdoes the same, without starting Flask.example.mp4(2 audio tracks), trimmed to 10 seconds:--astreams 7 --dry-runprintsERROR:root:Audio track 7 doesn't exist. This file has 2 audio tracks, numbered 0 to 1.and exits 1.-x 0x0x9999x9999(normal run) prints the crop error on one line and exits 1.--from 00:00:20 --to 00:00:10 --webprints the "Time Paradox?" message and exits 1 without starting the server.--astreams 0,1 --amix -s 5encodes with[0:a:0][0:a:1]amix=inputs=2and lands at 4.69 MB.