feat: add point history visualization and scrubbing #907
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add toggleable overlay layers to intrinsic calibration view: - Current frame points (red circles) - Accumulated points (teal, semi-transparent) - Selected calibration grids (cyan lines showing coverage) Presenter exposes thread-safe access to collected points and selection result. View owns all rendering with sizes scaled to image dimensions. Cached frame re-rendering avoids jumping to frame 0 on toggle. Refs #869
Root cause: PTS-to-frame-index calculation used int() which truncated incorrectly due to floating point precision (557.8125 -> 557 instead of 558). Changes: - Use round() instead of int() for PTS calculations in FrameSource - Add keyframe indexing at init to find actual last accessible frame - Handle seek failure gracefully (set skip_read=True to prevent death) - Use condition variable instead of polling for responsive pause loop - Implement min(timestamps, source) for last_frame_index safety - Fix button text bug by reordering state clearing in Presenter - Update test_last_frame_index to verify accessibility not metadata Refs #869
Naming cleanup from senior dev review: - FramePacketPublisher → FramePacketStreamer - create_publisher() → create_streamer() - jump_to() → seek_to() - exact parameter → precise parameter - break_on_last → end_behavior: Literal["stop", "pause"] - get_frame_fast() → get_nearest_keyframe() - FrameProcessingThread → FrameRenderThread These names better reflect the components' roles: - "Streamer" emphasizes video streaming over pub/sub - "seek_to" aligns with video player terminology - "precise" is clearer than "exact" for frame accuracy - "end_behavior" as Literal is self-documenting - "get_nearest_keyframe" describes what it actually returns - "FrameRenderThread" clarifies it renders, not processes Refs #869
mprib
deleted the
869-add-point-history-visualization-to-intrinsiccalibrationpresenter
branch
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Point History Visualization
Scrubbing Bug Fix
Root cause: PTS-to-frame-index calculation used
int()which truncated incorrectly due to floating point precision. Fixed withround().Naming Cleanup
FramePacketPublisherFramePacketStreamerjump_to()seek_to()exactparampreciseparambreak_on_lastend_behavior: Literal["stop", "pause"]get_frame_fast()get_nearest_keyframe()FrameProcessingThreadFrameRenderThreadTest plan
Closes #869