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.

Sign up for GitHub

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

Jump to bottom

Replace TensorBuffer::JPEG with ImageEncoded archetype #3803

Closed
jleibs opened this issue Oct 11, 2023 · 2 comments · Fixed by #6883 or #6884
Closed

Replace TensorBuffer::JPEG with ImageEncoded archetype #3803

jleibs opened this issue Oct 11, 2023 · 2 comments · Fixed by #6883 or #6884
Assignees
@emilk
Labels
🔩 data model enhancement New feature or request
Milestone
0.18 - MHz logging

Comments

@jleibs
Copy link
Member

jleibs commented Oct 11, 2023

Out of expediency, we added a JPEGBuffer to the TensorBuffer enum.

This means that any code working with TensorData needs to potentially support (or at least explicitly detect and not support) JPEG-Encoded data.

This leads to patterns such as wrapper types like https://github.com/rerun-io/rerun/blob/main/crates/re_types/src/tensor_data.rs#L392

The right way to handle this is to make ImageEncoded its own archetype which contains its own buffer type along with proper meta-information about the encoded format.

While it would be nice to handle this generically with "component-data-conversions" this refactor is likely still worthwhile as the image part could explicitly add support for this archetype (as it already handles 3 archetypes as is), and the Tensor space-view won't have to worry about it at all.
@jleibs jleibs added enhancement New feature or request 🔩 data model labels Oct 11, 2023
@jleibs jleibs mentioned this issue Oct 11, 2023
Closed
@Wumpf
Copy link
Member

Wumpf commented Oct 24, 2023

This is also problematic for C++ image logging: Right now you have to go through a pretty complicated path on TensorBuffer and then already know the jpeg size

@Wumpf Wumpf modified the milestones: Triage, 0.11 C++ polish Oct 24, 2023
@Wumpf

This comment was marked as outdated.

Sign in to view
@emilk emilk changed the title Make ImageEncoded an archetype and TensorBuffer:JPEG a separate component Make ImageEncoded an archetype and TensorBuffer::JPEG a separate component Oct 25, 2023
@Wumpf Wumpf modified the milestones: 0.11 C++ polish, Triage Nov 6, 2023
@Wumpf Wumpf mentioned this issue May 20, 2024
Closed
@Wumpf Wumpf removed their assignment Jul 9, 2024
@emilk emilk self-assigned this Jul 10, 2024
@emilk emilk mentioned this issue Jul 10, 2024
Open
1 task
@emilk emilk changed the title Make ImageEncoded an archetype and TensorBuffer::JPEG a separate component Replace TensorBuffer::JPEG with ImageEncoded archetype Jul 10, 2024
@emilk emilk assigned emilk and unassigned emilk Jul 10, 2024
This was referenced Jul 12, 2024
Merged
Merged
Merged
emilk added a commit that referenced this issue Jul 15, 2024
@emilk @Wumpf
[2/4] Add new archetypes.ImageEncoded with PNG and JPEG support (#6874
15db658 
)

### What
* Part of #6844
* Part of #3803

Has `do-no-merge` because of current merge-target.

### PR train
* Prev: #6882
* Next: #6883
* Next: #6884

### Coming in follow-up PRs:
* Fix image picking, hovering, and visualization in spatial view
* Switch to using `ImageEncoded` in all examples
* Remove old code for storing JPEGs in tensors

### Fixes
* Fixes #6520 

### 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)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6874?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6874?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/6874)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.

---------

Co-authored-by: Andreas Reich <[email protected]>
emilk added a commit that referenced this issue Jul 15, 2024
@emilk
[3/4] Introduce ImageChromaDownsampled (#6883)
1229d7c 
### What
* Part of #6844
* Closes #3803

This switches us to use the new `archetype.ImageEncoded` in all
examples.

Logging chroma-downsampled images (NV12/YUY2) is now done with the new
`ImageChromaDownsampled` helper (which will probably go away before
0.18).

### PR train
* Prev: #6882
* Prev: #6874
* Next: #6884

### 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)
* [ ] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6883?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6883?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/6883)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
emilk added a commit that referenced this issue Jul 15, 2024
@emilk
[4/4] Remove TensorBuffer::JPEG, DecodedTensor, `TensorDecodeCach…
8124967 
…e` (#6884)

* Part of #6844
* Closes #3803

⚠️ This breaks any existing `JPEG`-encoded RRDs

### Rust API
* Removed `TensorBuffer::JPEG`
* Removed `TensorData::from_jpeg_bytes`
* Deprecated `Image::from_file_path` and `from_file_contents`

For all of these, use `ImageEncoded` instead.

### PR train
* Prev: #6882
* Prev: #6874
* Prev: #6883

### 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)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6884?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6884?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/6884)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@emilk emilk

Labels
🔩 data model enhancement New feature or request
Projects
None yet
Milestone
0.18 - MHz logging
3 participants
@emilk @Wumpf @jleibs