diff --git a/SCRATCHPAD.md b/SCRATCHPAD.md index 1315a27..368f60f 100644 --- a/SCRATCHPAD.md +++ b/SCRATCHPAD.md @@ -9,9 +9,10 @@ - The biggest near-term wins are in startup cost, repeated subprocess work, repeated database query patterns, and general repo hygiene. - This list is intentionally optimization-oriented rather than bug-oriented. Some items below also improve correctness or maintainability, but they were selected because they can reduce runtime cost, operator friction, or iteration overhead. - A first modern integration slice now exists under [`tests/integration/subtrack_mapping`](/home/osgw/.local/src/codex/ffx/tests/integration/subtrack_mapping). Remaining test-suite cleanup is now mostly about migrating and shrinking the legacy harness surface under [`tests/legacy`](/home/osgw/.local/src/codex/ffx/tests/legacy). -- The CLI root now lazy-loads heavy runtime dependencies so lightweight commands such as `version`, `help`, `configure_workstation`, and `upgrade` stay import-light. +- The CLI root now lazy-loads heavy runtime dependencies so lightweight commands such as `version`, `help`, `setup`, `configure_workstation`, and `upgrade` stay import-light. - Shared CLI defaults for container/output tokens now live outside [`src/ffx/ffx_controller.py`](/home/osgw/.local/src/codex/ffx/src/ffx/ffx_controller.py), and a focused unit test locks in the lazy-import contract. - `FileProperties` now uses one cached `ffprobe -show_format -show_streams -of json` call per source file, and the combined payload was confirmed against the Dragonball asset to satisfy both previous probe call sites fully. +- Crop detection now uses configurable sampling windows plus per-process caching keyed by source file and sampling range, and the `cropdetect` CLI command now calls the real `FileProperties.findCropArguments()` path. - Database startup now bootstraps schema only when required tables are actually missing, while version enforcement still runs on ordinary DB-backed context creation. - Helper filename and rich-text utilities now use compiled raw regexes plus translate-based filename filtering, with unit coverage for TMDB suffix rewriting and Rich color stripping. - Process resource limiting now has explicit disabled/default states in the CLI and requirements, and combined CPU-plus-niceness wrapping now executes as `cpulimit -- nice -n ... ` instead of a less explicit prefix chain. @@ -19,37 +20,21 @@ - The process wrapper now uses `subprocess.run(...)` with centralized command formatting plus stable timeout and missing-command error mapping. - Active ORM controllers now use single-query accessors instead of paired `count()` plus `first()` lookups. - Pattern matching now uses cached compiled regexes plus explicit duplicate-match errors, and pattern creation flows no longer persist zero-track patterns. +- The two-step local setup flow now has aligned CLI wrappers for both phases: `ffx setup` for bundle prep and `ffx configure_workstation` for workstation prep, while the shell scripts remain the bootstrap entrypoints before the bundle exists. +- The large detail screens now share one screen-bootstrap helper for context, metadata-filter extraction, and controller wiring, and show-pattern loading now goes through `PatternController` instead of a screen-local session query. ## Focused Snapshot - Highest-leverage application optimizations: - - Revisit crop detection cost now that the probe path is consolidated. + - Decide whether placeholder help/settings screens should ship or disappear. + - Trim dead helpers and other dormant surface that still looks active. - Highest-leverage repo and workflow optimizations: - - Consolidate setup and upgrade tooling to reduce overlapping shell-script responsibilities. - Continue migrating the oversized legacy test/combinator surface into focused modern tests so it is easier to run, debug, and extend. ## Optimization Candidates -1. Crop detection is always a full extra ffmpeg scan -- [`src/ffx/file_properties.py`](/home/osgw/.local/src/codex/ffx/src/ffx/file_properties.py) runs a dedicated `ffmpeg -vf cropdetect` pass for each file when crop detection is requested. -- Optimization: - - Cache crop results for repeated runs on the same source. - - Consider exposing shorter sampling windows or probe presets for large files. -- Expected value: - - Lower latency on repeated experimentation. - -2. Tooling overlap and naming drift -- There are still overlapping workstation-setup entrypoints across [`tools/configure_workstation.sh`](/home/osgw/.local/src/codex/ffx/tools/configure_workstation.sh), [`tools/setup.sh`](/home/osgw/.local/src/codex/ffx/tools/setup.sh), and newer CLI maintenance commands. -- Optimization: - - Decide which scripts remain canonical. - - Replace or remove legacy wrappers once equivalent CLI commands exist. - - Keep CLI maintenance commands and shell wrappers aligned. -- Expected value: - - Less operator confusion. - - Fewer duplicated procedures to maintain. - -3. Placeholder UI surfaces should either ship or disappear +1. Placeholder UI surfaces should either ship or disappear - [`src/ffx/help_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/help_screen.py) and [`src/ffx/settings_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/settings_screen.py) are placeholders. - Optimization: - Either remove them from the active UI surface or complete them. @@ -58,16 +43,7 @@ - Leaner interface. - Lower UX ambiguity. -4. Large Textual screens repeat configuration and controller loading -- Screens such as [`src/ffx/media_details_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/media_details_screen.py), [`src/ffx/pattern_details_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/pattern_details_screen.py), and [`src/ffx/show_details_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/show_details_screen.py) repeat setup patterns and local metadata filtering extraction. -- Optimization: - - Extract a shared screen base or helper for common config/controller/bootstrap logic. - - Reduce repeated table refresh and repeated DB fetch code where possible. -- Expected value: - - Lower maintenance overhead. - - Easier UI iteration. - -5. Several helper functions are unfinished or dead-weight +2. Several helper functions are unfinished or dead-weight - [`src/ffx/helper.py`](/home/osgw/.local/src/codex/ffx/src/ffx/helper.py) contains `permutateList(...): pass`. - There are many combinator and conversion placeholders across tests and migrations. - Optimization: @@ -77,7 +53,7 @@ - Smaller mental model. - Less time spent re-evaluating inactive paths. -6. Test suite shape is expensive to understand and likely expensive to run +3. Test suite shape is expensive to understand and likely expensive to run - The project still carries a large legacy matrix of combinator files under [`tests/legacy`](/home/osgw/.local/src/codex/ffx/tests/legacy), several placeholder `pass` implementations, and at least one suspicious filename with an embedded space: [`tests/legacy/disposition_combinator_2_3 .py`](/home/osgw/.local/src/codex/ffx/tests/legacy/disposition_combinator_2_3 .py). - A first focused replacement slice now exists in [`tests/integration/subtrack_mapping/test_cli_bundle.py`](/home/osgw/.local/src/codex/ffx/tests/integration/subtrack_mapping/test_cli_bundle.py), so the remaining work is migration and consolidation rather than creating the modern test shape from scratch. - Optimization: @@ -102,9 +78,9 @@ ## Next 1. Triage the list into quick wins, medium refactors, and long-horizon cleanup. -2. Tackle the cheapest high-impact items first: - - crop detection sampling or caching pass. -3. Decide which setup and upgrade entrypoints stay canonical before adding more maintenance surface. +2. Tackle the cheapest remaining product-surface cleanup first: + - placeholder UI surfaces and dead helper cleanup. +3. Continue replacing oversized legacy test matrices with focused modern integration and unit coverage. ## Delete When diff --git a/requirements/architecture.md b/requirements/architecture.md index 1f0cd17..42be71b 100644 --- a/requirements/architecture.md +++ b/requirements/architecture.md @@ -32,12 +32,13 @@ ## High-Level Building Blocks - Frontend, CLI, API, or worker: - - A Click-based CLI in [`src/ffx/cli.py`](/home/osgw/.local/src/codex/ffx/src/ffx/cli.py), exposed as the `ffx` command and via `python -m ffx`. + - A Click-based CLI in [`src/ffx/cli.py`](/home/osgw/.local/src/codex/ffx/src/ffx/cli.py), exposed as the `ffx` command and via `python -m ffx`, including lightweight maintenance wrappers for bundle setup, workstation preparation, and upgrade tasks. - A Textual terminal UI rooted in [`src/ffx/ffx_app.py`](/home/osgw/.local/src/codex/ffx/src/ffx/ffx_app.py) with screens for shows, patterns, file inspection, tracks, tags, and shifted seasons. - Core business logic: - Descriptor objects model media files, shows, and tracks. - Controllers encapsulate CRUD operations and workflow orchestration for shows, patterns, tags, tracks, season shifts, configuration, and conversion. - `MediaDescriptorChangeSet` computes differences between a file and its stored target schema to drive metadata and disposition updates. + - File inspection caches combined `ffprobe` data and crop-detection results per source and sampling window within one process to avoid repeated subprocess work. - Storage: - SQLite via SQLAlchemy ORM, with schema rooted in shows, patterns, tracks, media tags, track tags, shifted seasons, and generic properties. - A configuration JSON file supplies optional path, metadata-filtering, and filename-template settings. diff --git a/requirements/project.md b/requirements/project.md index 9edf200..8182705 100644 --- a/requirements/project.md +++ b/requirements/project.md @@ -35,10 +35,12 @@ ## Functional Requirements -- The system shall provide a CLI entrypoint named `ffx` with commands for `convert`, `inspect`, `shows`, `unmux`, `cropdetect`, `configure_workstation`, `upgrade`, `version`, and `help`. +- The system shall provide a CLI entrypoint named `ffx` with commands for `convert`, `inspect`, `shows`, `unmux`, `cropdetect`, `setup`, `configure_workstation`, `upgrade`, `version`, and `help`. - The system shall support a two-step local installation and preparation flow: - - `tools/setup.sh` is the first step and shall own bundle virtualenv creation, package installation, shell alias exposure, and optional Python test-package installation. - - `tools/configure_workstation.sh` is the second step and shall own workstation dependency checks and installation plus local config and directory seeding. + - `tools/setup.sh` is the bootstrap entrypoint for the first step and shall own bundle virtualenv creation, package installation, shell alias exposure, and optional Python test-package installation. + - `tools/configure_workstation.sh` is the bootstrap entrypoint for the second step and shall own workstation dependency checks and installation plus local config and directory seeding. + - After the bundle is installed, `ffx setup` and `ffx configure_workstation` shall remain aligned wrapper entrypoints for those same two steps. +- The CLI command `ffx setup` shall act as a wrapper for the first-step bundle-preparation flow in `tools/setup.sh`. - The CLI command `ffx configure_workstation` shall act as a wrapper for the second-step preparation flow in `tools/configure_workstation.sh`. - The system shall persist reusable normalization rules in SQLite for: - shows and show formatting digits, @@ -67,6 +69,7 @@ - `--cpu` shall accept either a positive absolute `cpulimit` value such as `200`, or a percentage suffixed with `%` such as `25%` to represent a share of present CPUs; omitting the option or using `0` shall disable CPU limiting. - When both limits are configured, the process wrapper shall execute the target command through `cpulimit` around a `nice -n ...` invocation so both limits apply to the launched media command. - The system shall support extracting streams into separate files via `unmux` and reporting suggested crop parameters via `cropdetect`. +- Crop detection shall use a configurable sampling window, defaulting to a 60-second seek and a 180-second analysis duration, and repeated crop-detection requests for the same source plus sampling window shall reuse cached results within one process. - The system shall handle invalid input and system failures gracefully by logging warnings or raising `click` errors for missing files, invalid media, missing TMDB credentials, incompatible database versions, and ambiguous track dispositions when prompting is disabled. ## Quality Requirements @@ -94,7 +97,7 @@ - `ffmpeg`, `ffprobe`, and `cpulimit`. - TMDB API access through `TMDB_API_KEY` for metadata enrichment. - Installation assumptions: - - The Python-side bundle install step and optional Python test extras are managed by `tools/setup.sh`. + - The Python-side bundle install step and optional Python test extras are managed by `tools/setup.sh`, with `ffx setup` as the aligned wrapper after bootstrap. - The workstation-preparation step is managed separately by `tools/configure_workstation.sh` or `ffx configure_workstation`. ## Acceptance Scope diff --git a/src/ffx/cli.py b/src/ffx/cli.py index 50d6e25..296fbfd 100755 --- a/src/ffx/cli.py +++ b/src/ffx/cli.py @@ -15,6 +15,8 @@ if __package__ in (None, ''): from ffx.constants import ( DEFAULT_AC3_BANDWIDTH, + DEFAULT_CROPDETECT_DURATION_SECONDS, + DEFAULT_CROPDETECT_SEEK_SECONDS, DEFAULT_CONTAINER_EXTENSION, DEFAULT_CONTAINER_FORMAT, DEFAULT_DTS_BANDWIDTH, @@ -29,12 +31,20 @@ if TYPE_CHECKING: from ffx.media_descriptor import MediaDescriptor from ffx.track_descriptor import TrackDescriptor -LIGHTWEIGHT_COMMANDS = {None, 'version', 'help', 'configure_workstation', 'upgrade'} +LIGHTWEIGHT_COMMANDS = {None, 'version', 'help', 'setup', 'configure_workstation', 'upgrade'} CPU_OPTION_HELP = ( "Limit CPU for started processes. Use an absolute cpulimit value such as 200 " + "(about 2 cores), or use a percentage such as 25% for a share of present cores. " + "Omit to disable; 0 also disables." ) +CROPDETECT_SEEK_OPTION_HELP = ( + "Start crop detection this many seconds into the input. " + + "Useful for skipping logos, intros, or black frames." +) +CROPDETECT_DURATION_OPTION_HELP = ( + "Analyze this many seconds for crop detection. " + + "Shorter windows are faster; longer windows are usually steadier." +) def normalizeNicenessOption(ctx, param, value): @@ -111,7 +121,9 @@ def version(): @ffx.command() def help(): click.echo(f"ffx {VERSION}\n") - click.echo(f"Usage: ffx [input file] [output file] [vp9|av1] [q=[nn[,nn,...]]] [p=nn] [a=nnn[k]] [ac3=nnn[k]] [dts=nnn[k]] [crop]") + click.echo("Maintenance commands: setup, configure_workstation, upgrade") + click.echo("Media commands: shows, inspect, convert, unmux, cropdetect") + click.echo("Use 'ffx --help' or 'ffx --help' for full command help.") def getRepoRootPath(): @@ -123,6 +135,10 @@ def getConfigureWorkstationScriptPath(): return os.path.join(getRepoRootPath(), 'tools', 'configure_workstation.sh') +def getSetupScriptPath(): + return os.path.join(getRepoRootPath(), 'tools', 'setup.sh') + + def getBundleVenvDirectory(): return os.path.join(os.path.expanduser('~'), '.local', 'share', 'ffx.venv') @@ -153,23 +169,11 @@ def getTrackedGitChanges(repoPath): return [line for line in completed.stdout.splitlines() if line.strip()] -@ffx.command(name='configure_workstation') -@click.pass_context -@click.option('--check', is_flag=True, default=False, help='Only verify workstation-configuration readiness') -@click.argument('configure_args', nargs=-1, type=click.UNPROCESSED) -def configure_workstation(ctx, check, configure_args): - """Prepare workstation dependencies and local config after bundle install.""" - configureScriptPath = getConfigureWorkstationScriptPath() +def runScriptWrapper(ctx, scriptPath, missingDescription, commandArgs): + if not os.path.isfile(scriptPath): + raise click.ClickException(f"{missingDescription} not found at {scriptPath}") - if not os.path.isfile(configureScriptPath): - raise click.ClickException(f"Workstation configuration script not found at {configureScriptPath}") - - commandSequence = ['bash', configureScriptPath] - - if check: - commandSequence.append('--check') - - commandSequence += list(configure_args) + commandSequence = ['bash', scriptPath] + list(commandArgs) if ctx.obj.get('dry_run', False): click.echo(' '.join(commandSequence)) @@ -179,6 +183,44 @@ def configure_workstation(ctx, check, configure_args): ctx.exit(completed.returncode) +@ffx.command(name='setup') +@click.pass_context +@click.option('--check', is_flag=True, default=False, help='Only verify bundle-setup readiness') +@click.option('--with-tests', is_flag=True, default=False, help='Also install or verify Python test packages in the bundle venv') +@click.argument('setup_args', nargs=-1, type=click.UNPROCESSED) +def setup(ctx, check, with_tests, setup_args): + """Prepare or repair the FFX bundle virtualenv and shell alias.""" + commandArgs = [] + + if check: + commandArgs.append('--check') + if with_tests: + commandArgs.append('--with-tests') + + commandArgs += list(setup_args) + runScriptWrapper(ctx, getSetupScriptPath(), "Bundle setup script", commandArgs) + + +@ffx.command(name='configure_workstation') +@click.pass_context +@click.option('--check', is_flag=True, default=False, help='Only verify workstation-configuration readiness') +@click.argument('configure_args', nargs=-1, type=click.UNPROCESSED) +def configure_workstation(ctx, check, configure_args): + """Prepare workstation dependencies and local config after bundle install.""" + commandArgs = [] + + if check: + commandArgs.append('--check') + + commandArgs += list(configure_args) + runScriptWrapper( + ctx, + getConfigureWorkstationScriptPath(), + "Workstation configuration script", + commandArgs, + ) + + @ffx.command(name='upgrade') @click.pass_context @click.option('--branch', type=str, default='', help='Checkout this branch before pulling') @@ -389,10 +431,26 @@ def unmux(ctx, show_default='disabled', help=CPU_OPTION_HELP, ) +@click.option( + '--crop-seek', + type=click.IntRange(min=0), + default=DEFAULT_CROPDETECT_SEEK_SECONDS, + show_default=True, + help=CROPDETECT_SEEK_OPTION_HELP, +) +@click.option( + '--crop-duration', + type=click.IntRange(min=1), + default=DEFAULT_CROPDETECT_DURATION_SECONDS, + show_default=True, + help=CROPDETECT_DURATION_OPTION_HELP, +) def cropdetect(ctx, paths, nice, - cpu): + cpu, + crop_seek, + crop_duration): from ffx.file_properties import FileProperties existingSourcePaths = [p for p in paths if os.path.isfile(p)] @@ -402,6 +460,10 @@ def cropdetect(ctx, ctx.obj['resource_limits']['niceness'] = nice ctx.obj['resource_limits']['cpu_limit'] = cpu ctx.obj['resource_limits']['cpu_percent'] = cpu + ctx.obj['cropdetect'] = { + 'seek_seconds': crop_seek, + 'duration_seconds': crop_duration, + } for sourcePath in existingSourcePaths: @@ -409,7 +471,7 @@ def cropdetect(ctx, try: fp = FileProperties(ctx.obj, sourcePath) - cropParams = fp.findCropParams() + cropParams = fp.findCropArguments() click.echo(cropParams) @@ -506,6 +568,20 @@ def checkUniqueDispositions(context, mediaDescriptor: MediaDescriptor): @click.option('--rearrange-streams', type=str, default="", help='Rearrange output streams order. Use format comma separated integers') @click.option("--crop", is_flag=False, flag_value="auto", default="none") +@click.option( + '--crop-seek', + type=click.IntRange(min=0), + default=DEFAULT_CROPDETECT_SEEK_SECONDS, + show_default=True, + help='When --crop auto is used, start crop detection this many seconds into the input.', +) +@click.option( + '--crop-duration', + type=click.IntRange(min=1), + default=DEFAULT_CROPDETECT_DURATION_SECONDS, + show_default=True, + help='When --crop auto is used, analyze this many seconds for crop detection.', +) @click.option("--cut", is_flag=False, flag_value="default", default="none") @click.option("--output-directory", type=str, default='') @@ -578,6 +654,8 @@ def convert(ctx, rearrange_streams, crop, + crop_seek, + crop_duration, cut, output_directory, @@ -652,6 +730,10 @@ def convert(ctx, context['resource_limits']['niceness'] = nice context['resource_limits']['cpu_limit'] = cpu context['resource_limits']['cpu_percent'] = cpu + context['cropdetect'] = { + 'seek_seconds': crop_seek, + 'duration_seconds': crop_duration, + } context['import_subtitles'] = (subtitle_directory and subtitle_prefix) diff --git a/src/ffx/constants.py b/src/ffx/constants.py index b1471db..63f67b9 100644 --- a/src/ffx/constants.py +++ b/src/ffx/constants.py @@ -16,6 +16,9 @@ DEFAULT_AC3_BANDWIDTH = "256" DEFAULT_DTS_BANDWIDTH = "320" DEFAULT_7_1_BANDWIDTH = "384" +DEFAULT_CROPDETECT_SEEK_SECONDS = 60 +DEFAULT_CROPDETECT_DURATION_SECONDS = 180 + DEFAULT_cut_start = 60 DEFAULT_cut_length = 180 diff --git a/src/ffx/file_properties.py b/src/ffx/file_properties.py index 1b45a06..2f8d0af 100644 --- a/src/ffx/file_properties.py +++ b/src/ffx/file_properties.py @@ -1,5 +1,11 @@ import os, re, json +from .constants import ( + DEFAULT_CROPDETECT_DURATION_SECONDS, + DEFAULT_CROPDETECT_SEEK_SECONDS, + FFMPEG_COMMAND_TOKENS, + FFMPEG_NULL_OUTPUT_TOKENS, +) from .media_descriptor import MediaDescriptor from .pattern_controller import PatternController @@ -11,6 +17,7 @@ from ffx.model.pattern import Pattern class FileProperties(): + _cropdetect_cache: dict[tuple[str, int, int, int, int], dict[str, str]] = {} FILE_EXTENSIONS = ['mkv', 'mp4', 'avi', 'flv', 'webm'] FFPROBE_COMMAND_TOKENS = ["ffprobe", "-hide_banner", "-show_format", "-show_streams", "-of", "json"] @@ -81,6 +88,34 @@ class FileProperties(): self.__ffprobeData = None + def _getCropdetectWindow(self): + cropdetectContext = self.context.get('cropdetect', {}) + + seekSeconds = int(cropdetectContext.get('seek_seconds', DEFAULT_CROPDETECT_SEEK_SECONDS)) + durationSeconds = int(cropdetectContext.get('duration_seconds', DEFAULT_CROPDETECT_DURATION_SECONDS)) + + if seekSeconds < 0: + raise ValueError("Crop detection seek seconds must be zero or greater.") + if durationSeconds <= 0: + raise ValueError("Crop detection duration seconds must be greater than zero.") + + return seekSeconds, durationSeconds + + def _getCropdetectCacheKey(self): + sourceStat = os.stat(self.__sourcePath) + seekSeconds, durationSeconds = self._getCropdetectWindow() + + return ( + os.path.abspath(self.__sourcePath), + sourceStat.st_mtime_ns, + sourceStat.st_size, + seekSeconds, + durationSeconds, + ) + + @classmethod + def _clear_cropdetect_cache(cls): + cls._cropdetect_cache.clear() def _getFfprobeData(self): if self.__ffprobeData is not None: @@ -172,16 +207,25 @@ class FileProperties(): def findCropArguments(self): """""" - # ffmpeg -i -vf cropdetect -f null - - ffprobeOutput, ffprobeError, returnCode = executeProcess(["ffmpeg", "-i", - self.__sourcePath, - "-vf", "cropdetect", - "-ss", "60", - "-t", "180", - "-f", "null", "-" - ]) + cacheKey = self._getCropdetectCacheKey() + cachedCropArguments = FileProperties._cropdetect_cache.get(cacheKey) + if cachedCropArguments is not None: + self.__logger.debug( + "FileProperties.findCropArguments(): Reusing cached cropdetect result for %s", + self.__sourcePath, + ) + return dict(cachedCropArguments) - errorLines = ffprobeError.split('\n') + seekSeconds, durationSeconds = self._getCropdetectWindow() + + cropdetectCommand = ( + list(FFMPEG_COMMAND_TOKENS) + + ["-ss", str(seekSeconds), "-i", self.__sourcePath, "-t", str(durationSeconds), "-vf", "cropdetect"] + + list(FFMPEG_NULL_OUTPUT_TOKENS) + ) + _ffmpegOutput, ffmpegError, returnCode = executeProcess(cropdetectCommand, context=self.context) + + errorLines = ffmpegError.split('\n') crops = {} for el in errorLines: @@ -194,21 +238,26 @@ class FileProperties(): crops[cropParam] = crops.get(cropParam, 0) + 1 if crops: - cropHistogram = sorted(crops, reverse=True) - cropString = cropHistogram[0] + cropString = max(crops.items(), key=lambda item: (item[1], item[0]))[0] cropTokens = cropString.split('=') cropValueTokens = cropTokens[1] cropValues = cropValueTokens.split(':') - return { + cropArguments = { CropFilter.OUTPUT_WIDTH_KEY: cropValues[0], CropFilter.OUTPUT_HEIGHT_KEY: cropValues[1], CropFilter.OFFSET_X_KEY: cropValues[2], CropFilter.OFFSET_Y_KEY: cropValues[3] } - else: - return {} + FileProperties._cropdetect_cache[cacheKey] = dict(cropArguments) + return cropArguments + + if returnCode != 0: + raise Exception(f"ffmpeg cropdetect returned with error {returnCode}") + + FileProperties._cropdetect_cache[cacheKey] = {} + return {} def getMediaDescriptor(self): diff --git a/src/ffx/media_details_screen.py b/src/ffx/media_details_screen.py index dfb837f..7f61622 100644 --- a/src/ffx/media_details_screen.py +++ b/src/ffx/media_details_screen.py @@ -6,13 +6,9 @@ from textual.containers import Grid from ffx.audio_layout import AudioLayout -from .pattern_controller import PatternController -from .show_controller import ShowController -from .track_controller import TrackController -from .tag_controller import TagController - from .show_details_screen import ShowDetailsScreen from .pattern_details_screen import PatternDetailsScreen +from .screen_support import build_screen_bootstrap, build_screen_controllers from ffx.track_type import TrackType from ffx.track_codec import TrackCodec @@ -135,29 +131,23 @@ class MediaDetailsScreen(Screen): def __init__(self): super().__init__() - self.context = self.app.getContext() - self.Session = self.context['database']['session'] # convenience + bootstrap = build_screen_bootstrap(self.app.getContext()) + self.context = bootstrap.context + self.__removeGlobalKeys = bootstrap.remove_global_keys + self.__ignoreGlobalKeys = bootstrap.ignore_global_keys - self.__configurationData = self.context['config'].getData() - - metadataConfiguration = self.__configurationData['metadata'] if 'metadata' in self.__configurationData.keys() else {} - - self.__signatureTags = metadataConfiguration['signature'] if 'signature' in metadataConfiguration.keys() else {} - self.__removeGlobalKeys = metadataConfiguration['remove'] if 'remove' in metadataConfiguration.keys() else [] - self.__ignoreGlobalKeys = metadataConfiguration['ignore'] if 'ignore' in metadataConfiguration.keys() else [] - self.__removeTrackKeys = (metadataConfiguration['streams']['remove'] - if 'streams' in metadataConfiguration.keys() - and 'remove' in metadataConfiguration['streams'].keys() else []) - self.__ignoreTrackKeys = (metadataConfiguration['streams']['ignore'] - if 'streams' in metadataConfiguration.keys() - and 'ignore' in metadataConfiguration['streams'].keys() else []) - - - self.__pc = PatternController(context = self.context) - self.__sc = ShowController(context = self.context) - self.__tc = TrackController(context = self.context) - self.__tac = TagController(context = self.context) + controllers = build_screen_controllers( + self.context, + pattern=True, + show=True, + track=True, + tag=True, + ) + self.__pc = controllers['pattern'] + self.__sc = controllers['show'] + self.__tc = controllers['track'] + self.__tac = controllers['tag'] if not 'command' in self.context.keys() or self.context['command'] != 'inspect': raise click.ClickException(f"MediaDetailsScreen.__init__(): Can only perform command 'inspect'") diff --git a/src/ffx/pattern_controller.py b/src/ffx/pattern_controller.py index e10d5a7..b0886ee 100644 --- a/src/ffx/pattern_controller.py +++ b/src/ffx/pattern_controller.py @@ -305,6 +305,29 @@ class PatternController: if session is not None: session.close() + def getPatternsForShow(self, showId: int) -> list[Pattern]: + + if type(showId) is not int: + raise ValueError( + "PatternController.getPatternsForShow(): Argument showId is required to be of type int" + ) + + session = None + try: + session = self.Session() + return ( + session.query(Pattern) + .filter(Pattern.show_id == int(showId)) + .order_by(Pattern.id) + .all() + ) + + except Exception as ex: + raise click.ClickException(f"PatternController.getPatternsForShow(): {repr(ex)}") + finally: + if session is not None: + session.close() + def getPattern(self, patternId: int): if type(patternId) is not int: diff --git a/src/ffx/pattern_details_screen.py b/src/ffx/pattern_details_screen.py index bdb1257..fe64352 100644 --- a/src/ffx/pattern_details_screen.py +++ b/src/ffx/pattern_details_screen.py @@ -7,16 +7,12 @@ from textual.containers import Grid from ffx.model.pattern import Pattern -from .pattern_controller import PatternController -from .show_controller import ShowController -from .track_controller import TrackController -from .tag_controller import TagController - from .track_details_screen import TrackDetailsScreen from .track_delete_screen import TrackDeleteScreen from .tag_details_screen import TagDetailsScreen from .tag_delete_screen import TagDeleteScreen +from .screen_support import build_screen_bootstrap, build_screen_controllers from ffx.track_type import TrackType @@ -107,27 +103,23 @@ class PatternDetailsScreen(Screen): def __init__(self, patternId = None, showId = None): super().__init__() - self.context = self.app.getContext() - self.Session = self.context['database']['session'] # convenience + bootstrap = build_screen_bootstrap(self.app.getContext()) + self.context = bootstrap.context - self.__configurationData = self.context['config'].getData() + self.__removeGlobalKeys = bootstrap.remove_global_keys + self.__ignoreGlobalKeys = bootstrap.ignore_global_keys - metadataConfiguration = self.__configurationData['metadata'] if 'metadata' in self.__configurationData.keys() else {} - - self.__signatureTags = metadataConfiguration['signature'] if 'signature' in metadataConfiguration.keys() else {} - self.__removeGlobalKeys = metadataConfiguration['remove'] if 'remove' in metadataConfiguration.keys() else [] - self.__ignoreGlobalKeys = metadataConfiguration['ignore'] if 'ignore' in metadataConfiguration.keys() else [] - self.__removeTrackKeys = (metadataConfiguration['streams']['remove'] - if 'streams' in metadataConfiguration.keys() - and 'remove' in metadataConfiguration['streams'].keys() else []) - self.__ignoreTrackKeys = (metadataConfiguration['streams']['ignore'] - if 'streams' in metadataConfiguration.keys() - and 'ignore' in metadataConfiguration['streams'].keys() else []) - - self.__pc = PatternController(context = self.context) - self.__sc = ShowController(context = self.context) - self.__tc = TrackController(context = self.context) - self.__tac = TagController(context = self.context) + controllers = build_screen_controllers( + self.context, + pattern=True, + show=True, + track=True, + tag=True, + ) + self.__pc = controllers['pattern'] + self.__sc = controllers['show'] + self.__tc = controllers['track'] + self.__tac = controllers['tag'] self.__pattern : Pattern = self.__pc.getPattern(patternId) if patternId is not None else None self.__showDescriptor = self.__sc.getShowDescriptor(showId) if showId is not None else None @@ -135,26 +127,6 @@ class PatternDetailsScreen(Screen): self.__draftTags : dict[str, str] = {} - #TODO: per controller - def loadTracks(self, show_id): - - try: - - tracks = {} - tracks['audio'] = {} - tracks['subtitle'] = {} - - s = self.Session() - q = s.query(Pattern).filter(Pattern.show_id == int(show_id)) - - return [{'id': int(p.id), 'pattern': p.pattern} for p in q.all()] - - except Exception as ex: - raise click.ClickException(f"loadTracks(): {repr(ex)}") - finally: - s.close() - - def updateTracks(self): self.tracksTable.clear() diff --git a/src/ffx/screen_support.py b/src/ffx/screen_support.py new file mode 100644 index 0000000..a7e24b6 --- /dev/null +++ b/src/ffx/screen_support.py @@ -0,0 +1,65 @@ +from __future__ import annotations + +from dataclasses import dataclass + +from .pattern_controller import PatternController +from .show_controller import ShowController +from .shifted_season_controller import ShiftedSeasonController +from .tag_controller import TagController +from .tmdb_controller import TmdbController +from .track_controller import TrackController + + +@dataclass(frozen=True) +class ScreenBootstrap: + context: dict + configuration_data: dict + signature_tags: dict + remove_global_keys: list + ignore_global_keys: list + remove_track_keys: list + ignore_track_keys: list + + +def build_screen_bootstrap(context: dict) -> ScreenBootstrap: + configurationData = context['config'].getData() + metadataConfiguration = configurationData.get('metadata', {}) + streamMetadataConfiguration = metadataConfiguration.get('streams', {}) + + return ScreenBootstrap( + context=context, + configuration_data=configurationData, + signature_tags=metadataConfiguration.get('signature', {}), + remove_global_keys=metadataConfiguration.get('remove', []), + ignore_global_keys=metadataConfiguration.get('ignore', []), + remove_track_keys=streamMetadataConfiguration.get('remove', []), + ignore_track_keys=streamMetadataConfiguration.get('ignore', []), + ) + + +def build_screen_controllers( + context: dict, + *, + pattern: bool = False, + show: bool = False, + track: bool = False, + tag: bool = False, + tmdb: bool = False, + shifted_season: bool = False, +) -> dict[str, object]: + controllers = {} + + if pattern: + controllers['pattern'] = PatternController(context=context) + if show: + controllers['show'] = ShowController(context=context) + if track: + controllers['track'] = TrackController(context=context) + if tag: + controllers['tag'] = TagController(context=context) + if tmdb: + controllers['tmdb'] = TmdbController() + if shifted_season: + controllers['shifted_season'] = ShiftedSeasonController(context=context) + + return controllers diff --git a/src/ffx/show_details_screen.py b/src/ffx/show_details_screen.py index c0e2153..8d840c1 100644 --- a/src/ffx/show_details_screen.py +++ b/src/ffx/show_details_screen.py @@ -5,16 +5,9 @@ from textual.widgets import Header, Footer, Static, Button, DataTable, Input from textual.containers import Grid from textual.widgets._data_table import CellDoesNotExist -from ffx.model.pattern import Pattern - from .pattern_details_screen import PatternDetailsScreen from .pattern_delete_screen import PatternDeleteScreen -from .show_controller import ShowController -from .pattern_controller import PatternController -from .tmdb_controller import TmdbController -from .shifted_season_controller import ShiftedSeasonController - from .show_descriptor import ShowDescriptor from .shifted_season_details_screen import ShiftedSeasonDetailsScreen @@ -23,6 +16,7 @@ from .shifted_season_delete_screen import ShiftedSeasonDeleteScreen from ffx.model.shifted_season import ShiftedSeason from .helper import filterFilename +from .screen_support import build_screen_bootstrap, build_screen_controllers # Screen[dict[int, str, int]] @@ -94,31 +88,24 @@ class ShowDetailsScreen(Screen): def __init__(self, showId = None): super().__init__() - self.context = self.app.getContext() - self.Session = self.context['database']['session'] # convenience - - self.__sc = ShowController(context = self.context) - self.__pc = PatternController(context = self.context) - self.__tc = TmdbController() - self.__ssc = ShiftedSeasonController(context = self.context) + bootstrap = build_screen_bootstrap(self.app.getContext()) + self.context = bootstrap.context + + controllers = build_screen_controllers( + self.context, + pattern=True, + show=True, + tmdb=True, + shifted_season=True, + ) + self.__sc = controllers['show'] + self.__pc = controllers['pattern'] + self.__tc = controllers['tmdb'] + self.__ssc = controllers['shifted_season'] self.__showDescriptor = self.__sc.getShowDescriptor(showId) if showId is not None else None - def loadPatterns(self, show_id : int): - - try: - s = self.Session() - q = s.query(Pattern).filter(Pattern.show_id == int(show_id)) - - return [{'id': int(p.id), 'pattern': str(p.pattern)} for p in q.all()] - - except Exception as ex: - raise click.ClickException(f"ShowDetailsScreen.loadPatterns(): {repr(ex)}") - finally: - s.close() - - def updateShiftedSeasons(self): @@ -166,10 +153,8 @@ class ShowDetailsScreen(Screen): #raise click.ClickException(f"show_id {showId}") - patternList = self.loadPatterns(showId) - # raise click.ClickException(f"patternList {patternList}") - for pattern in patternList: - row = (pattern['pattern'],) + for pattern in self.__pc.getPatternsForShow(showId): + row = (pattern.getPattern(),) self.patternTable.add_row(*map(str, row)) self.updateShiftedSeasons() @@ -489,4 +474,4 @@ class ShowDetailsScreen(Screen): self.updateShiftedSeasons() def handle_delete_shifted_season(self, screenResult): - self.updateShiftedSeasons() \ No newline at end of file + self.updateShiftedSeasons() diff --git a/tests/unit/test_cli_lazy_imports.py b/tests/unit/test_cli_lazy_imports.py index 36b6553..c535cb7 100644 --- a/tests/unit/test_cli_lazy_imports.py +++ b/tests/unit/test_cli_lazy_imports.py @@ -99,6 +99,43 @@ class CliLazyImportTests(unittest.TestCase): result["modules"], ) + def test_lightweight_setup_command_stays_light(self): + result = self.run_python( + textwrap.dedent( + f""" + import json + import sys + from click.testing import CliRunner + + sys.path.insert(0, {str(SRC_ROOT)!r}) + + import ffx.cli + + runner = CliRunner() + invoke_result = runner.invoke( + ffx.cli.ffx, + ["--dry-run", "setup", "--check", "--with-tests"], + ) + if invoke_result.exit_code != 0: + raise SystemExit(invoke_result.output) + + print(json.dumps({{ + "output": invoke_result.output, + "modules": {{ + module_name: module_name in sys.modules + for module_name in {HEAVY_MODULES!r} + }}, + }})) + """ + ) + ) + + self.assertIn("tools/setup.sh --check --with-tests", result["output"]) + self.assertTrue( + all(not is_loaded for is_loaded in result["modules"].values()), + result["modules"], + ) + def test_convert_help_describes_absolute_and_percent_cpu_limits(self): result = self.run_python( textwrap.dedent( diff --git a/tests/unit/test_file_properties_probe.py b/tests/unit/test_file_properties_probe.py index 6961d0a..d99012b 100644 --- a/tests/unit/test_file_properties_probe.py +++ b/tests/unit/test_file_properties_probe.py @@ -4,6 +4,7 @@ import json import logging from pathlib import Path import sys +from types import SimpleNamespace import unittest from unittest.mock import patch @@ -106,6 +107,69 @@ class FilePropertiesProbeTests(unittest.TestCase): + ["/tmp/example_s01e01.mkv"] ) + def test_cropdetect_uses_configured_window_and_caches_results(self): + file_properties_module = self.import_module() + file_properties_module.FileProperties._clear_cropdetect_cache() + + cropdetect_stderr = "\n".join( + [ + "[Parsed_cropdetect_0] crop=1440:1080:240:0", + "[Parsed_cropdetect_0] crop=1440:1080:240:0", + "[Parsed_cropdetect_0] crop=1438:1080:242:0", + ] + ) + context = self.make_context() + context["cropdetect"] = {"seek_seconds": 15, "duration_seconds": 45} + + with ( + patch.object( + file_properties_module.os, + "stat", + return_value=SimpleNamespace(st_mtime_ns=1234, st_size=5678), + ), + patch.object(file_properties_module, "PatternController", DummyPatternController), + patch.object( + file_properties_module, + "executeProcess", + return_value=("", cropdetect_stderr, 0), + ) as mocked_execute, + ): + file_properties = file_properties_module.FileProperties( + context, + "/tmp/example_s01e01.mkv", + ) + + first = file_properties.findCropArguments() + second = file_properties.findCropArguments() + + self.assertEqual(first, second) + self.assertEqual( + { + "output_width": "1440", + "output_height": "1080", + "x_offset": "240", + "y_offset": "0", + }, + first, + ) + mocked_execute.assert_called_once_with( + list(file_properties_module.FFMPEG_COMMAND_TOKENS) + + [ + "-ss", + "15", + "-i", + "/tmp/example_s01e01.mkv", + "-t", + "45", + "-vf", + "cropdetect", + ] + + list(file_properties_module.FFMPEG_NULL_OUTPUT_TOKENS), + context=context, + ) + + file_properties_module.FileProperties._clear_cropdetect_cache() + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_screen_support.py b/tests/unit/test_screen_support.py new file mode 100644 index 0000000..5bc8b3e --- /dev/null +++ b/tests/unit/test_screen_support.py @@ -0,0 +1,86 @@ +from __future__ import annotations + +from pathlib import Path +import sys +import unittest +from unittest.mock import patch + + +SRC_ROOT = Path(__file__).resolve().parents[2] / "src" + +if str(SRC_ROOT) not in sys.path: + sys.path.insert(0, str(SRC_ROOT)) + + +from ffx import screen_support # noqa: E402 + + +class StaticConfig: + def __init__(self, data): + self._data = data + + def getData(self): + return self._data + + +class ScreenSupportTests(unittest.TestCase): + def make_context(self): + return { + "config": StaticConfig( + { + "metadata": { + "signature": {"RECODED_WITH": "FFX"}, + "remove": ["VERSION-eng"], + "ignore": ["ENCODER"], + "streams": { + "remove": ["BPS"], + "ignore": ["language"], + }, + } + } + ), + "database": {"session": object()}, + } + + def test_build_screen_bootstrap_extracts_metadata_filters(self): + context = self.make_context() + + bootstrap = screen_support.build_screen_bootstrap(context) + + self.assertIs(context, bootstrap.context) + self.assertEqual({"RECODED_WITH": "FFX"}, bootstrap.signature_tags) + self.assertEqual(["VERSION-eng"], bootstrap.remove_global_keys) + self.assertEqual(["ENCODER"], bootstrap.ignore_global_keys) + self.assertEqual(["BPS"], bootstrap.remove_track_keys) + self.assertEqual(["language"], bootstrap.ignore_track_keys) + + def test_build_screen_controllers_only_creates_requested_instances(self): + context = self.make_context() + + with ( + patch.object(screen_support, "PatternController", side_effect=lambda context: ("pattern", context)), + patch.object(screen_support, "ShowController", side_effect=lambda context: ("show", context)), + patch.object(screen_support, "TmdbController", side_effect=lambda: "tmdb"), + patch.object(screen_support, "ShiftedSeasonController", side_effect=lambda context: ("shifted", context)), + ): + controllers = screen_support.build_screen_controllers( + context, + pattern=True, + show=True, + tmdb=True, + shifted_season=True, + ) + + self.assertEqual( + { + "pattern": ("pattern", context), + "show": ("show", context), + "tmdb": "tmdb", + "shifted_season": ("shifted", context), + }, + controllers, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/configure_workstation.sh b/tools/configure_workstation.sh index 7d302bc..5f86948 100755 --- a/tools/configure_workstation.sh +++ b/tools/configure_workstation.sh @@ -51,6 +51,7 @@ Environment overrides: Notes: - tools/setup.sh is the first installation step and owns bundle venv setup. - This script is the second step and owns system dependencies plus local config. + - After the bundle is installed, the aligned CLI wrapper is: ffx configure_workstation - Python test packages are installed by tools/setup.sh --with-tests, not here. EOF } diff --git a/tools/setup.sh b/tools/setup.sh index 8cc378a..aaa89e5 100755 --- a/tools/setup.sh +++ b/tools/setup.sh @@ -50,6 +50,7 @@ Options: Notes: - This is the first installation step. + - After the bundle is installed, the aligned CLI wrapper is: ffx setup - tools/configure_workstation.sh is the second step and configures system dependencies plus local user files. EOF }