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 jpeg_quality parameter to log_image #2418

Merged
merged 20 commits into from Jun 14, 2023
Merged

Conversation

emilk
Copy link
Member

@emilk emilk commented Jun 13, 2023

Closes #2175

What

You can now do log_image("image", image, jpeg_quality=75) to compress your logged images so they take up less space on disk and in RAM.

I also updated the UI so you can actually tell whether or not a tensor was compressed.

This makes a many of our example data lot smaller. For instance, tracking_hf_opencv goes from 753 MB to 101MB.

Checklist

PR Build Summary: https://build.rerun.io/pr/2418

Docs preview: https://rerun.io/preview/194b4be/docs
Examples preview: https://rerun.io/preview/194b4be/examples

@emilk emilk added 🐍 Python API Python logging API 📉 performance Optimization, memory use, etc labels Jun 13, 2023
@abey79
Copy link
Contributor

abey79 commented Jun 13, 2023

OpenCV isn't the nicest dependency to have. For example, sometime one must use opencv-python-headless when it interferes with some UI toolkits, etc. Could we consider alternative package, e.g. Pillow?

@emilk
Copy link
Member Author

emilk commented Jun 13, 2023

Good idea - pillow we can probably depend on directly too; it is a pretty light dependency. Let's see if it supports jpeg compression…

EDIT: it did, and it works great!

@emilk emilk changed the title Add jpeg_quality parameter to log_image, requiring cv2 Add jpeg_quality parameter to log_image Jun 13, 2023
@emilk emilk mentioned this pull request Jun 13, 2023
1 task
@nikolausWest
Copy link
Member

Pillow is supposed to be pretty slow. Did you test speed at all? simplejpeg looks like it might be a better fit.

@abey79
Copy link
Contributor

abey79 commented Jun 13, 2023

Pillow is supposed to be pretty slow. Did you test speed at all? simplejpeg looks like it might be a better fit.

Unfortunately, they dont back their bold (and indeed tempting) claims with proper binaries for Apple silicon. I tried to install the package, it installed from source, and failed :(

Edit: it looks like next release will fix this: https://gitlab.com/jfolz/simplejpeg/-/issues/7

@emilk
Copy link
Member Author

emilk commented Jun 14, 2023

Yes, this costs the user a bit of CPU at log-time. We could alleviate this by throwing the jpeg encoding in a background thread and wait for that thread on shutdown, but I don't want to do such a risky change right now. I've warned the user about the CPU cost in the docstring.

@emilk emilk requested a review from nikolausWest June 14, 2023 06:44
@abey79
Copy link
Contributor

abey79 commented Jun 14, 2023

.github/workflows/reusable_build_and_test_wheels.yml needs fixing. I share @jleibs's perplexity around that part btw (line 242):

      - name: Install wheel dependencies
        if: needs.set-config.outputs.RUN_TESTS == 'true'
        # First we install the dependencies manually so we can use `--no-index` when installing the wheel.
        # This needs to be a separate step for some reason or the following step fails
        # TODO(jleibs): pull these deps from pyproject.toml
        # TODO(jleibs): understand why deps can't be installed in the same step as the wheel
        shell: bash
        run: |
          pip install deprecated numpy>=1.23 pyarrow==10.0.1 pytest==7.1.2

      - name: Install built wheel
        if: needs.set-config.outputs.RUN_TESTS == 'true'
        # Now install the wheel using a specific version and --no-index to guarantee we get the version from
        # the pre-dist folder. Note we don't use --force-reinstall here because --no-index means it wouldn't
        # find the dependencies to reinstall them.
        shell: bash
        run: |
          pip uninstall rerun-sdk
          pip install rerun-sdk==${{ steps.expected_version.outputs.EXPECTED_VERSION }} --no-index --find-links dist

Couldn't we just do that?

pip uninstall rerun-idk
pip install local/path/to/dist/rerun_xxx.whl

@emilk emilk removed the request for review from nikolausWest June 14, 2023 07:57
@@ -90,6 +102,23 @@ def log_image(
if interpretable_as_image and num_non_empty_dims != len(shape):
image = np.squeeze(image)

if jpeg_quality is not None:
# TODO(emilk): encode JPEG in background thread instead
Copy link
Member

Choose a reason for hiding this comment

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

it might be nice to do this in rust instead in the future to make it more cross-platform and to introduce less python dependencies

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree

@emilk emilk merged commit bf3725b into main Jun 14, 2023
20 checks passed
@emilk emilk deleted the emilk/log_image-jpeg-encode branch June 14, 2023 09:15
Comment on lines 161 to +163
img = cv2.imread(str(image_file))
img = cv2.resize(img, resize)
jpeg_quality = [int(cv2.IMWRITE_JPEG_QUALITY), 75]
_, encimg = cv2.imencode(".jpg", img, jpeg_quality)
rr.log_image_file("camera/image", img_bytes=encimg)
rr.log_image("camera/image", img, jpeg_quality=75)
Copy link
Member

Choose a reason for hiding this comment

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

This is a BGR vs RGB bug

@emilk emilk mentioned this pull request Jun 14, 2023
2 tasks
emilk added a commit that referenced this pull request Jun 14, 2023
### What
Bug introduced in #2418

It's back to being red:

![image](https://github.com/rerun-io/rerun/assets/1148717/9ce4ab6b-a782-4b0d-9f75-2c905bf3313e)


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2430

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/df06d4c/docs
Examples preview: https://rerun.io/preview/df06d4c/examples
<!-- pr-link-docs:end -->
abey79 added a commit that referenced this pull request Jun 14, 2023
### What
Updates the titles, and adds tags to all "python examples with real
data" (a separate PR is needed for the "fake data" examples).

Renames:

| before | after |
| --- | --- |
| arkitscenes | arkit_scenes |
| colmap | structure_from_motion |
| deep_sdf | signed_distance_fields |
| dicom | dicom_mri |
| mp_pose | human_pose_tracking |
| nyud | rgbd |
| opencv_canny | live_camera_edge_detection |
| ros | ros_node |
| segment_anything | segment_anything_model |
| stable_diffusion | depth_guided_stable_diffusion |
| tracking_hf_opencv | detect_and_track_objects |

In order to minimise merge issues, this is best updated and merged
AFTER:
- #2419
- #2420
- #2418
- #2424
- #2426
- #2360 

TODO

- [x] Update names script names
- [x] Update any references to the scripts
- [ ] ~~Mirror updates for Rust examples~~ (none of the renamed examples
have rust counterparts)
- [x] Convert tags to list
- [x] Quick typo/grammar pass with pycharm's checker

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [ ] I've included a screenshot or gif (if applicable)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2416

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/3fb07ee/docs
Examples preview: https://rerun.io/preview/3fb07ee/examples
<!-- pr-link-docs:end -->

---------

Co-authored-by: Antoine Beyeler <antoine@rerun.io>
emilk added a commit that referenced this pull request Jun 15, 2023
Closes #2175

You can now do `log_image("image", image, jpeg_quality=75)` to compress
your logged images so they take up less space on disk and in RAM.

I also updated the UI so you can actually tell whether or not a tensor
was compressed.

This makes a many of our example data lot smaller. For instance,
`tracking_hf_opencv` goes from 753 MB to 101MB.

* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2418

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/194b4be/docs
Examples preview: https://rerun.io/preview/194b4be/examples
<!-- pr-link-docs:end -->
emilk added a commit that referenced this pull request Jun 15, 2023
### What
Bug introduced in #2418

It's back to being red:

![image](https://github.com/rerun-io/rerun/assets/1148717/9ce4ab6b-a782-4b0d-9f75-2c905bf3313e)


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2430

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/df06d4c/docs
Examples preview: https://rerun.io/preview/df06d4c/examples
<!-- pr-link-docs:end -->
emilk pushed a commit that referenced this pull request Jun 15, 2023
Updates the titles, and adds tags to all "python examples with real
data" (a separate PR is needed for the "fake data" examples).

Renames:

| before | after |
| --- | --- |
| arkitscenes | arkit_scenes |
| colmap | structure_from_motion |
| deep_sdf | signed_distance_fields |
| dicom | dicom_mri |
| mp_pose | human_pose_tracking |
| nyud | rgbd |
| opencv_canny | live_camera_edge_detection |
| ros | ros_node |
| segment_anything | segment_anything_model |
| stable_diffusion | depth_guided_stable_diffusion |
| tracking_hf_opencv | detect_and_track_objects |

In order to minimise merge issues, this is best updated and merged
AFTER:
- #2419
- #2420
- #2418
- #2424
- #2426
- #2360

TODO

- [x] Update names script names
- [x] Update any references to the scripts
- [ ] ~~Mirror updates for Rust examples~~ (none of the renamed examples
have rust counterparts)
- [x] Convert tags to list
- [x] Quick typo/grammar pass with pycharm's checker

* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [ ] I've included a screenshot or gif (if applicable)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2416

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/3fb07ee/docs
Examples preview: https://rerun.io/preview/3fb07ee/examples
<!-- pr-link-docs:end -->

---------

Co-authored-by: Antoine Beyeler <antoine@rerun.io>
This was referenced Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📉 performance Optimization, memory use, etc 🐍 Python API Python logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optional jpeg-encoding of images
3 participants