Skip to content

Remove dead code and trim over-built helpers #129

Closed

Description

@zfleeman

A read-through of the whole package found a few spots that do more than they need to. Each one makes the code shorter and easier to follow. Only the _parse_astreams change affects users: a bad --astreams value will stop the run instead of being ignored.

Worth doing

  • Flatten CODEC_OVERRIDES (in twopass.py, used in _generate_params). No codec uses the "both" or "pass1" sections. The comment says the table is module-level so a test can patch in a fake "both" entry, which means the code exists only to serve its own test. Make it {codec: {pass 2 extras}}, replace the loop with one params["pass2"].update(...), and delete the patching test.
  • Rewrite _parse_astreams (in arguments.py). It's 50 lines that accept a list or a string, remove duplicates, skip negative numbers, and log and ignore bad input. Show TwoPass setting errors on the CLI without a traceback #121 made bad settings stop the run with an error, so this should too. About 10 lines can split on commas, convert to int, reject bad or negative values with an error, and remove duplicates with list(dict.fromkeys(...)). Keep list support, because config files can pass a JSON list.
  • Shrink versioning.py. The pyproject.toml fallback (_get_version_from_pyproject) never runs, because uv run installs the package, so metadata.version() always works. The package_name= parameters are never passed. Remove both, along with their tests.
  • Remove the "Functions:" / "Classes:" lists from module docstrings. They're already out of date (the one in __main__.py is missing EncodeJob, progress_seconds, and exit_on_setting_error), and nothing keeps them current. Keep a one-line summary of each module.

Small, optional

  • FILE_SIZE_MULT = 0.00000095367432 can be 1 / 1024**2, which makes its 4-line comment unnecessary.
  • clip_times converts a percentage to a timestamp string, then parses it back into seconds. One parse_time(value, duration) -> float would avoid the round trip.
  • run_pass builds its own command string when it could reuse command_for_display.
  • _normalize_amix_args is a one-liner, and its docstring describes behavior ("unless amix was explicitly set false") that the code doesn't have. Inline it in get_args.
  • logging.getLogger().setLevel(logging.INFO) runs when twopass.py is imported. Move it to main().
  • Add a small red(text) helper for the "\033[31m" + dedent(...).strip() + "\033[0m" pattern. It appears 6 times in twopass.py, once in arguments.py, and once in __main__.py, and saves about 20 lines. Leave windows.py alone, since it must not import other modules' setup.
  • The 96 kbps audio default is written twice: default=96 on the -a flag in arguments.py, and FALLBACK_AUDIO_BR = 96_000 in twopass.py (added in Fix MKV crash and read the source audio bitrate from the first track #130). Have the flag use the constant (default=FALLBACK_AUDIO_BR / 1000) so the two can't drift apart. arguments.py already imports from twopass.py.

Leave as is

  • The Flask app defined inside main(). It reads top to bottom, and pulling it out would be churn.
  • The same form checks in the browser, Flask, and TwoPass. Each layer catches a different case (Web UI: check the form in the browser before submitting #102).
  • The Windows-specific code in remove_temp_folder and command_for_display. Both handle real Windows behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions