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

Clemens-E commented 10 months ago

Search before asking

[X] 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?

[X] Yes I'd like to help by submitting a PR!

SkalskiP commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 :/

roboflow / supervision