-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ignore deleted frames when converting tracks to shapes #8834
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes primarily focus on enhancing frame deletion handling across the CVAT dataset management system. The modifications introduce a new Changes
Sequence DiagramsequenceDiagram
participant User
participant AnnotationManager
participant TrackManager
User->>AnnotationManager: to_shapes(end_frame, deleted_frames)
AnnotationManager-->>User: Filtered shapes excluding deleted frames
User->>TrackManager: to_shapes(end_frame, deleted_frames)
TrackManager-->>User: Filtered shapes excluding deleted frames
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cvat/apps/dataset_manager/tests/test_formats.py (1)
534-589
: LGTM: Comprehensive test for deleted frame handlingThe test effectively verifies that keyframes on deleted frames don't affect interpolation. Consider adding edge cases:
- Multiple consecutive deleted frames
- Deleted frames at track boundaries
def test_track_keyframes_on_multiple_deleted_frames_do_not_affect_later_frames(self): images = self._generate_task_images(5) task = self._generate_task(images) job = self._get_task_jobs(task["id"])[0] annotations = { "version": 0, "tags": [], "shapes": [], "tracks": [{ "frame": 0, "label_id": task["labels"][0]["id"], "group": None, "source": "manual", "attributes": [], "shapes": [ { "frame": 0, "points": [1, 2, 3, 4], "type": "rectangle", "occluded": False, "outside": False, "attributes": [], }, { "frame": 1, "points": [5, 6, 7, 8], "type": "rectangle", "occluded": False, "outside": False, "attributes": [], }, { "frame": 2, "points": [9, 10, 11, 12], "type": "rectangle", "occluded": False, "outside": False, "attributes": [], }, { "frame": 4, "points": [13, 14, 15, 16], "type": "rectangle", "occluded": False, "outside": False, "attributes": [], } ] }] } self._put_api_v2_job_id_annotations(job["id"], annotations) # Delete two consecutive frames self._delete_job_frames(job["id"], [1, 2]) task_ann = TaskAnnotation(task["id"]) task_ann.init_from_db() task_data = TaskData(task_ann.ir_data, Task.objects.get(pk=task["id"])) extractor = CvatTaskOrJobDataExtractor(task_data) dm_dataset = Dataset.from_extractors(extractor) # Verify frame 4 is not affected assert len(dm_dataset.get("image_4").annotations) == 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cvat/apps/dataset_manager/annotation.py
(3 hunks)cvat/apps/dataset_manager/bindings.py
(5 hunks)cvat/apps/dataset_manager/tests/test_formats.py
(4 hunks)
🔇 Additional comments (4)
cvat/apps/dataset_manager/tests/test_formats.py (1)
525-533
: LGTM: Well-structured helper method for frame deletion
The method is focused, uses appropriate HTTP semantics, and includes error checking.
cvat/apps/dataset_manager/annotation.py (2)
202-222
: LGTM: Clean addition of deleted frames filtering
The change is minimal, focused, and maintains backward compatibility.
470-482
: LGTM: Efficient implementation of shape filtering
The implementation efficiently filters out shapes from deleted frames while preserving the track structure.
cvat/apps/dataset_manager/bindings.py (1)
323-323
: LGTM: Improved data structure for deleted frames
Changed from dict to set for better performance in membership testing operations.
f91055c
to
11eacbc
Compare
91431e1
to
4535f5d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8834 +/- ##
===========================================
- Coverage 73.93% 73.91% -0.02%
===========================================
Files 409 409
Lines 43930 43932 +2
Branches 3986 3986
===========================================
- Hits 32478 32471 -7
- Misses 11452 11461 +9
|
b506ae7
to
6bdee91
Compare
6bdee91
to
eb3ce13
Compare
Quality Gate passedIssues Measures |
@bsekachev, do you know if the previous behavior about the interpolation on deleted frames was intentional? |
Motivation and context
When a frame which contains a keyframe of a track is deleted, the keyframe continues to exist. When a dataset is exported (in all formats except CVAT for video), tracks are interpolated and keyframe from deleted frame is not ignored.
Fixing it.
How has this been tested?
Checklist
develop
branch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
AnnotationManager
andTrackManager
.Bug Fixes
Tests