Skip to content

docs: document camera_stability tuning knobs - #437

Open
Machumerre wants to merge 1 commit into
Hebbian-Robotics:mainfrom
Machumerre:patch-1
Open

docs: document camera_stability tuning knobs#437
Machumerre wants to merge 1 commit into
Hebbian-Robotics:mainfrom
Machumerre:patch-1

Conversation

@Machumerre

Copy link
Copy Markdown

What it does

  • Adds a new subsection in docs/how-to/enable-built-in-checks.md that explains the two optional arguments introduced in src/hflow/checks.py:948-949:
    • shake_threshold_dps – the angular velocity threshold (degrees per second) that defines a “shake”.
    • unstable_min_duration_s – the minimum duration (seconds) a camera must exceed the threshold to be considered unstable.
  • Provides a short example of how to use these knobs when enabling the check.
  • Updates the table of built-in checks to include the new parameters, mirroring the style used for topics=, nominal_fps=, and dimension_scales=.

Why it matters

  • Users who run camera_stability on hand-held footage will no longer see the entire episode marked as unstable.
  • The documentation now matches the code, preventing confusion and making the check useful for real-world recordings.

Summary

Why

Validation

Checklist

  • I added or updated outcome-focused tests for changed business logic.
  • I updated documentation for changed behavior, flags, formats, or requirements.
  • I ran uv run ruff check --fix, uv run ruff format, and uv run ty check.
  • I ran the relevant pytest suite.
  • I did not add recordings, generated media, credentials, private URLs, or runtime artifacts.
  • I preserved stored-data compatibility or documented an explicit version change.

### What it does
* Adds a new subsection in `docs/how-to/enable-built-in-checks.md` that explains the two optional arguments introduced in `src/hflow/checks.py:948-949`:
  * `shake_threshold_dps` – the angular velocity threshold (degrees per second) that defines a “shake”.
  * `unstable_min_duration_s` – the minimum duration (seconds) a camera must exceed the threshold to be considered unstable.
* Provides a short example of how to use these knobs when enabling the check.
* Updates the table of built-in checks to include the new parameters, mirroring the style used for `topics=`, `nominal_fps=`, and `dimension_scales=`.

### Why it matters
* Users who run `camera_stability` on hand-held footage will no longer see the entire episode marked as unstable.
* The documentation now matches the code, preventing confusion and making the check useful for real-world recordings.
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

👋 Hi @Machumerre — thank you so much for your first contribution to HFlow!

A maintainer will review your pull request as soon as possible. In the meantime:

💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game.

We are excited to have you here and appreciate your help making the project better! 🙌

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up, and welcome. The instinct to give the knobs a table and a worked example is right. Three things need fixing before it can go in, and one of them would actively mislead a reader, so I would rather be specific than vague.

The example calls an API that does not exist. There is no CameraStability class anywhere in the package, and hflow/__init__.py exports no camera name at all. camera_stability is a plain function imported from hflow.checks and registered like every other check in this file. The working version, in the idiom the page already uses at :84-99:

import functools

from hflow.checks import camera_stability

# Instead of `app.check(version="1")(camera_stability)`:
app.check(version="1")(
    functools.partial(camera_stability, shake_threshold_dps=3.0, unstable_min_duration_s=0.25)
)

Worth pausing on: a docs PR is the one place a plausible-looking snippet costs the most, because nothing runs it. If you have not run a line you are about to document, grep for the name first.

The code fence is never closed. The file now ends inside an open ```.

Both descriptions are close but wrong in ways that matter.

shake_threshold_dps is not "when angular velocity exceeds this value". A frame pair counts as unstable when its shake rate beats its deliberate rate and clears a floor. Total angular velocity is not the quantity, and this threshold is not the only condition: it raises the instrument's own resolution floor of one pixel per frame. Panning the camera fast is high angular velocity and perfectly stable.

unstable_min_duration_s does not change what "is reported as unstable". It filters intervals only. unstable_share and unstable_s still count every pair the rule caught, so a user who raises this and watches the measurements will see nothing move and conclude it does not work. That asymmetry is the single most useful sentence the section can contain, and it is the one the issue asked for by name.

Missing the reason anyone needs these. Right now the section says the knobs exist. #436 asks for the situation that motivates them: hand-held footage clears the resolution floor almost continuously by fractions of a degree, which is real motion but not what a person calls a shaky camera, so the check reports nearly the whole episode as unstable and looks broken. That is what a reader arrives with. Compare the dimension_scales= paragraph at :180-186, which spends its words on why the obvious alternative is wrong rather than on restating the signature.

Two smaller things: the section landed after "See also", which closes the page, so it belongs beside the existing camera_stability paragraph at :172-177; and the file uses straight quotes throughout, not curly.

Happy to look again as soon as you push.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants