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

fix/708: fixes issue with processing .png images with large file sizes. #754

Merged
merged 3 commits into from
Apr 2, 2024

Conversation

Sidsector9
Copy link
Member

@Sidsector9 Sidsector9 commented Apr 1, 2024

Description of the Change

This PR:

  • sets the image scanning timeout to 60 seconds.
  • adds the filter hook 'classifai_ms_computer_vision_scan_image_timeout'

Closes #708

How to test the Change

This can be tested with the following sample images:

5mb-png-image
10mb-png-image

Changelog Entry

Added - filter hook 'classifai_ms_computer_vision_scan_image_timeout' to increase image processing timeout.
Fixed - timeout issue with processing .png images of large file sizes.

Credits

Props @QAharshalkadu @Sidsector9

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@Sidsector9 Sidsector9 requested review from dkotter, jeffpaul and a team as code owners April 1, 2024 14:18
@Sidsector9 Sidsector9 self-assigned this Apr 1, 2024
@github-actions github-actions bot added this to the 3.1.0 milestone Apr 1, 2024
@github-actions github-actions bot added the needs:code-review This requires code review. label Apr 1, 2024
Copy link
Collaborator

@dkotter dkotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor change request on the documentation side.

But also a question that may need looked at further. According to the API docs, the max image supported is 4MB. We have some code in place to return an error if the image size is larger than that so in theory, a 5MB image shouldn't even work, though it sounds like that's not the case.

Some time should be spent here to figure out how images larger than 4MB are making it through and making any needed adjustments. I know we check the file size in the rest_endpoint_callback method but maybe that check needs added other places as well?

includes/Classifai/Providers/Azure/ComputerVision.php Outdated Show resolved Hide resolved
@Sidsector9
Copy link
Member Author

According to the API docs, the max image supported is 4MB. We have some code in place to return an error if the image size is larger than that so in theory, a 5MB image shouldn't even work, though it sounds like that's not the case.

@dkotter what you're saying is correct and the check we have is working as expected. When we upload an image size > 4MB, the check selects the largest image variant that is < 4MB to process.

This particular issue is a weird one pertaining to just the .png format. From what I am understanding, the AI service takes more time to process a .png compared to a .jpeg. So, even with a .png file which has a size of 3.5MB, it still fails.

This works well when the timeout is increased.

@Sidsector9 Sidsector9 requested a review from dkotter April 2, 2024 12:55
@dkotter dkotter merged commit 35c6e4b into develop Apr 2, 2024
13 checks passed
@dkotter dkotter deleted the fix/708 branch April 2, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:code-review This requires code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image processing > When 5 MB png type image is uploaded and selected for processing then getting errors
2 participants