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

Adjust Roboflow models to primarily use base64 payloads #798

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PawelPeczek-Roboflow
Copy link
Collaborator

Description

Currently, running Roboflow model blocks against hosted API we take numpy_image and serialise it to base64 - usually following the path of decoding input image (which is sent in base64) just to encode it back.

Sometimes it has detrimental effect - valid payloads that are sent to the fromtend API may get decoded into numpy and encoded to JPEG base64 - which explodes the size.

There may be good and bad side-effects of this change:

  • needed to change jpeg_quality in WorkflowImageData to match the behaviour of inference HTTP client (which uses opencv default)
  • all workflow blocks with roboflow models may return different results, yet avoiding double compression should generally help

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • 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?

  • CI still 🟢
  • will be tested e2e on staging once approach agreed

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

  • Docs updated? What were the changes:

@@ -375,7 +375,7 @@ def base64_image(self) -> str:
return self._base64_image
numpy_image = self.numpy_image
self._base64_image = base64.b64encode(
encode_image_to_jpeg_bytes(numpy_image)
encode_image_to_jpeg_bytes(numpy_image, jpeg_quality=95)
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably want to do whatever we do during the export process for training. I think it’s quality 70

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd vote to keep default compression used by opencv, if we introduce higher compression we will have users complaining that inference works differently when they run it locally vs when they make request to our platform..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants