Skip to content

camera_frame_stats and camera_signal_quality skip their threshold guards on a camera-less episode, and name internal fields when they do fire #447

Description

@kstonekuan

Held for @Chama2001. I have twice pointed them at an issue and had it taken before they replied, so this one is spoken for. GitHub will not let me assign someone who has not commented on the issue yet, and it fails silently rather than saying so. Chama2001: comment here and I will assign it. If a week passes with no comment, it is open to anyone.

#445 was one instance of a pattern. Here are five more, across two checks, verified on current main.

Both halves, measured

camera_frame_stats (src/hflow/checks.py:567-576) and camera_signal_quality (:1315-1321) hand their thresholds to FrameStatisticsSettings, whose __post_init__ (src/hflow/_video_measurements/_frame_statistics.py:79) does the refusing. That dataclass is constructed inside the per-camera loop.

camera_frame_stats.black_pixel_threshold=-5
    with camera : ValueError: black_pixel_luma_threshold must be between 0 and 255
    no camera   : ACCEPTED

camera_frame_stats.black_frame_amount_pct=900
    with camera : ValueError: black_frame_minimum_pixel_share_percent must be between 0 and 100
    no camera   : ACCEPTED

camera_frame_stats.freeze_min_duration_s=nan
    with camera : ValueError: freeze_minimum_duration_seconds must be finite and positive
    no camera   : ACCEPTED

camera_signal_quality.black_pixel_threshold=999
    with camera : ValueError: black_pixel_luma_threshold must be between 0 and 255
    no camera   : ACCEPTED

camera_signal_quality.freeze_noise_db=nan
    with camera : ValueError: freeze_noise_tolerance_decibels must be finite
    no camera   : ACCEPTED

Two separate defects in that table.

The guard is skipped when the loop body does not run. A state-only episode accepts black_pixel_threshold=-5 and returns a clean result. Register the check over a corpus with a typo in a threshold and every camera-less episode passes while the first episode with a camera crashes. tests/test_gates.py:20-24 builds a camera-less episode, and the how-to tells users to skip joint checks on human egocentric corpora, so mixed corpora are ordinary here.

Every message names a field the caller cannot have typed. You pass black_pixel_threshold and are told about black_pixel_luma_threshold. You pass freeze_noise_db and are told about freeze_noise_tolerance_decibels. Four of the five above rename the parameter in the error. This is the same problem #434 fixed for shake_threshold_dps, which used to report minimum_shake_degrees_per_second.

The pattern to copy

Both fixes already exist in this file. #441 and #446 put camera_stability's three numeric arguments at the top of the function, before selected_cameras is resolved, each refused under its own name. Do the same thing here.

Keep the FrameStatisticsSettings guards where they are. They protect that class's other callers, and they are the reason the bounds are known. You are adding an earlier refusal in the caller's vocabulary, not moving one.

Definition of done

  1. Both functions refuse a bad threshold before selected_cameras is resolved.
  2. Each message names the parameter as the check's signature spells it.
  3. Every case in the table above is covered, and at least one runs against a camera-less episode. A test that only uses a video episode passes today and proves nothing.
  4. bool is rejected for the int-typed thresholds. bool subclasses int, so True otherwise passes as 1.
  5. expected_hz and the other non-numeric arguments are out of scope.
  6. Removing any guard you add must turn a test red. Say which ones you checked.

Worth deciding rather than assuming

The five rows are not the whole surface: bright_luma_threshold, and camera_fps_conformance's max_plausible_fps and downsample_tolerance_fps, are the same shape and I did not verify them. Check them yourself and either include them or say why not. Do not take my list as the boundary of the problem.

Validation

uv sync --locked --all-extras
uv run ruff check
uv run ruff format --check
uv run ty check
uv run pytest -q

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinggood first issueGood for newcomershelp wantedExtra attention is needed

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions