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

Unify GPU texture conversion (with focus on chroma downsampled formats) #7608

Closed
Tracked by #7605
Wumpf opened this issue Oct 7, 2024 · 4 comments
Closed
Tracked by #7605

Unify GPU texture conversion (with focus on chroma downsampled formats) #7608

Wumpf opened this issue Oct 7, 2024 · 4 comments
Assignees
@Wumpf
Labels
🚀 performance Optimization, memory use, etc 🔺 re_renderer affects re_renderer itself 🎞️ video

Comments

@Wumpf
Copy link
Member

Wumpf commented Oct 7, 2024 •
edited
Loading

Today, we have a variety of formats that we directly support by out "rectangle" shader.
There's a few issues with this though:

What we need instead is a flexible system for arbitrary (potentially) on-gpu texture conversion.
The interface we want is something that gives you a buffer to write in your special-format bytes and then get back a gpu texture. How the texture is filled then is up to the system and may scheduled more gpu operations.

The immediate need for this is chroma conversions for video data, but everything we enable there, we'd also like to remove from the rectangle shader.

Open questions:

  • exact interface
  • does this live on the 2d texture manager? Makes sense, but gets bulky. Have it as a extendable sub-system "under" it instead?
  • any alternatives to scheduling work on the "global" per-frame encoder?
    • likely the easy go-to solution for the moment
  • Output format limited to RGBA8(srgb)?
    • good start definitely
@emilk emilk mentioned this issue Oct 7, 2024
Closed
3 tasks
@Wumpf Wumpf added the 🔺 re_renderer affects re_renderer itself label Oct 7, 2024
@emilk emilk added the 🚀 performance Optimization, memory use, etc label Oct 7, 2024
@Wumpf Wumpf self-assigned this Oct 7, 2024
This was referenced Oct 7, 2024
Merged
Closed
emilk added a commit that referenced this issue Oct 7, 2024
@emilk
Fix main CI (#7613)
2945950 
### What
This disables the `asm` feature of `rav1d`, which means:
* No need to install `nasm` (fixing the build)
* Slower video decoding (but still limited mainly by
#7608)

### 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/7613?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/7613?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/7613)
- [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
Copy link
Member

emilk commented Oct 7, 2024 •
edited
Loading

We should add the newly supported pixel formats to rerun::PixelFormat as well.

We should consider following the naming convention of https://facebookresearch.github.io/ocean/docs/images/pixel_formats_and_plane_layout/

@emilk
Copy link
Member

emilk commented Oct 7, 2024

Here are the pixel formats we need to support for video: https://docs.rs/dav1d/0.10.3/dav1d/enum.PixelLayout.html

@Wumpf
Copy link
Member Author

Wumpf commented Oct 7, 2024 •
edited
Loading

And these are the color primaries we have to support:
https://docs.rs/dav1d/0.10.3/dav1d/pixel/enum.ColorPrimaries.html

For the moment we shouldn't try to support everything and focus on the most common ones. Which is likely just BT709 for now. I found BT2020 source material as well, but this gets us into HDR support territory, see

Everything we don't support, we should log a warning and cary on with what makes most sense (bt.601 for low res content and bt.709 for higher res (full hd +)... or just always bt.709 because afaik that's the most common anyways)

Other aspects we will need to consider in the future are...

@Wumpf Wumpf mentioned this issue Oct 8, 2024
Merged
9 tasks
Wumpf added a commit that referenced this issue Oct 9, 2024
@Wumpf
Introduce image data conversion pipeline, taking over existing YUV co…
c3af105 
…nversions (#7640)

### What

* Major part of #7608
* Related to #3931
* since it makes the rectangle shader smaller which kept acting up in
the past


via `pixi run -e py python
./tests/python/chroma_downsample_image/main.py`:

https://rerun.io/viewer/pr/7640?url=https%3A%2F%2Fstatic.rerun.io%2Frrd%2F0.19%2Fchroma_formats_ad9697d05933f4104fbbe66af23073ad4e9e4a58.rrd


![image](https://github.com/user-attachments/assets/76756c5f-75db-44b7-8db0-f04256f72e3a)



The new functionality is tightly integrated into `TextureManager2D`
which now ingests all incoming data via a new internal method
`transfer_image_data_to_texture`.
This in turn takes care of data upload via `GpuReadCpuWriteBelt` (new!
previously, we would use `queue.write_texture` in this case) and may if
needed run gpu based conversion.

Gpu based conversion is for convenience implemented like a `Renderer` -
while it is not _used_ like a typical `Renderer`, the lifecycle (create
lazily once, store on context, feed with data bundles [...]) was so
similar that it made sense to me to use this existing pattern even
though it's not a perfect match.

**A curious sideeffect of this is that you can now put chroma
downsampled textures on custom meshes!**

Next steps:
* support formats required for AV1
   * while on it, check if things could get simpler by us  
* move BGR handling into the new pipeline
* revisit the re_renderer sided input data description. Not sure if I'm
happy about it drifting away from the user facing one!
* once we're there we're also very close to get rid of
`SourceImageDataFormat::WgpuCompatible`. But it's not strictly
necessary, this might yet prove a convenient loophole, although its
presence makes me a little bit nervous (details see code comments) 🤔
* consider exposing color primaries to public API
* the only thing keeping us from it is adding a new enum to the color
format!
    * should it be called "primaries" or "color space" 🤔   

### Checklist
* [x] test on web Mac Chrome/Firefox/Safari
* [x] test on web Window Chrome/Firefox
* [ ] test on web Firefox Chrome/Firefox (@jleibs plz 🥺 )
* [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/7640?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/7640?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/7640)
- [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 mentioned this issue Oct 10, 2024
Merged
6 tasks
@Wumpf Wumpf mentioned this issue Oct 10, 2024
Merged
6 tasks
Wumpf added a commit that referenced this issue Oct 11, 2024
@Wumpf
Use GPU based transfer to do chroma conversions of native decoder out…
bf26fb6 
…put (#7682)

### What

* Important chunk of #7605
* Related to #7608
   * todo: create a new ticket for bgr shenanigans and close this one

---

Fixes performance issues with our previous inefficient cpu based chroma
conversion:


![image](https://github.com/user-attachments/assets/fed67eb9-902b-43d8-bbd6-8574a5f3b18d)

... and fixes color space issues:

Top left: now
Top right: before
Bottom: VLC

![image](https://github.com/user-attachments/assets/f5b797bd-e12d-46b0-96dd-32176d43e78c)

---

There was an oversight in the conversion pipeline that if fixed now: It
only allowed for creating new textures.
Now it can also update existing textures! This means that it takes a
target texture from the outside. Naturally, there's some constraints on
what the target texture looks like and as a consequence I added some
validation for that. Could be more, but I think this is solid enough for
our purposes :)


### 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/7682?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/7682?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/7682)
- [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`.
@Wumpf Wumpf mentioned this issue Oct 11, 2024
Open
@Wumpf
Copy link
Member Author

Wumpf commented Oct 11, 2024

We have what we needed for video. Closing this in favor of this follow-up task:

@Wumpf Wumpf closed this as completed Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Wumpf Wumpf

Labels
🚀 performance Optimization, memory use, etc 🔺 re_renderer affects re_renderer itself 🎞️ video
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@emilk @Wumpf