Skip to content

Introduce ImageChromaDownsampled#6883

Merged
emilk merged 7 commits intomainfrom
emilk/use-image-encoded
Jul 15, 2024
Merged

Introduce ImageChromaDownsampled#6883
emilk merged 7 commits intomainfrom
emilk/use-image-encoded

Conversation

@emilk
Copy link
Copy Markdown
Member

@emilk emilk commented Jul 14, 2024

What

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

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

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

@emilk emilk added examples Issues relating to the Rerun examples exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jul 14, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 14, 2024

Deployed docs

Commit Link
b54fbbd https://landing-mgpkiyw1c-rerun.vercel.app/docs

@emilk emilk force-pushed the emilk/use-image-encoded branch 2 times, most recently from c919adb to 9c17ccc Compare July 14, 2024 14:33
@emilk emilk added sdk-python Python logging API include in changelog and removed exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jul 14, 2024
@emilk emilk changed the title Switch to using new archetype.ImageEncoded Introduce ImageChromaDownsampled to Python LOG API Jul 14, 2024
@emilk emilk marked this pull request as ready for review July 14, 2024 14:38
@emilk emilk changed the title Introduce ImageChromaDownsampled to Python LOG API Introduce ImageChromaDownsampled Jul 14, 2024
@emilk emilk mentioned this pull request Jul 14, 2024
1 task
@emilk emilk added the do-not-merge Do not merge this PR label Jul 14, 2024
@emilk emilk changed the title Introduce ImageChromaDownsampled [3/4] Introduce ImageChromaDownsampled Jul 14, 2024
@Wumpf Wumpf self-requested a review July 15, 2024 08:09
Copy link
Copy Markdown
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

onwards! :)

Obviously not blocking on this since ImageChromaDownsampled is meant to be temporary as well, but I really hope we can get away without any of these pseudo archetypes at the end of this :)

Comment on lines -21 to -45
BMP: ImageFormat
"""
BMP file format.
"""

GIF: ImageFormat
"""
JPEG/JPG file format.
"""

JPEG: ImageFormat
"""
JPEG/JPG file format.
"""

PNG: ImageFormat
"""
PNG file format.
"""

TIFF: ImageFormat
"""
TIFF file format.
"""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

from a python user's pov this will now look like we support fewer formats. We need to write something in the changelog/migration guide to clarify what's happening 🤔
Also we should probably enable gif, bmp and tiff support as well

*,
path: str | pathlib.Path | None = None,
contents: datatypes.BlobLike | None = None,
contents: bytes | IO[bytes] | datatypes.BlobLike | None = None,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

BlobLike should probably contain the IO[bytes] alias? This can be done in its fbs definition with the right python attribute

emilk added a commit that referenced this pull request Jul 15, 2024
### What
* Part of #6844
* Next PR: #6874
* Next +1: #6883 (where we rename
it to `ImageChromaDownsampled`)
* Next +2: #6884

This is a temporary measure to make room for a new
`archetypes.ImageEncoded` without name collision.
I plan to remove `ImageEncodedHelper` fully before 0.18.

`ImageEncodedHelper` handles both image files (JPEG, PNG, …) and chroma
sub-sampled data (NV12, YUY2).

In the new design, the former will be handled by `ImageEncoded`, but
chroma-subsampled images will be stored directly as `Image`s, with a
special `PixelFormat`.

This means that once 0.18 is done, only users using chroma subsampling
will have to update their code. I'll make sure the new `ImageEncoded`
archetype ctor supports all of the same parameters as the old helper, so
that there are helpful runtime error messages for our users.


### 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/6882?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/6882?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/6882)
- [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 emilk force-pushed the emilk/image-encoded branch from 9cd1305 to 34efa0d Compare July 15, 2024 09:38
emilk added a commit that referenced this pull request Jul 15, 2024
)

### 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 <andreas@rerun.io>
Base automatically changed from emilk/image-encoded to main July 15, 2024 10:11
An error occurred while trying to automatically change base from emilk/image-encoded to main July 15, 2024 10:11
@emilk emilk force-pushed the emilk/use-image-encoded branch from 209e0d2 to b54fbbd Compare July 15, 2024 10:11
@emilk emilk merged commit 1229d7c into main Jul 15, 2024
@emilk emilk deleted the emilk/use-image-encoded branch July 15, 2024 10:12
emilk added a commit that referenced this pull request Jul 15, 2024
…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`.
@teh-cmc teh-cmc changed the title [3/4] Introduce ImageChromaDownsampled Introduce ImageChromaDownsampled Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Do not merge this PR examples Issues relating to the Rerun examples include in changelog sdk-python Python logging API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace TensorBuffer::JPEG with ImageEncoded archetype

2 participants