From 1acab5f91891aeb7aeb72b5b2678d90f09366e62 Mon Sep 17 00:00:00 2001 From: "user.email" Date: Fri, 20 Mar 2026 23:07:56 +0530 Subject: [PATCH 1/6] Fix fps truncation, load_video buffer guard, remove np.int patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove np.int monkey-patch — dtcwt 0.14.0 doesn't need it. Keep fps as float instead of truncating with int() (3.2% error on 29.97fps). Add frame_count guard in load_video to prevent buffer overflow. Guard flattop_filter_1d against zero window size. Fixes #2 --- motion_mag.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/motion_mag.py b/motion_mag.py index 77d47c2..5faa229 100644 --- a/motion_mag.py +++ b/motion_mag.py @@ -23,10 +23,6 @@ import numpy as np from scipy import ndimage, signal -# NumPy compatibility: dtcwt uses np.int which was removed in NumPy 1.24+ -if not hasattr(np, 'int'): - np.int = np.int64 - import dtcwt @@ -117,7 +113,7 @@ def flattop_filter_1d(data, width, axis=0, mode='reflect'): Returns: Filtered numpy array with same shape as input. """ - window_size = round(width / 0.2327) + window_size = max(1, round(width / 0.2327)) window = signal.windows.flattop(window_size) window = window / np.sum(window) return ndimage.convolve1d(data, window, axis=axis, mode=mode) @@ -143,13 +139,15 @@ def load_video(path): frame_count = int(cap.get(cv2.CAP_PROP_FRAME_COUNT)) width = int(cap.get(cv2.CAP_PROP_FRAME_WIDTH)) height = int(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)) - fps = int(cap.get(cv2.CAP_PROP_FPS)) + fps = cap.get(cv2.CAP_PROP_FPS) # Read all frames into a single array, then split channels frames = np.zeros((frame_count, height, width, 3), dtype=np.uint8) i = 0 t_start = time.time() while cap.isOpened(): + if i >= frame_count: + break ret, frame = cap.read() if not ret: break From 1b3bcad482ff86b1857ea4e1da250546ab11617b Mon Sep 17 00:00:00 2001 From: "user.email" Date: Fri, 20 Mar 2026 23:08:13 +0530 Subject: [PATCH 2/6] Add VERSION file and Docker version labels Single source of truth for version in VERSION file. Build script reads from it, tags image, and passes version as Docker build arg. Fixes #3 --- Dockerfile | 3 +++ VERSION | 1 + docker-build.sh | 8 ++++++-- 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 VERSION diff --git a/Dockerfile b/Dockerfile index de1f75b..2ac6885 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,6 +18,9 @@ RUN pip install --no-cache-dir -r requirements.txt COPY motion_mag.py . +ARG VERSION +LABEL version=${VERSION} + USER ${UNAME} ENTRYPOINT ["python", "-u", "motion_mag.py"] diff --git a/VERSION b/VERSION new file mode 100644 index 0000000..3eefcb9 --- /dev/null +++ b/VERSION @@ -0,0 +1 @@ +1.0.0 diff --git a/docker-build.sh b/docker-build.sh index f98a20b..d08e3da 100755 --- a/docker-build.sh +++ b/docker-build.sh @@ -1,10 +1,14 @@ #!/bin/bash set -e +VERSION=$(cat VERSION) + docker build \ --build-arg UID="$(id -u)" \ --build-arg GID="$(id -g)" \ --build-arg UNAME="$(whoami)" \ - -t motion-magnification-dtcwt . + --build-arg VERSION="${VERSION}" \ + -t motion-mag-dtcwt:${VERSION} \ + -t motion-mag-dtcwt:latest . -echo "Built motion-magnification-dtcwt image as user: $(whoami) (uid=$(id -u), gid=$(id -g))" +echo "Built motion-mag-dtcwt:${VERSION} (also tagged :latest)" From aad552ff12beb0798098e6946bb9dea623637be0 Mon Sep 17 00:00:00 2001 From: "user.email" Date: Fri, 20 Mar 2026 23:10:20 +0530 Subject: [PATCH 3/6] Add pytest infrastructure, test.sh, and dev deps in Docker Add requirements-dev.txt (pytest, ruff), tests directory with initial format_duration tests. Dockerfile installs dev deps and copies tests. test.sh builds image if needed, runs lint + tests inside Docker. Fix ruff F541 lint error. Fixes #4 --- .gitignore | 1 + Dockerfile | 7 +++++-- motion_mag.py | 2 +- requirements-dev.txt | 2 ++ test.sh | 25 +++++++++++++++++++++++++ tests/__init__.py | 0 tests/test_motion_mag.py | 21 +++++++++++++++++++++ 7 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 requirements-dev.txt create mode 100755 test.sh create mode 100644 tests/__init__.py create mode 100644 tests/test_motion_mag.py diff --git a/.gitignore b/.gitignore index 1782f26..9511839 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ __pycache__/ *.py[cod] *.egg-info/ +.pytest_cache/ # Output videos *.avi diff --git a/Dockerfile b/Dockerfile index 2ac6885..2945e32 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,10 +13,13 @@ RUN groupadd -g ${GID} ${UNAME} && \ WORKDIR /app -COPY requirements.txt . -RUN pip install --no-cache-dir -r requirements.txt +COPY requirements.txt requirements-dev.txt ./ +RUN pip install --no-cache-dir -r requirements.txt -r requirements-dev.txt COPY motion_mag.py . +COPY tests/ tests/ + +RUN chown -R ${UID}:${GID} /app ARG VERSION LABEL version=${VERSION} diff --git a/motion_mag.py b/motion_mag.py index 5faa229..ecf535a 100644 --- a/motion_mag.py +++ b/motion_mag.py @@ -360,7 +360,7 @@ def main(): print(f" {frame_count} frames, {frame_size[0]}x{frame_size[1]}, {fps} fps") # --- Parameters --- - print(f"\nParameters:") + print("\nParameters:") print(f" Magnification: {args.magnification}x") print(f" Filter width: {args.width}") print(f" DTCWT levels: {args.nlevels}\n") diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..f643047 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,2 @@ +pytest>=7.0,<9 +ruff>=0.4.0,<1 diff --git a/test.sh b/test.sh new file mode 100755 index 0000000..dd6fc84 --- /dev/null +++ b/test.sh @@ -0,0 +1,25 @@ +#!/bin/bash +set -e + +IMAGE="motion-mag-dtcwt-dev" + +# Build image if it doesn't exist or --build flag passed +if [[ "$1" == "--build" ]] || ! docker image inspect ${IMAGE} &>/dev/null; then + echo "Building test image..." + docker build \ + --build-arg UID="$(id -u)" \ + --build-arg GID="$(id -g)" \ + --build-arg UNAME="$(whoami)" \ + -t ${IMAGE} . + echo "" +fi + +echo "=== Lint ===" +docker run --rm --entrypoint "" ${IMAGE} ruff check . + +echo "" +echo "=== Tests ===" +docker run --rm --entrypoint "" ${IMAGE} python -m pytest tests/ -v + +echo "" +echo "All checks passed." diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_motion_mag.py b/tests/test_motion_mag.py new file mode 100644 index 0000000..93fe053 --- /dev/null +++ b/tests/test_motion_mag.py @@ -0,0 +1,21 @@ +"""Unit tests for motion_mag.py — Phase-Based Motion Magnification.""" + +import sys +import os + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +import motion_mag + + +class TestFormatDuration: + def test_seconds_only(self): + assert motion_mag.format_duration(30.0) == "30.0s" + + def test_minutes_and_seconds(self): + assert motion_mag.format_duration(90.5) == "1m 30.5s" + + def test_zero(self): + assert motion_mag.format_duration(0) == "0.0s" + + def test_exactly_60(self): + assert motion_mag.format_duration(60.0) == "1m 0.0s" From ca0be6ed75409acc2e904bbed92de61f6aa43899 Mon Sep 17 00:00:00 2001 From: "user.email" Date: Fri, 20 Mar 2026 23:11:26 +0530 Subject: [PATCH 4/6] Add core unit tests and input validation tests Tier 1: normalize_phase (unit magnitude, zero safety, phase preservation). Tier 2: flattop_filter (DC passthrough, smoothing, small width guard), extract_temporal_phases (constant phase, output shape). Tier 3: magnify_motions smoke tests (shape, dtype, finite values). Buffer guard test with mocked VideoCapture. Input validation: all CLI error paths. Fixes #5, fixes #6 --- tests/test_motion_mag.py | 219 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 219 insertions(+) diff --git a/tests/test_motion_mag.py b/tests/test_motion_mag.py index 93fe053..cac8e72 100644 --- a/tests/test_motion_mag.py +++ b/tests/test_motion_mag.py @@ -1,12 +1,23 @@ """Unit tests for motion_mag.py — Phase-Based Motion Magnification.""" +import subprocess import sys import os +from unittest.mock import MagicMock, patch + +import cv2 +import dtcwt +import numpy as np +import pytest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import motion_mag +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + class TestFormatDuration: def test_seconds_only(self): assert motion_mag.format_duration(30.0) == "30.0s" @@ -19,3 +30,211 @@ def test_zero(self): def test_exactly_60(self): assert motion_mag.format_duration(60.0) == "1m 0.0s" + + +# --------------------------------------------------------------------------- +# Tier 1: Strict tolerance +# --------------------------------------------------------------------------- + +class TestNormalizePhase: + """normalize_phase should return unit-magnitude complex numbers.""" + + def test_unit_magnitude(self): + rng = np.random.RandomState(42) + x = rng.randn(100) + 1j * rng.randn(100) + result = motion_mag.normalize_phase(x) + magnitudes = np.abs(result) + np.testing.assert_allclose(magnitudes, 1.0, atol=1e-10) + + def test_preserves_phase_angle(self): + x = np.array([1 + 1j, -1 + 0j, 0 + 1j], dtype=np.complex128) + result = motion_mag.normalize_phase(x) + np.testing.assert_allclose(np.angle(result), np.angle(x), atol=1e-10) + + def test_zero_magnitude_safety(self): + """Near-zero elements should be returned as-is (not NaN/Inf).""" + x = np.array([1e-25 + 1e-25j, 0 + 0j, 1 + 1j], dtype=np.complex128) + result = motion_mag.normalize_phase(x) + assert np.all(np.isfinite(result)) + # Third element should be unit magnitude + assert abs(abs(result[2]) - 1.0) < 1e-10 + + def test_already_unit_magnitude(self): + x = np.exp(1j * np.array([0, np.pi / 4, np.pi / 2, np.pi])) + result = motion_mag.normalize_phase(x) + np.testing.assert_allclose(result, x, atol=1e-10) + + +# --------------------------------------------------------------------------- +# Tier 2: Moderate tolerance +# --------------------------------------------------------------------------- + +class TestFlattopFilter: + """flattop_filter_1d should smooth data along the time axis.""" + + def test_dc_passthrough(self): + """A constant signal should pass through unchanged.""" + data = np.ones((100, 4), dtype=np.float64) * 5.0 + filtered = motion_mag.flattop_filter_1d(data, width=20, axis=0) + np.testing.assert_allclose(filtered, 5.0, atol=1e-6) + + def test_smoothing_reduces_variance(self): + """Filtering should reduce the variance of noisy data.""" + rng = np.random.RandomState(42) + data = rng.randn(200, 10) + filtered = motion_mag.flattop_filter_1d(data, width=20, axis=0) + assert np.var(filtered) < np.var(data) + + def test_output_shape_preserved(self): + data = np.random.rand(50, 8).astype(np.float64) + filtered = motion_mag.flattop_filter_1d(data, width=10, axis=0) + assert filtered.shape == data.shape + + def test_small_width_no_crash(self): + """Very small width should not crash (window_size guard).""" + data = np.random.rand(20, 4).astype(np.float64) + filtered = motion_mag.flattop_filter_1d(data, width=0.01, axis=0) + assert filtered.shape == data.shape + assert np.all(np.isfinite(filtered)) + + +class TestExtractTemporalPhases: + """extract_temporal_phases on known pyramids.""" + + def test_constant_phase_gives_linear_cumsum(self): + """If all frames have the same coefficients, cumulative phase + should be approximately constant (frame 0 angle repeated).""" + transform = dtcwt.Transform2d() + # Create identical frames + frame = np.random.RandomState(42).rand(32, 32).astype(np.float64) + pyramids = [transform.forward(frame, nlevels=3) for _ in range(10)] + + phases = motion_mag.extract_temporal_phases(pyramids, level=1) + + assert phases.shape[0] == 10 + # Frame-to-frame deltas should be ~0, so cumsum should be ~constant + # (close to frame 0 angle at each position) + for i in range(1, 10): + np.testing.assert_allclose(phases[i], phases[0], atol=1e-10) + + def test_output_shape(self): + transform = dtcwt.Transform2d() + frame = np.random.rand(16, 16).astype(np.float64) + pyramids = [transform.forward(frame, nlevels=2) for _ in range(5)] + num_coeffs = pyramids[0].highpasses[0].size + + phases = motion_mag.extract_temporal_phases(pyramids, level=0) + assert phases.shape == (5, num_coeffs) + assert phases.dtype == np.float64 + + +# --------------------------------------------------------------------------- +# Tier 3: Smoke tests +# --------------------------------------------------------------------------- + +class TestMagnifyMotions: + """Smoke test magnify_motions on tiny synthetic data.""" + + def test_output_shape_and_dtype(self): + rng = np.random.RandomState(42) + data = rng.rand(10, 32, 32).astype(np.float64) + result = motion_mag.magnify_motions(data, magnification=2.0, width=5, nlevels=2) + assert result.shape == data.shape + assert result.dtype == np.float64 + + def test_values_finite(self): + rng = np.random.RandomState(42) + data = rng.rand(5, 16, 16).astype(np.float64) + result = motion_mag.magnify_motions(data, magnification=2.0, width=3, nlevels=2) + assert np.all(np.isfinite(result)) + + def test_magnification_one_near_identity(self): + """With magnification=1.0, output should be close to input + (no amplification of phase deviations).""" + rng = np.random.RandomState(42) + data = rng.rand(10, 32, 32).astype(np.float64) + result = motion_mag.magnify_motions(data, magnification=1.0, width=5, nlevels=2) + # Not exact due to DTCWT roundtrip + filtering, but should be close + error = np.mean(np.abs(result - data)) + assert error < 10.0 # generous bound — just checking it's not garbage + + +# --------------------------------------------------------------------------- +# Bug fix: load_video buffer guard +# --------------------------------------------------------------------------- + +class TestLoadVideoBufferGuard: + def test_frame_count_too_low(self): + actual_frames = 10 + reported_count = 5 + h, w = 8, 8 + fake_frames = [np.zeros((h, w, 3), dtype=np.uint8) for _ in range(actual_frames)] + call_idx = [0] + + mock_cap = MagicMock() + mock_cap.get.side_effect = lambda prop: { + cv2.CAP_PROP_FRAME_COUNT: reported_count, + cv2.CAP_PROP_FRAME_WIDTH: w, + cv2.CAP_PROP_FRAME_HEIGHT: h, + cv2.CAP_PROP_FPS: 30.0, + }[prop] + mock_cap.isOpened.return_value = True + + def mock_read(): + if call_idx[0] < actual_frames: + frame = fake_frames[call_idx[0]] + call_idx[0] += 1 + return True, frame + return False, None + + mock_cap.read.side_effect = mock_read + + with patch("cv2.VideoCapture", return_value=mock_cap): + channels, fps, frame_size = motion_mag.load_video("fake.mp4") + + assert channels[0].shape[0] == reported_count + assert fps == 30.0 + + +# --------------------------------------------------------------------------- +# Input validation tests +# --------------------------------------------------------------------------- + +SCRIPT = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "motion_mag.py") + + +def run_cli(*args): + result = subprocess.run( + [sys.executable, SCRIPT] + list(args), + capture_output=True, text=True + ) + return result.returncode, result.stderr + + +@pytest.fixture +def dummy_video(tmp_path): + p = tmp_path / "dummy.mp4" + p.write_bytes(b"\x00" * 100) + return str(p) + + +class TestInputValidation: + def test_nonexistent_input_file(self): + code, stderr = run_cli("-i", "nonexistent.mp4") + assert code == 1 + assert "not found" in stderr + + def test_magnification_zero(self, dummy_video): + code, stderr = run_cli("-i", dummy_video, "-k", "0") + assert code == 1 + assert "--magnification must be positive" in stderr + + def test_width_zero(self, dummy_video): + code, stderr = run_cli("-i", dummy_video, "-w", "0") + assert code == 1 + assert "--width must be positive" in stderr + + def test_nlevels_zero(self, dummy_video): + code, stderr = run_cli("-i", dummy_video, "--nlevels", "0") + assert code == 1 + assert "--nlevels must be at least 1" in stderr From d23802a6403beb4e9326fe950eaafd5606d6ea44 Mon Sep 17 00:00:00 2001 From: "user.email" Date: Fri, 20 Mar 2026 23:11:47 +0530 Subject: [PATCH 5/6] Modernize CI: Docker-based with ruff linting Run all CI steps inside Docker matching dev workflow. Add ruff lint step. Update actions/checkout to v6. Remove actions/setup-python (not needed with Docker). Fixes #7 --- .github/workflows/ci.yml | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7ddb85e..a7986ff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,26 +10,34 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - - uses: actions/setup-python@v5 - with: - python-version: "3.11" + - name: Build Docker image + run: | + docker build \ + --build-arg UID=$(id -u) \ + --build-arg GID=$(id -g) \ + --build-arg UNAME=$(whoami) \ + -t dtcwt-ci . - - name: Install dependencies - run: pip install -r requirements.txt + - name: Lint + run: docker run --rm --entrypoint "" dtcwt-ci ruff check . - name: Verify import - run: python -c "import motion_mag" + run: docker run --rm --entrypoint "" dtcwt-ci python -c "import motion_mag" - name: Verify --help - run: python motion_mag.py --help + run: docker run --rm --entrypoint "" dtcwt-ci python motion_mag.py --help - name: Verify --version - run: python motion_mag.py --version + run: docker run --rm --entrypoint "" dtcwt-ci python motion_mag.py --version - name: Run full pipeline on face.mp4 - run: python motion_mag.py -i face.mp4 -o face_magnified.avi -k 3 -w 80 --nlevels 4 + run: | + docker run --rm --entrypoint "" \ + -v "${{ github.workspace }}:/data" \ + dtcwt-ci \ + python -u motion_mag.py -i /data/face.mp4 -o /data/face_magnified.avi -k 3 -w 80 --nlevels 4 - name: Verify output exists run: test -f face_magnified.avi From d313c6ef42745cd6de364e8630677ad2321fb64d Mon Sep 17 00:00:00 2001 From: "user.email" Date: Fri, 20 Mar 2026 23:14:06 +0530 Subject: [PATCH 6/6] Add changelog, design doc, pin dtcwt, and document dev workflow Create CHANGELOG.md (Keep a Changelog, starting fresh). Commit design doc covering architecture decisions and hardening plan. Pin dtcwt upper bound to <1. Add Development section to README (testing, versioning, project structure). Update CONTRIBUTING.md with test.sh instructions. Fixes #8 --- CHANGELOG.md | 34 +++++++++ CONTRIBUTING.md | 7 +- README.md | 64 ++++++++++++++++ docs/design/dtcwt-hardening.md | 131 +++++++++++++++++++++++++++++++++ requirements.txt | 2 +- 5 files changed, 234 insertions(+), 4 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 docs/design/dtcwt-hardening.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..be60b19 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,34 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/), +and this project adheres to [Semantic Versioning](https://semver.org/). + +## [Unreleased] + +### Added +- Unit tests for core functions (normalize_phase, flattop_filter, extract_temporal_phases, magnify_motions) +- Input validation tests for all CLI error paths +- `VERSION` file as single source of truth for versioning +- Docker image version labels and tags +- `CHANGELOG.md` +- `requirements-dev.txt` for dev dependencies (pytest, ruff) +- `test.sh` for running lint and tests inside Docker +- Design doc (`docs/design/dtcwt-hardening.md`) +- Development section in README (testing, versioning, project structure) + +### Fixed +- `load_video` buffer overflow when `CAP_PROP_FRAME_COUNT` underreports +- `fps` truncation: keep as float instead of casting to int (3.2% speed error on 29.97fps videos) +- `flattop_filter_1d` window size guard against zero +- Ruff F541 lint error (extraneous f-prefix) + +### Changed +- CI modernized: runs inside Docker with ruff linting, updated to actions/checkout v6 +- Dev dependencies (pytest, ruff) baked into Docker image +- Build script reads version from `VERSION` file, tags image accordingly +- `dtcwt` dependency pinned with upper bound (`<1`) + +### Removed +- `np.int` monkey-patch — no longer needed with dtcwt 0.14.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cf4dd6d..6a91c63 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,11 +7,12 @@ Thanks for your interest in contributing! 1. **Open an issue first** — describe the bug or feature you'd like to work on. 2. **Fork the repo** and create a branch from `main`. 3. **Keep PRs small** — one logical change per pull request. -4. **Follow PEP 8** for Python code style. -5. **Test your changes** — run the CLI on `face.mp4` to verify nothing is broken: +4. **Follow PEP 8** for Python code style. We use [ruff](https://docs.astral.sh/ruff/) for linting. +5. **Test your changes** before opening a PR: ```bash - python motion_mag.py -i face.mp4 -o test_output.avi -k 3 -w 80 + ./test.sh ``` + Tests run inside Docker — no local Python dependencies needed. See [Development](README.md#development) in the README for details. 6. **Open a pull request** against `main` with a clear description of your changes. ## Reporting bugs diff --git a/README.md b/README.md index a5f4b92..001c1f7 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,10 @@ Phase-based motion magnification amplifies subtle motions invisible to the naked - [CLI Tool](#cli-tool) - [Notebook](#notebook) - [Tips](#tips) +- [Development](#development) + - [Running Tests](#running-tests) + - [Versioning](#versioning) + - [Project Structure](#project-structure) - [References](#references) --- @@ -213,6 +217,66 @@ Open the notebook and run all cells. By default, it downloads a sample face vide --- +## Development + +### Running Tests + +All tests run inside Docker — no local Python dependencies needed: + +```bash +# Run lint + unit tests (builds image automatically if not found) +./test.sh + +# Force rebuild before testing +./test.sh --build +``` + +**Tests** (`tests/test_motion_mag.py`) cover: +- Phase normalization (unit magnitude, zero safety) +- Flat-top temporal filter (DC passthrough, smoothing, edge cases) +- Temporal phase extraction (constant phase, output shape) +- `magnify_motions` smoke tests (shape, dtype, finite values) +- `load_video` buffer safety +- All CLI input validation error paths + +**Dev workflow:** +1. Make your changes +2. Run `./test.sh` +3. If all tests pass, commit and open a PR +4. CI runs lint + smoke tests automatically + +### Versioning + +Version is tracked in a `VERSION` file at the project root. `motion_mag.py` has `__version__` baked into the source (updated at release time). + +**To cut a release:** +1. Update `VERSION` with the new version number +2. Update `__version__` in `motion_mag.py` +3. Update `CHANGELOG.md` — move items from `[Unreleased]` to `[X.Y.Z] - YYYY-MM-DD` +4. Commit: `Release vX.Y.Z` +5. Tag: `git tag -a vX.Y.Z -m "Release vX.Y.Z"` +6. Push: `git push && git push origin vX.Y.Z` +7. Rebuild Docker image: `./docker-build.sh` + +### Project Structure + +``` +motion_mag.py # CLI tool +MotionMagDtcwt.ipynb # Jupyter notebook +Dockerfile # Docker image +docker-build.sh # Build + tag image +test.sh # Run lint + unit tests (Docker) +requirements.txt # Runtime dependencies +requirements-dev.txt # Dev dependencies (pytest, ruff) +tests/ + test_motion_mag.py # Unit tests +docs/design/ # Architecture decision records +VERSION # Single source of truth for version +CHANGELOG.md # Release history +``` + +--- + ## References 1. Wadhwa, N., Rubinstein, M., Durand, F., & Freeman, W.T. (2013). [Phase-Based Video Motion Processing](https://people.csail.mit.edu/nwadhwa/phase-video/). *ACM Transactions on Graphics (SIGGRAPH)*, 32(4). diff --git a/docs/design/dtcwt-hardening.md b/docs/design/dtcwt-hardening.md new file mode 100644 index 0000000..296aa8b --- /dev/null +++ b/docs/design/dtcwt-hardening.md @@ -0,0 +1,131 @@ +# Design Doc: DTCWT Motion Magnification — Project Hardening + +**Status: APPROVED** + +## Context + +The Motion Magnification Using 2D DTCWT project is a Python implementation of phase-based motion magnification (Anfinogentov & Nakariakov, 2016) using the Dual-Tree Complex Wavelet Transform. It's a single-file CLI tool (`motion_mag.py`) shipped via Docker. + +A code review identified the same gaps as the sibling EVM project: no unit tests, no versioning infrastructure, no design documentation, and minor bugs. This design doc covers retroactive architecture decisions and the hardening plan. + +## Goals and Non-Goals + +**Goals:** +- Add unit tests for core functions +- Fix bugs: `load_video` buffer overflow, `fps` truncation, remove dead `np.int` monkey-patch +- Establish versioning infrastructure (VERSION file, Docker labels, changelog) +- Modernize CI (Docker-based, add linting) +- Add `test.sh`, `requirements-dev.txt`, and dev workflow documentation +- Pin `dtcwt` upper bound +- Document architecture decisions + +**Non-Goals:** +- Refactoring into a pip-installable package +- Integration tests with video output comparison +- Regenerating demo GIFs (deferred to after GPU implementation) + +## Proposed Design + +### A. Architecture Decisions (retroactive) + +**Why 2D DTCWT over complex steerable pyramids?** +The original Wadhwa et al. (SIGGRAPH 2013) phase-based method uses complex steerable pyramids, which require custom filter bank implementations. The 2D DTCWT (Selesnick et al., 2005) provides similar properties — near shift-invariance, directional selectivity (6 orientations per level) — with an off-the-shelf Python library (`dtcwt`). Anfinogentov & Nakariakov (2016) demonstrated this substitution works for motion magnification with comparable quality. The tradeoff: DTCWT has fixed 6 orientations per level vs steerable pyramids' configurable orientation count, but 6 is sufficient for general video motion. + +**Why flat-top window for temporal filtering?** +The flat-top window (scipy.signal.windows.flattop) has a very flat passband, meaning signals within the passband are preserved with minimal amplitude distortion. For motion magnification, this matters because amplitude distortion in the temporal filter would non-uniformly scale phase changes, creating visible artifacts. The tradeoff: flat-top has wider transition band (slower rolloff) than alternatives like Hann or Hamming, so frequency selectivity is lower. For the typical use case (separating slow base motion from subtle detail motion), this is acceptable. + +**Why frame-to-frame complex division for phase extraction?** +Phase could be extracted by computing `np.angle()` of each frame's coefficients directly, then differencing. Instead, the code computes `curr / prev` (complex division) and takes `np.angle()` of the ratio. This is more numerically stable because: (1) it avoids phase wrapping issues at ±π boundaries (the ratio's angle is always the correct small delta), (2) it works correctly even when individual coefficients have very small magnitude (the ratio's angle is still meaningful). The cumulative sum of these deltas gives the total phase evolution. + +**Why float64 throughout?** +Verified: dtcwt produces `complex128` highpass coefficients (float64 real/imaginary pairs). Float64 roundtrip error is `4.4e-16` vs `3.6e-07` for float32. Phase extraction via cumulative complex division compounds errors across frames — with 300+ frames and 8 pyramid levels, float32 errors could become visible. Memory cost is 2x, but correctness is more important for a research tool. + +### B. Hardening Changes + +All follow the same patterns established in EVM hardening: + +**Bug fixes:** +- Remove `np.int` monkey-patch — verified dtcwt 0.14.0 doesn't need it (numpy 1.26.4, no `np.int` references in dtcwt source) +- Fix `fps` truncation: `int(cap.get(...))` → keep as float (verified `cv2.VideoWriter` accepts float fps, 3.2% speed error on 29.97fps videos) +- Add `load_video` buffer guard: `if i >= frame_count: break` +- `flattop_filter_1d`: guard against zero window size with `max(1, ...)` + +**Versioning:** +- Add `VERSION` file (Approach B — build-time injection, same as EVM) +- Stamp `__version__` in `motion_mag.py` at release time +- `docker-build.sh` reads from `VERSION`, tags image, passes build arg for Docker label +- Dockerfile gets `ARG VERSION` + `LABEL version=${VERSION}` + +**Testing:** +- `requirements-dev.txt` with pytest and ruff +- `tests/test_motion_mag.py` — unit tests: + - Tier 1 (strict): `normalize_phase` (unit magnitude property, zero-magnitude safety), `format_duration` + - Tier 2 (moderate): `flattop_filter_1d` (DC passthrough, smoothing effect), `extract_temporal_phases` (known constant phase → zero delta) + - Tier 3 (smoke): `magnify_motions` on tiny synthetic data (shapes, dtypes, finite values) + - Input validation error paths +- `test.sh` — build once, run lint + tests in Docker +- CI modernized: Docker-based, ruff lint, smoke tests only + +**CI modernization:** +- Run inside Docker (matching dev workflow and EVM pattern) +- Add ruff linting +- Update `actions/checkout` to v6 +- Remove `actions/setup-python` (not needed when running in Docker) + +**Documentation:** +- `CHANGELOG.md` (Keep a Changelog, starting fresh) +- Design doc (this document) +- `dtcwt` upper bound pinned (`<1`) +- README updated with Development section (testing, versioning, project structure) +- CONTRIBUTING.md updated with `test.sh` instructions + +### C. Future: GPU Acceleration (deferred, implement after hardening) + +**Approach**: Same hybrid pattern as the Visual-Mic project (`joeljose/Visual-Mic`): +- CPU path: existing `dtcwt` library (unchanged) +- GPU path: `pytorch_wavelets` (`DTCWTForward`) with PyTorch CUDA backend +- `--gpu` flag switches between them, `--batch-size` controls GPU memory usage + +**Key considerations for motion magnification vs Visual-Mic:** +- Visual-Mic does single-pass phase extraction (streaming) — motion magnification needs all pyramids in memory for temporal filtering +- GPU batching applies to forward/inverse DTCWT transforms (the expensive part) +- Temporal filtering (flat-top convolution on phase arrays) stays on CPU/numpy — it's already fast and operates on extracted phase angles, not full wavelet coefficients +- `pytorch_wavelets` uses float32 internally — phase extraction precision may differ from CPU float64 path. Needs validation. + +**Dependencies**: `pytorch_wavelets` (installed from `git+https://github.com/fbcotter/pytorch_wavelets.git`), PyTorch with CUDA, `Dockerfile.gpu` based on PyTorch CUDA image. + +**After GPU implementation**: Regenerate demo GIFs using the GPU path for faster processing. + +## Alternatives Considered + +### Temporal filter: Flat-top vs ideal bandpass (FFT-based, as in EVM) + +| Approach | Pros | Cons | Verdict | +|----------|------|------|---------| +| Flat-top window convolution | Minimal amplitude distortion in passband, simple | Wider transition band, less frequency selectivity | **Chosen** — amplitude fidelity matters for phase-based method | +| Ideal bandpass via FFT | Sharp frequency cutoff, matches EVM | Gibbs ringing, requires all frames for FFT | Rejected — ringing introduces phase artifacts | + +### Phase extraction: Complex division vs direct angle subtraction + +| Approach | Pros | Cons | Verdict | +|----------|------|------|---------| +| Complex division + cumsum | No phase wrapping, numerically stable | Slightly more computation | **Chosen** — correctness | +| Direct `angle()` subtraction | Simpler code | Phase wrapping at ±π, needs unwrapping | Rejected — unwrapping adds complexity and failure modes | + +### GPU framework: pytorch_wavelets vs custom CuPy DTCWT + +| Approach | Pros | Cons | Verdict | +|----------|------|------|---------| +| pytorch_wavelets | Proven in Visual-Mic, maintained library, batched transforms | PyTorch is a large dependency, float32 only | **Chosen** (deferred) — minimal effort, proven approach | +| Custom CuPy DTCWT | No PyTorch dependency, could use float64 | Massive implementation effort, no existing library | Rejected — not worth writing from scratch | + +## Tradeoffs and Risks + +- **No GPU acceleration (current)**: DTCWT is CPU-bound and slow for large videos. Accepted for now, GPU path planned as next phase. +- **Memory intensive**: All frame pyramids stay in memory for temporal filtering. A 1080p 30s video at float64 needs ~42 GB for channels plus pyramid storage. Documented, not fixed. +- **Removing `np.int` patch**: If someone uses an older dtcwt (<0.14.0), it will break. Mitigated by pinning `dtcwt>=0.12.0,<1`. +- **GPU float32 vs CPU float64**: When GPU path is added, outputs may differ slightly from CPU. Will need validation and documentation. + +## Open Questions + +None for the hardening phase. GPU acceleration details to be resolved during that implementation. diff --git a/requirements.txt b/requirements.txt index f2bd0f4..64880da 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ scipy>=1.7.1,<2 numpy>=1.20.3,<2 -dtcwt>=0.12.0 +dtcwt>=0.12.0,<1 opencv-python>=4.7.0,<5