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

PixelateAnnotator throws errors if the area is too small, BlurAnnotator is not censoring if area is too large #703

Open
2 tasks done
Clemens-E opened this issue Dec 30, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@Clemens-E
Copy link

Search before asking

  • I have searched the Supervision issues and found no similar bug report.

Bug

When using PixelateAnnotator with no additional configuration, it throws an error if the area to pixelate is too small:

Traceback (most recent call last):
  File "/home/ultra/clean.py", line 83, in <module>
    annotated_frame = blur_annotator.annotate(
  File "/opt/conda/lib/python3.10/site-packages/supervision/annotators/core.py", line 1253, in annotate
    scaled_up_roi = cv2.resize(
cv2.error: OpenCV(4.8.1) /io/opencv/modules/imgproc/src/resize.cpp:4068: error: (-215:Assertion failed) !dsize.empty() in function 'resize'

I added this debug to this section

    print(f"Box: {(x1, y1, x2, y2)}")
    roi = scene[y1:y2, x1:x2]
    print(f"ROI shape: {roi.shape}")

The last output before the error is:
Box: (644, 444, 678, 453)
ROI shape: (9, 34, 3)
I left the pixel size at the default (10), so I'm guessing the 9 is too small for it
As seen in the below code, I made this adjustment to automatically pick the largest possible pixel size and half it by 2, this works very well.

self.pixel_size = min(y2 - y1, x2 - x1) / 2

roi = scene[y1:y2, x1:x2]


scaled_down_roi = cv2.resize(
    src=roi, dsize=None, fx=self.pixel_size, fy=self.pixel_size
)

This approach would also resolve an issue with the BlurAnnotator.
If the kernel size set to the default, and the area is large, the resulting area is still very identifiable.

Maybe we can resolve this by having the option to provide a lambda instead of a fixed number, so the user can dynamically decide how large the used kernel/pixel size should be.
If that's something worth implementing in the project, I would be happy to create a PR.

Environment

  • Supervision 0.17.0 & 0.17.1
    hardware not relevant

Minimal Reproducible Example

No response

Additional

No response

Are you willing to submit a PR?

  • Yes I'd like to help by submitting a PR!
@Clemens-E Clemens-E added the bug Something isn't working label Dec 30, 2023
@SkalskiP SkalskiP self-assigned this Jan 2, 2024
@SkalskiP
Copy link
Collaborator

SkalskiP commented Jan 2, 2024

Hi, @Clemens-E 👋🏻 ! Thanks a lot for your interest in supervision. Good catch.

I think we can handle this problem as follows:

  • Create two new functions, calculate_dynamic_kernel_size and calculate_dynamic_pixel_size, and place them in supervision/annotators/utils. We already have similar functions: calculate_dynamic_text_scale and calculate_dynamic_line_thickness so this would not be anything new.
  • Let's make kernel_size in BlurAnnotator and pixel_size in PixelateAnnotator optional. If the user provides us with a value, it will be used, but if not, we will use calculate_dynamic_kernel_size and calculate_dynamic_pixel_size to calculate them dynamically.
  • In addition, if the user specifies a pixel_size value but it is too small let us silently fill the whole box with an average color.

@Clemens-E
Copy link
Author

Sounds like a good plan, however this doesn't allow the user to decide what specific size they want to use depending on the detection area.
For most people this is probably fine, so I don't expect this to cater my specific needs

Another option would be to allow passing the kernel/pixel size in the annotate function.

@SkalskiP
Copy link
Collaborator

SkalskiP commented Jan 2, 2024

Like you said, I don't think most people need this level of control. In general, we try to make the API as simple as possible and don't overcomplicate it unless necessary.

Are you interested in implementing this fix?

@Clemens-E
Copy link
Author

Clemens-E commented Jan 2, 2024

I will try doing that, you might have to review multiple times though, my python skills aren't fully enterprise ready 😄
Just not sure about the last point:

In addition, if the user specifies a pixel_size value but it is too small let us silently fill the whole box with an average color.

I would fall back to the dynamic version, but I can try doing an average color

@SkalskiP
Copy link
Collaborator

SkalskiP commented Jan 2, 2024

I will try doing that, you might have to review multiple times though, my python skills aren't fully enterprise ready

No worries. I'm happy to help with my reviews.

I would fall back to the dynamic version, but I can try doing an average color

Problem is that dynamic version will try to update parameters for all boxes :/

DrawingProcess added a commit to DrawingProcess/supervision that referenced this issue Jan 22, 2024
DrawingProcess added a commit to DrawingProcess/supervision that referenced this issue Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

2 participants