Updated MIL tracker implementation #8942
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 WalkthroughThis pull request introduces several enhancements across the CVAT (Computer Vision Annotation Tool) codebase, focusing on improving frame data handling, annotation actions, OpenCV tracking capabilities, and overall code quality. The changes include modifications to core functionality in frame data retrieval, updating modal interactions, introducing a new MIL (Minimum Information Loss) tracker implementation, and refining image processing utilities. The updates aim to provide more flexible and robust annotation and tracking features while maintaining the existing architectural design. Changes
Sequence DiagramsequenceDiagram
participant User
participant AnnotationModal
participant TrackerMIL
participant OpenCVWrapper
User->>AnnotationModal: Open Modal
AnnotationModal->>OpenCVWrapper: Initialize Tracker
OpenCVWrapper->>TrackerMIL: Register MIL Tracker
User->>AnnotationModal: Select Tracking Action
AnnotationModal->>TrackerMIL: Run Tracking
TrackerMIL->>TrackerMIL: Process Frames
TrackerMIL-->>AnnotationModal: Return Tracked Objects
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
cvat-ui/src/utils/opencv-wrapper/annotations-actions/tracker-mil.ts (1)
275-279: Ensure default values are correctly set inparametersThe default value for
'Convert rectangle shapes to tracks'may not reflect the initial state ifthis.#convertRectangleShapesToTrackschanges.Consider using a static default value or initializing
this.#convertRectangleShapesToTracksbefore assigning it as a default:'Convert rectangle shapes to tracks': { type: ActionParameterType.CHECKBOX, values: ['true', 'false'], - defaultValue: String(this.#convertRectangleShapesToTracks), + defaultValue: 'false', },cvat-ui/src/utils/opencv-wrapper/histogram-equalization.ts (1)
13-13: Consistent formatting of type annotationsAdding spaces after colons in type annotations enhances readability and maintains a consistent coding style.
-private cv:any; +private cv: any; -public processImage(src: ImageData, frameNumber: number): ImageData { +public processImage(src: ImageData, frameNumber: number): ImageData {Also applies to: 20-20
cvat-ui/src/utils/opencv-wrapper/opencv-wrapper.ts (1)
128-129: Review the initialization orderThe registration of
TrackerMILActionis added after the initialization flag is set to true. Consider moving the initialization flag setting after all initialization steps are complete to ensure atomic initialization.- this.initialized = true; await core.actions.register(new TrackerMILAction(this)); + this.initialized = true;cvat-ui/src/components/annotation-page/annotations-actions/annotations-actions-modal.tsx (1)
327-335: Add error handling for invalid default actionThe code silently ignores cases where the default action is invalid. Consider adding error notification for better user feedback.
if (defaultAnnotationAction) { const defaultAction = list.find((action) => action.name === defaultAnnotationAction); + if (!defaultAction) { + notification.warning({ + message: 'Invalid default action', + description: `Action "${defaultAnnotationAction}" not found`, + }); + } if ( defaultAction && (!defaultTargetObjectState || defaultAction.isApplicableForObject(defaultTargetObjectState)) ) { dispatch(reducerActions.setActiveAnnotationsAction(defaultAction)); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
cvat-core/src/frames.ts(3 hunks)cvat-ui/src/components/annotation-page/annotations-actions/annotations-actions-modal.tsx(5 hunks)cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsx(10 hunks)cvat-ui/src/components/annotation-page/standard-workspace/styles.scss(2 hunks)cvat-ui/src/containers/annotation-page/standard-workspace/objects-side-bar/object-item.tsx(2 hunks)cvat-ui/src/containers/annotation-page/standard-workspace/objects-side-bar/objects-list.tsx(2 hunks)cvat-ui/src/utils/opencv-wrapper/annotations-actions/tracker-mil.ts(1 hunks)cvat-ui/src/utils/opencv-wrapper/histogram-equalization.ts(3 hunks)cvat-ui/src/utils/opencv-wrapper/opencv-wrapper.ts(4 hunks)cvat-ui/src/utils/opencv-wrapper/tracker-mil.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- cvat-ui/src/containers/annotation-page/standard-workspace/objects-side-bar/object-item.tsx
- cvat-ui/src/components/annotation-page/standard-workspace/styles.scss
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: testing
- GitHub Check: generate_github_pages
🔇 Additional comments (16)
cvat-ui/src/utils/opencv-wrapper/annotations-actions/tracker-mil.ts (2)
223-234: Handle deletion of shapes cautiouslyWhen deleting shapes, ensure that only the intended shapes are removed. Currently, all rectangle shapes are deleted if
this.#convertRectangleShapesToTracksis true, which might affect shapes not involved in tracking.Please verify that this deletion logic does not unintentionally remove unrelated rectangle shapes. If necessary, adjust the deletion criteria to target only the converted shapes.
1-297: Overall implementation is well-structuredThe
OpenCVTrackerMILclass is thoughtfully designed, with clear logic and appropriate asynchronous handling. The integration with OpenCV's MIL tracker is well-executed, and the code follows best practices.cvat-ui/src/utils/opencv-wrapper/histogram-equalization.ts (2)
51-52: Improved error handling in catch blockRefining the error handling by specifying the error type as
unknownand rethrowing using type checking ensures that unexpected errors are properly propagated.
54-61: Enhanced readability in thefinallyblockAdding braces around the
ifstatements in thefinallyblock improves code clarity and maintainability.cvat-ui/src/utils/opencv-wrapper/tracker-mil.ts (4)
16-17: IntroducedmaxSizeandimageScalepropertiesAdding
maxSizeandimageScaleproperties helps manage large images by scaling them appropriately, improving performance and preventing potential issues with high-resolution images.
28-59: ImplementedoptResizemethod for image scalingThe
optResizemethod effectively resizes images that exceedmaxSize, maintaining the aspect ratio. This enhances performance and resource management when processing large images.
115-120: Added resolution consistency check inupdatemethodEnsuring that the current image has the same resolution as the initial image enhances robustness by preventing errors due to mismatched image sizes.
61-96: Adjusted bounding box coordinates based on scaling factorModifying the bounding box coordinates to account for image scaling ensures accurate tracking results after resizing, which is crucial for the tracker’s precision.
cvat-ui/src/utils/opencv-wrapper/opencv-wrapper.ts (1)
314-314: LGTM: Improved description clarityThe updated description better reflects the nature of the tracking functionality.
cvat-ui/src/containers/annotation-page/standard-workspace/objects-side-bar/objects-list.tsx (1)
602-602: LGTM: Improved API usageThe update to use an object parameter for
openAnnotationsActionModalimproves code clarity and maintainability by making the parameter purpose explicit.cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsx (3)
48-48: LGTM: Improved type safetyReplacing
anywith the properJobtype improves type safety and code maintainability.
417-418: LGTM: Enhanced user guidanceAdded help message improves user experience by clearly explaining the tracker's functionality.
447-452: Verify tracker initializationThe code opens the annotations action modal with the tracker name but doesn't verify if the tracker is properly initialized.
cvat-ui/src/components/annotation-page/annotations-actions/annotations-actions-modal.tsx (2)
305-305: LGTM: Enhanced modal configurationAdded
defaultAnnotationActionprop improves the modal's flexibility by allowing pre-selection of actions.Also applies to: 309-309
635-641: LGTM: Improved function signatureThe updated
openAnnotationsActionModalsignature with optional named parameters improves code readability and maintainability.cvat-core/src/frames.ts (1)
303-307: LGTM! Enhanced type safety in the return type.The updated method signature provides better type safety and clearer separation of concerns by explicitly returning render dimensions alongside the image data.
Let's verify the impact on API consumers:
✅ Verification successful
Verified: The changes to FrameData.data() are backward compatible
The codebase analysis shows that existing consumers already handle the structured return type with
imageDataproperty, and the addition ofrenderWidthandrenderHeightproperties is non-breaking. All current usages in tracker-mil.ts, canvas-wrapper.tsx, and other files will continue to work as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct calls to FrameData.data() that might need updates rg -g '*.{ts,tsx}' -A 2 'data\(\s*\).*\b(ImageBitmap|Blob)\b'Length of output: 63
Script:
#!/bin/bash # Search for calls to data() method and frame-related patterns rg -g '*.{ts,tsx}' -B 2 -A 2 '\.data\(\s*[^)]*\)' # Search for frame-related type definitions and interfaces ast-grep --pattern 'interface $_ { $$$ data($$$): $$$ $$$ }'Length of output: 4170
| this.#targetFrame = +parameters['Target frame']; | ||
| this.#convertRectangleShapesToTracks = parameters['Convert rectangle shapes to tracks'] === 'true'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter validation in the init method
The init method assigns values from the parameters object without validation. If the expected parameters are missing or invalid, this could lead to runtime errors.
Consider adding validation to ensure that 'Target frame' and 'Convert rectangle shapes to tracks' exist and are of the correct type:
public async init(instance: Job | Task, parameters: Record<string, string>): Promise<void> {
this.#instance = instance;
+ if (!parameters['Target frame'] || isNaN(Number(parameters['Target frame']))) {
+ throw new Error('Invalid or missing parameter: Target frame must be a valid number');
+ }
this.#targetFrame = +parameters['Target frame'];
+ if (parameters['Convert rectangle shapes to tracks'] !== 'true' && parameters['Convert rectangle shapes to tracks'] !== 'false') {
+ throw new Error('Invalid or missing parameter: Convert rectangle shapes to tracks must be "true" or "false"');
+ }
this.#convertRectangleShapesToTracks = parameters['Convert rectangle shapes to tracks'] === 'true';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.#targetFrame = +parameters['Target frame']; | |
| this.#convertRectangleShapesToTracks = parameters['Convert rectangle shapes to tracks'] === 'true'; | |
| public async init(instance: Job | Task, parameters: Record<string, string>): Promise<void> { | |
| this.#instance = instance; | |
| if (!parameters['Target frame'] || isNaN(Number(parameters['Target frame']))) { | |
| throw new Error('Invalid or missing parameter: Target frame must be a valid number'); | |
| } | |
| this.#targetFrame = +parameters['Target frame']; | |
| if (parameters['Convert rectangle shapes to tracks'] !== 'true' && parameters['Convert rectangle shapes to tracks'] !== 'false') { | |
| throw new Error('Invalid or missing parameter: Convert rectangle shapes to tracks must be "true" or "false"'); | |
| } | |
| this.#convertRectangleShapesToTracks = parameters['Convert rectangle shapes to tracks'] === 'true'; | |
| } |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8942 +/- ##
===========================================
- Coverage 73.86% 73.82% -0.04%
===========================================
Files 416 417 +1
Lines 44413 44578 +165
Branches 3993 4025 +32
===========================================
+ Hits 32804 32909 +105
- Misses 11609 11669 +60
|
|
Motivation and context
Resolved #8894
The patch updates approach we use to run OpenCV MIL tracker.
Now it is run through annotations actions. It allows us to get more memory control, fix some issues related to non-consequent frames in GT jobs and track object on many frames. Tracking in general works faster.
However it removes user's ability to adjust objects during tracking.
How has this been tested?
Checklist
developbranch(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
Bug Fixes
Refactor
Style