From 609f93b78380dc9ce4e7ddfbde8d7b36c73e3e4d Mon Sep 17 00:00:00 2001 From: Javanaut Date: Sat, 11 Apr 2026 16:30:41 +0200 Subject: [PATCH] Fix cpu percentage interpretations --- requirements/architecture.md | 2 +- requirements/project.md | 2 +- src/ffx/cli.py | 20 +++++++++----- src/ffx/process.py | 41 ++++++++++++++++++++++++----- tests/unit/test_cli_lazy_imports.py | 32 ++++++++++++++++++++++ tests/unit/test_process.py | 12 +++++++-- 6 files changed, 92 insertions(+), 17 deletions(-) diff --git a/requirements/architecture.md b/requirements/architecture.md index e5c86b4..1f0cd17 100644 --- a/requirements/architecture.md +++ b/requirements/architecture.md @@ -42,7 +42,7 @@ - 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. - Integration adapters: - - Process execution wrapper for `ffmpeg`, `ffprobe`, `nice`, and `cpulimit`, with explicit disabled states for niceness and CPU limiting and a combined `cpulimit -- nice -n ... ` execution shape when both limits are configured. + - Process execution wrapper for `ffmpeg`, `ffprobe`, `nice`, and `cpulimit`, with explicit disabled states for niceness and CPU limiting, support for both absolute `cpulimit` values and machine-wide percent input, and a combined `cpulimit -- nice -n ... ` execution shape when both limits are configured. - HTTP adapter for TMDB via `requests`. ## Data And Interface Notes diff --git a/requirements/project.md b/requirements/project.md index 2c38517..9edf200 100644 --- a/requirements/project.md +++ b/requirements/project.md @@ -64,7 +64,7 @@ - The system shall allow CLI overrides for stream languages, stream titles, default and forced tracks, stream order, TMDB show and episode data, output directory, label prefix, and processing resource limits. - Processing resource limit rules: - `--nice` shall accept niceness values from `-20` through `19`; omitting the option shall disable niceness adjustment. - - `--cpu` shall accept CPU limit values from `1` through `99`; omitting the option shall disable CPU limiting. + - `--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`. - 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. diff --git a/src/ffx/cli.py b/src/ffx/cli.py index 4854297..50d6e25 100755 --- a/src/ffx/cli.py +++ b/src/ffx/cli.py @@ -30,6 +30,11 @@ if TYPE_CHECKING: from ffx.track_descriptor import TrackDescriptor LIGHTWEIGHT_COMMANDS = {None, 'version', 'help', '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." +) def normalizeNicenessOption(ctx, param, value): @@ -286,11 +291,11 @@ def getUnmuxSequence(trackDescriptor: TrackDescriptor, sourcePath, targetPrefix, ) @click.option( '--cpu', - type=int, + type=str, default=None, callback=normalizeCpuOption, show_default='disabled', - help='Limit CPU percent of started processes (1..99). Omit to disable; 0 also disables.', + help=CPU_OPTION_HELP, ) def unmux(ctx, paths, @@ -309,6 +314,7 @@ def unmux(ctx, ctx.obj['resource_limits'] = {} ctx.obj['resource_limits']['niceness'] = nice + ctx.obj['resource_limits']['cpu_limit'] = cpu ctx.obj['resource_limits']['cpu_percent'] = cpu for sourcePath in existingSourcePaths: @@ -377,11 +383,11 @@ def unmux(ctx, ) @click.option( '--cpu', - type=int, + type=str, default=None, callback=normalizeCpuOption, show_default='disabled', - help='Limit CPU percent of started processes (1..99). Omit to disable; 0 also disables.', + help=CPU_OPTION_HELP, ) def cropdetect(ctx, paths, @@ -394,6 +400,7 @@ def cropdetect(ctx, ctx.obj['resource_limits'] = {} ctx.obj['resource_limits']['niceness'] = nice + ctx.obj['resource_limits']['cpu_limit'] = cpu ctx.obj['resource_limits']['cpu_percent'] = cpu for sourcePath in existingSourcePaths: @@ -536,11 +543,11 @@ def checkUniqueDispositions(context, mediaDescriptor: MediaDescriptor): ) @click.option( '--cpu', - type=int, + type=str, default=None, callback=normalizeCpuOption, show_default='disabled', - help='Limit CPU percent of started processes (1..99). Omit to disable; 0 also disables.', + help=CPU_OPTION_HELP, ) @click.option('--rename-only', is_flag=True, default=False, help='Only renaming, no recoding') @@ -643,6 +650,7 @@ def convert(ctx, context['resource_limits'] = {} context['resource_limits']['niceness'] = nice + context['resource_limits']['cpu_limit'] = cpu context['resource_limits']['cpu_percent'] = cpu diff --git a/src/ffx/process.py b/src/ffx/process.py index c186f9c..429961c 100644 --- a/src/ffx/process.py +++ b/src/ffx/process.py @@ -1,3 +1,4 @@ +import os import shlex import subprocess from typing import Iterable, List @@ -9,9 +10,9 @@ COMMAND_NOT_FOUND_RETURN_CODE = 127 MIN_NICENESS = -20 MAX_NICENESS = 19 DISABLED_NICENESS_SENTINEL = 99 -MIN_CPU_PERCENT = 1 -MAX_CPU_PERCENT = 99 DISABLED_CPU_PERCENT_SENTINEL = 0 +MIN_CPU_PERCENT = 1 +MAX_CPU_PERCENT = 100 def formatCommandSequence(commandSequence: Iterable[str]) -> str: @@ -35,18 +36,42 @@ def normalizeNiceness(niceness) -> int | None: return niceness +def getPresentCpuCount() -> int: + if hasattr(os, 'sched_getaffinity'): + affinity = os.sched_getaffinity(0) + if affinity: + return len(affinity) + + cpuCount = os.cpu_count() + return cpuCount if cpuCount and cpuCount > 0 else 1 + + def normalizeCpuPercent(cpuPercent) -> int | None: if cpuPercent is None: return None + cpuPercent = str(cpuPercent).strip() + if cpuPercent.endswith('%'): + percentValue = int(cpuPercent[:-1].strip()) + if percentValue == DISABLED_CPU_PERCENT_SENTINEL: + return None + + if percentValue < MIN_CPU_PERCENT or percentValue > MAX_CPU_PERCENT: + raise ValueError( + f"CPU percentage must be between {MIN_CPU_PERCENT}% and {MAX_CPU_PERCENT}%, " + + f"or {DISABLED_CPU_PERCENT_SENTINEL} to disable." + ) + + return percentValue * getPresentCpuCount() + cpuPercent = int(cpuPercent) if cpuPercent == DISABLED_CPU_PERCENT_SENTINEL: return None - if cpuPercent < MIN_CPU_PERCENT or cpuPercent > MAX_CPU_PERCENT: + if cpuPercent < MIN_CPU_PERCENT: raise ValueError( - f"CPU limit must be between {MIN_CPU_PERCENT} and {MAX_CPU_PERCENT}, " - + f"or {DISABLED_CPU_PERCENT_SENTINEL} to disable." + "CPU limit must be a positive absolute value such as 200, " + + f"a percentage such as 25%, or {DISABLED_CPU_PERCENT_SENTINEL} to disable." ) return cpuPercent @@ -55,7 +80,7 @@ def normalizeCpuPercent(cpuPercent) -> int | None: def getWrappedCommandSequence(commandSequence: List[str], context: dict = None) -> List[str]: """ niceness: -20 to 19, disabled when unset - cpu_percent: 1 to 99, disabled when unset + cpu limit: positive absolute cpulimit value, or a machine-wide percentage When both limits are configured, cpulimit wraps a nice-adjusted command: cpulimit -l -- nice -n @@ -63,7 +88,9 @@ def getWrappedCommandSequence(commandSequence: List[str], context: dict = None) resourceLimits = (context or {}).get('resource_limits', {}) niceness = normalizeNiceness(resourceLimits.get('niceness')) - cpu_percent = normalizeCpuPercent(resourceLimits.get('cpu_percent')) + cpu_percent = normalizeCpuPercent( + resourceLimits.get('cpu_limit', resourceLimits.get('cpu_percent')) + ) wrappedCommandSequence = [str(token) for token in commandSequence] if niceness is not None: diff --git a/tests/unit/test_cli_lazy_imports.py b/tests/unit/test_cli_lazy_imports.py index 3707219..36b6553 100644 --- a/tests/unit/test_cli_lazy_imports.py +++ b/tests/unit/test_cli_lazy_imports.py @@ -99,6 +99,38 @@ class CliLazyImportTests(unittest.TestCase): result["modules"], ) + def test_convert_help_describes_absolute_and_percent_cpu_limits(self): + result = self.run_python( + textwrap.dedent( + f""" + import click + import json + import sys + + sys.path.insert(0, {str(SRC_ROOT)!r}) + + import ffx.cli + + help_output = ffx.cli.convert.get_help(click.Context(ffx.cli.convert)) + + print(json.dumps({{ + "output": help_output, + "modules": {{ + module_name: module_name in sys.modules + for module_name in {HEAVY_MODULES!r} + }}, + }})) + """ + ) + ) + + self.assertIn("200", result["output"]) + self.assertIn("25%", result["output"]) + self.assertTrue( + all(not is_loaded for is_loaded in result["modules"].values()), + result["modules"], + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_process.py b/tests/unit/test_process.py index 62ea007..05ef254 100644 --- a/tests/unit/test_process.py +++ b/tests/unit/test_process.py @@ -3,6 +3,7 @@ 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" @@ -69,11 +70,11 @@ class ProcessTests(unittest.TestCase): def test_get_wrapped_command_sequence_wraps_cpulimit_around_nice_when_both_configured(self): wrapped = getWrappedCommandSequence( ["ffmpeg", "-i", "input.mkv"], - context={"resource_limits": {"niceness": 5, "cpu_percent": 42}}, + context={"resource_limits": {"niceness": 5, "cpu_limit": 200}}, ) self.assertEqual( - ["cpulimit", "-l", "42", "--", "nice", "-n", "5", "ffmpeg", "-i", "input.mkv"], + ["cpulimit", "-l", "200", "--", "nice", "-n", "5", "ffmpeg", "-i", "input.mkv"], wrapped, ) @@ -83,6 +84,13 @@ class ProcessTests(unittest.TestCase): def test_normalize_cpu_percent_accepts_disabled_sentinel(self): self.assertIsNone(normalizeCpuPercent(0)) + def test_normalize_cpu_percent_accepts_absolute_cpulimit_values(self): + self.assertEqual(200, normalizeCpuPercent(200)) + + def test_normalize_cpu_percent_converts_percent_of_present_cores(self): + with patch("ffx.process.getPresentCpuCount", return_value=8): + self.assertEqual(200, normalizeCpuPercent("25%")) + if __name__ == "__main__": unittest.main()