diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cbf14f..f22b916 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## 0.7.1 (unreleased) +### Major Changes + +- Change `-r/--render` command line flag to take an output path (#382) + ### Changelog - Update NumPy so `poetry install` on Python 3.8+ won't build NumPy from source (#371) diff --git a/README.md b/README.md index dcf2ff0..0e6e37d 100644 --- a/README.md +++ b/README.md @@ -83,8 +83,8 @@ poetry run corr (args) 1. Play (requires ffmpeg): - `corrscope master.yaml -p/--play` -1. Render and encode MP4 video (requires ffmpeg) - - `corrscope master.yaml -r/--render` +1. Render and encode video (requires ffmpeg) + - `corrscope master.yaml -r/--render file.mp4` (other file extensions supported) ## Contributing diff --git a/corrscope/cli.py b/corrscope/cli.py index d5baa9d..00d1fce 100644 --- a/corrscope/cli.py +++ b/corrscope/cli.py @@ -34,7 +34,7 @@ YAML_EXTS = [".yaml"] # Default extension when writing Config. YAML_NAME = YAML_EXTS[0] -# Default output extension +# Default output extension, only used in GUI and unit tests VIDEO_NAME = ".mp4" @@ -85,14 +85,14 @@ CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"]) # Override default .yaml settings (only if YAML file not supplied) # Incorrect [option] name order: https://github.com/pallets/click/issues/793 @click.option('--audio', '-a', type=File, help= - 'Config: Input path for master audio file') + 'Input path for master audio file') # Disables GUI @click.option('--write', '-w', is_flag=True, help= "Write config YAML file to current directory (don't open GUI).") @click.option('--play', '-p', is_flag=True, help= "Preview (don't open GUI).") -@click.option('--render', '-r', is_flag=True, help= - "Render and encode MP4 video (don't open GUI).") +@click.option('--render', '-r', type=OutFile, help= + "Render and encode video to file (don't open GUI).") # Debugging @click.option('--profile', is_flag=True, help= 'Debug: Write CProfiler snapshot') @@ -105,7 +105,7 @@ def main( # gui write: bool, play: bool, - render: bool, + render: Optional[str], profile: bool, ): """Intelligent oscilloscope visualizer for .wav files. @@ -227,7 +227,7 @@ def main( outputs.append(FFplayOutputConfig()) if render: - video_path = _get_file_name(cfg_path, cfg, ext=VIDEO_NAME) + video_path = render outputs.append(cfg.get_ffmpeg_cfg(video_path)) if outputs: diff --git a/corrscope/corrscope.py b/corrscope/corrscope.py index aa5fc4f..372b0ea 100644 --- a/corrscope/corrscope.py +++ b/corrscope/corrscope.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +import os.path import time from contextlib import ExitStack, contextmanager from enum import unique @@ -91,7 +92,7 @@ class Config( ffmpeg_cli: FFmpegOutputConfig = attr.ib(factory=lambda: FFmpegOutputConfig(None)) def get_ffmpeg_cfg(self, video_path: str) -> FFmpegOutputConfig: - return attr.evolve(self.ffmpeg_cli, path=video_path) + return attr.evolve(self.ffmpeg_cli, path=os.path.abspath(video_path)) benchmark_mode: BenchmarkMode = attr.ib( BenchmarkMode.NONE, converter=BenchmarkMode.by_name diff --git a/corrscope/gui/__init__.py b/corrscope/gui/__init__.py index 03a639a..96055c6 100644 --- a/corrscope/gui/__init__.py +++ b/corrscope/gui/__init__.py @@ -166,9 +166,11 @@ class MainWindow(qw.QMainWindow, Ui_MainWindow): - Otherwise empty string. - self.get_save_filename() calls cli.get_file_stem(). - CLI filename is the same, - but defaults to "corrscope.{yaml, mp4}" instead of empty string. + CLI YAML filename is the same, + but defaults to "corrscope.yaml" instead of empty string. - cli._get_file_name() calls cli.get_file_stem(). + + CLI video filename is explicitly specified by the user. """ def __init__(self, cfg_or_path: Union[Config, Path]): diff --git a/corrscope/outputs.py b/corrscope/outputs.py index 1d48d2b..af199b8 100644 --- a/corrscope/outputs.py +++ b/corrscope/outputs.py @@ -22,6 +22,8 @@ class IOutputConfig(DumpableAttrs): cls: "ClassVar[Type[Output]]" def __call__(self, corr_cfg: "Config") -> "Output": + """Must be called in the .yaml file's directory. + This is used to properly resolve corr_cfg.master_audio.""" return self.cls(corr_cfg, cfg=self) @@ -217,6 +219,25 @@ class PipeOutput(Output): class FFmpegOutputConfig(IOutputConfig): # path=None writes to stdout. + # + # This parameter is not loaded from disk, but set when the user picks a render path + # on the GUI or calls the CLI with `--render out.mp4`. + # + # It must be an absolute path. How is this ensured? + # + # - Paths supplied from the GUI are always absolute. + # - Paths supplied from the CLI must be resolved before storing in + # FFmpegOutputConfig. + # + # Why are relative paths not allowed? Currently, to resolve `corr_cfg.master_audio` + # relative to the config file, we change directories to the config dir, then call + # `abspath(corr_cfg.master_audio)`. As a result, if we called `corr dir/cfg.yaml -r + # out.mp4` and corrscope didn't resolve `out.mp4` before passing into + # FFmpegOutputConfig, it would mistakenly write to `dir/out.mp4`. + # + # In the future, relative paths could be allowed by not switching directories to the + # config dir, and finding another way to resolve `corr_cfg.master_audio` based on + # the config dir. path: Optional[str] args: str = "" diff --git a/tests/test_cli.py b/tests/test_cli.py index e256815..9280056 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -140,15 +140,19 @@ def test_load_yaml_another_dir(mocker, Popen): # Log execution of CorrScope().play() Wave = mocker.spy(corrscope.channel, "Wave") - # Issue: this test does not use cli.main() to compute output path. - # Possible solution: Call cli.main() via Click runner. - output = FFmpegOutputConfig(cli._get_file_name(None, cfg, cli.VIDEO_NAME)) + # Same function as used in cli.py and gui/__init__.py. + output = cfg.get_ffmpeg_cfg(mp4) + corr = CorrScope(cfg, Arguments(subdir, [output])) corr.play() - # Compute absolute paths + # The .wav path (specified in Config) should be resolved relative to the config + # file. wav_abs = abspath(f"{subdir}/{wav}") - mp4_abs = abspath(f"{subdir}/{mp4}") + + # The output video path (specified in CLI --render) should be resolved relative to + # the shell's working directory. + mp4_abs = abspath(mp4) # Test `wave_path` args, kwargs = Wave.call_args