Migrate crop config creator node #191
Conversation
aljazkonec1
requested review from
tersekmatija,
jkbmrz,
kkeroo and
klemen1999
as code owners
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #191 +/- ##
=========================================
+ Coverage 0 48.68% +48.68%
=========================================
Files 0 76 +76
Lines 0 4404 +4404
=========================================
+ Hits 0 2144 +2144
- Misses 0 2260 +2260 ☔ View full report in Codecov by Sentry. |
|
@aljazkonec1 can you also adjust the unit test so it could be used in stability tests as well? The adjustments are simple.
This would be then compatible with stability tests and the workflow could take this node also into the stability tests pipeline. You can check if it works by running |
| will keep at most the first 100 detections. | ||
|
|
||
| Before use, the source and target image sizes need to be set with the build function. | ||
| The node assumes the last frame is saved in the dai.ImgManipV2 node and when recieving a detection_message the node sends an empty crop config that skips the frame and loads the next frame in the queue. |
There was a problem hiding this comment.
why do we need this assumption? How can we get rid of it?
Wouldn't it be better if this node also accepted the image and then it could make sure the crops are always correctly created?
There was a problem hiding this comment.
Problem is that this is a host node and is run on host. Recieving a frame from device and then sending n frames back to the device completely fills the network and slows the pipeline considerably. And we cant make this a script node because there are no ImgDetectionsExtended on the devices (yet).
There was a problem hiding this comment.
huh, i see. this is going to be tough to make the host nodes work in both the peripheral and local modes.
I remember there was a ImageManip config, something like wait_for_image and wait_for_config. wait_for_config ment that if the manipulation has an image ready, it has to wait for a manip cfg before processing it. Mabe if this is still the case in depthaiV3 requestion the manipulation to be configured like this would be better dependency then doing this workaround which will sometimes fail?
There was a problem hiding this comment.
cfg.setSkipCurrentImage() does what wait_for_image, so it discards the current image in the ImageManipNode and executes the rest of the config on the new image that arrives.
I did some more testing and I found some unexpected behavior that I think is is the reason behind why I had to add the assumption. The ImageManipV2 node has, by default, .inputConfig.setReusePreviousMessage set to True. There is also an initialConfig that just forwards the first image. This causes the node to use that cofig to send an uncropped image forward before an actual inputConfig is recieved from CropConfigsCreatorNode. This causes an error with sizes which fills up the entire queue. I'll make a report on it to dai team.
| The output link for the ImageManipConfigV2 messages | ||
| detections_output : dai.Output | ||
| The output link for the ImgDetectionsExtended message | ||
| w : int |
There was a problem hiding this comment.
why cannot we work with relative coordinates?
There was a problem hiding this comment.
When there is a rotated rectangle, cropping in relative coordinates creates a sheared img crop in the original coordinates due to the aspect ratio in relative coordinates being 1:1 while in absolute coordinates the aspect ratio can be anything (16:9, 3:2, etc.)
There was a problem hiding this comment.
isn't it a bug in depthai? I don't see why it matters if the bbox is rotated or not.
There was a problem hiding this comment.
Technically not a bug as it does the correct rotation in relative coordinates. Thing is, that in relative coordinates a rotation is not the same as in absolute coordinates. Think if original coordinates are 1000x500 and you rotate the point (0, 1) by 45 deg in relative coordinates, you get (sqrt(2)/2, sqrt(2)/2) in relative, which is (1000 * sqrt(2) /2, 500 *sqrt(2) /2) in absolute coordinates, but that is not 45 deg in the absolute coordinates.
| The height of the source image. | ||
| target_w : int | ||
| The width of the target image. | ||
| target_h : int |
There was a problem hiding this comment.
I don't understand what does this do. I can see it's used here: cfg.setOutputSize(self.target_w, self.target_h)
But I don't understand it's purpose. Is this a command to resize the crop? If so, what if I don't want to resize it?
There was a problem hiding this comment.
Yes, because the size of the detected rectanlge can be anything, this is so that the output of crop node is always the same so the cropped images can be used in a second NN for example.
There was a problem hiding this comment.
Now that you mention it, I doo see that its an assumption that someone will want the same size outputs. I can change it to be an option (eg. equalize_crop_size = True). Thoughts @klemen1999
There was a problem hiding this comment.
In the minemaestro app, we were generating crops based on bboxes and we did not resize, unless the resulting crop was then a threshold. So for me, a general node would have these options:
- crop and keep the resulting size
- crop and set a fixed size. with this, we would need the options to set different resizing options like: stratch or letterbox
- crop and resize if resulting size is bigger then threshold
There was a problem hiding this comment.
Forgot about the 1. option. I'll adopt the node such that all 3. options are covered. Thanks!
Purpose
This PR migrates the
ProcessDetectionsnode, renamed toCropConfigCreatorNode, from depthai-experiments to depthai-nodes. The node simplifies the pipeline by removing one extra script node and expands the usability as it unblocks RVC2 usage. The naming of the node and any other suggestions are kindly welcome!Specification
Add a new node, called CropConfigCreatorNode. This node is used to create a dai.ImageManipConfigV2() object for every detection. The node works on both RVC2 and RVC4. Simultaneously, this node also addresses the issue with RVC2 with pool size that has been blocking depthai-experiments.
Dependencies & Potential Impact
No breaking changes outright, but an update to depthai-experiments will be needed. One risk is the way the node handles syncing of which crop config is for which frame. The node assumes the last frame is saved in the ImgManipV2 node and when recieving a detection_message it sends an empty crop config that skips the frame and loads the next frame in the queue. That frame should be the one from which the detection_message was gotten.
Deployment Plan
None / not applicable
Testing & Validation
Tested on age-gender experiment on this branch on both RVC2 and 4 for 0 to 6 faces present in the frame. No unsynced frames could be detected. PS: pay attention to how the node is used in main.py and how the following ImageManipNode (script_node) has to be set up in order for the correct sequence.