[go: nahoru, domu]

Skip to content
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

Add sv.CropAnnotator which annotates a cropped part of detections #888

Merged
merged 18 commits into from
Mar 14, 2024
Merged

Add sv.CropAnnotator which annotates a cropped part of detections #888

merged 18 commits into from
Mar 14, 2024

Conversation

xaristeidou
Copy link
Contributor
@xaristeidou xaristeidou commented Feb 12, 2024

Description

This is an implementation of sv.CropAnnotator. Crops a part for each detection in sv.Detections based on the selected sv.Position.

The following methods have been developed:

  • annotate: annotates the image scene using the methods below.
  • apply_zoom: utilized cv2.resize() to apply zoom of the cropped part.
  • calculate_place_coordinates_and_crop: if/else statements for each sv.Position. Returns x,y pixels that are the top left pixel for all cases of position, and trims any exceeding part of the cropped images from detections.
  • place_cropped_scene: places the cropped part to the calculated position x,y up to cropped part shape dimensions.

List any dependencies that are required for this change.

None

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

Tested using YOLOv8 detection and applied on video sample of walking people.

Any specific deployment considerations

Although the calculate_place_coordinates_and_crop method is wide enough, execution time is pretty fast.
Noticed some delay (3ms) when using very high value of zoom_factor, which I think due to cv2.resize() delay.

Docs

  • Docs updated? Not updated

@xaristeidou
Copy link
Contributor Author
xaristeidou commented Feb 12, 2024

I struggled with the method that calculates the final x,y coordinates to placement and applies a trim to exceeding part of the image, that has to apply for all positions. Maybe you can refine a bit if you can to make it shorter. Nevertheless, execution time is pretty good.

It would be nice if you could add a feature to use different anchor coordinates from another detections. For example, if we have two YOLO models to detect cars, and the second to detect plates, to have the ability to place the cropped plates in anchor positions of cars.

Also we could use the filtering of sv.Detections to distinguish class predictions from the same model.

These two methods will have a significant problem two cluster two objects from different class together, i.e. a person and a handbag, and not assign it to different once. Also the difference in number of prediction from both classes because sometimes a person may not carry a bag or may not be predicted.

Maybe we could try that as an independent scenario in another issue.

@SkalskiP
Copy link
Collaborator

Hi @xaristeidou 👋🏻 Thanks for opening the PR 🙏🏻

Yeah I saw calculate_place_coordinates_and_crop implementation. It is pretty long. I'll do my best to optimize it.

@xaristeidou
Copy link
Contributor Author

@SkalskiP The problem is that in each position a lot of checks have to be done to avoid numpy broadcasting errors, and these are not the same for all cases of position. I tried first to make it work for all cases and then would try to optimize. If I can achieve any optimization I will let you know.

@onuralpszr
Copy link
Collaborator

Hello @xaristeidou 👋

First of all before reading code first thing caught my eyes is your commit messages needs clean up I personally don't like to see that "as" or "asdf" as commit message, it has no value and no meaning please rebase/squash those or I can do in the end or I can just simply squash merge because we also want to see meaningful commit messages

@xaristeidou
Copy link
Contributor Author

@onuralpszr Holy!! I wanted to test first the whole annotator and had a thought to re-write the commits as single separate for each method, but totally forgot that because that was days ago when I started it! I was in a hurry yesterday to finish and submit due to new version release! I am very very sorry for that, I don't like that also.

I would be grateful if you could squash them for me because I haven't done it before and I don't want to mesh it up until I search a bit an test how to!!

Thanks, and sorry again.

@onuralpszr
Copy link
Collaborator

@onuralpszr Holy!! I wanted to test first the whole annotator and had a thought to re-write the commits as single separate for each method, but totally forgot that because that was days ago when I started it! I was in a hurry yesterday to finish and submit due to new version release! I am very very sorry for that, I don't like that also.

I would be grateful if you could squash them for me because I haven't done it before and I don't want to mesh it up until I search a bit an test how to!!

Thanks, and sorry again.

It is okay just do what you need to, I can handle when it is done (squash or fix commits)

@SkalskiP
Copy link
Collaborator

Awesome! I'll try to help out with refactoring. I probably won't be able to do it until Friday. I now have some Roboflow duties with a higher priority than Supervision. :/

@SkalskiP
Copy link
Collaborator

Hi @xaristeidou 👋🏻, I've made all the appropriate changes. I'm merging your PR.

@SkalskiP SkalskiP merged commit d669619 into roboflow:develop Mar 14, 2024
9 checks passed
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.

None yet

3 participants