New planar pixel formats: `Y_U_V24`/`Y_U_V16`/`Y_U_V12` - `_LimitedRange`/`FullRange` #7666

Wumpf · 2024-10-09T17:06:34Z

What

Adds a whole bunch of new pixel formats with limited & full range YUV variants. All of these are needed by our AV1 decoder (branch is a misnomer though, this is coming in another pr!)

Naming & categorizing these formats is quite the struggle just as expected:
If limited to YUV variants, the overall picutre becomes sharper (ironic, eh) and it's possible to specify the combinatorics in a deceptively simple pattern:

planarity & downsample ratio
limited/full value range
bit depth (we only do 8 bit right now)
color primaries

That's what is happening in re_renderer's YUV converter now and is very useful for writing the conversions themselves!

However (!!!), in the wild names make these properties often convoluted with each other, making the categorization a lot harder. E.g. NV12 is almost always (afaik just almost) implying LimitedRange YUV (which is generally a common default unless you're dealing with jpeg derived data [...]).
This, and the fact that we already set out with PixelFormat for a flat list of names, strongly compelled me to continue the current naming scheme (and Ocean's numbering as far as possible) in the user facing API. Worth revisiting in the future.

The primary motivation to expose these formats right away was to be able to easily test these variants. However, I refrained from throwing color primaries into the mix here because I think they should in fact be separately defined, causing a bigger expansion of the APIs. The docs are intentionally kept vague on color space for the moment (trying hard to not further get entangled here).
As a result, our internal processing is now fairly explicit on the matter, but the input interpretation is rather vague.

(screenshot of the release checklist which is part of this pr (moved from a previous test). I manually tested the color inspection feature against a reference color picker - slight rounding errors occasionally, but they are hard to avoid since GPU/CPU don't always do the exact same thing :/)

https://rerun.io/viewer/pr/7666?url=https%3A%2F%2Fstatic.rerun.io%2Frrd%2F0.19.0%2Fchroma_4688549ca3c20018fa2fb658b56482b955817b6a.rrd

Checklist

chrome/firefox windows
chrome/firefox/safari mac
chrome/firefox linux
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):
- Using examples from latest main build: rerun.io/viewer
- Using full set of examples from nightly build: rerun.io/viewer
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.

github-actions · 2024-10-09T17:07:10Z

Deployed docs

Commit	Link
`cfe85fd`	https://landing-963lg4x1c-rerun.vercel.app/docs

Wumpf · 2024-10-10T09:02:03Z

crates/store/re_types/definitions/rerun/datatypes/pixel_format.fbs

+    /// This uses limited range YUV, i.e. Y is expected to be within [16, 235] and U/V within [16, 240].
+    ///
+    /// First comes entire image in Y in one plane, followed by the U and V planes.
+    Y_U_V24_LimitedRange = 39,


the ordering in respect to the numbers in here is rather dubious
I tried to order this logically rather than by enum numbers (which are derived from Ocean). But that makes it hard for the codes beyond Ocean :/

Wumpf · 2024-10-10T09:12:07Z

.gitignore

@@ -31,6 +31,7 @@ _deps
 # Python build artifacts:
 __pycache__
 *.pyc
+*.pyd


not sure why this didn't come up before, but while working on this I accidentally pushed a .pyd file which wasn't on the ignore list. better safe than sorry (again), so I added this here

crates/store/re_types/src/image.rs

crates/store/re_video/src/decode/av1.rs

crates/viewer/re_renderer/src/resource_managers/yuv_converter.rs

…. lotsa fixes etc. on the way

squasing and deleting this branch should be enough to remove it from history

emilk

Niiiice!

emilk · 2024-10-10T09:59:46Z

crates/store/re_types/definitions/rerun/datatypes/pixel_format.fbs

+    ///
+    /// This uses entire range YUV, i.e. Y is expected to be within [0, 255].
+    /// (as opposed to "limited range" YUV as used e.g. in NV12).
+    Y8_FullRange = 30,


At some point it would be nice if LimitedRange/FullRange was its own component. Maybe.

yeah... maybe. See PR description 😄

emilk · 2024-10-10T10:02:57Z

crates/store/re_types/src/datatypes/pixel_format_ext.rs

+            // TODO(andreas): This shouldn't be ColorModel::RGB, but our YUV converter can't do anything else right now.
+            Self::Y8_LimitedRange | Self::Y8_FullRange => ColorModel::RGB,


Can you elaborate on this?

You mean the GPU always outputs RGB, so we consider Y to be a very stupid way of compressing RGB?

What is color_model used for? Is there anything that will break due to this "lie"?

(will add to comment)
GPU doesn't have to always output RGB, but having it sometimes output R(8) specifically for the YUV converter requires me to do more bookkeeping (needs a new renderpipeline and I expect other ripples) which I just don't want to do right now.
We do use this property mostly for things in the hover logic afaik.

very stupid way of compressing RGB

hm well. no. That's just because our gpu conversion pipeline is stupid atm. But it still doesn't take up extra CPU memory and GPU textures are only held as long as they are in use (image cache for instance doesn't cache gpu transfer step!)

a fun way to see this short-lived-cacheing in action is to observe what happens when you change the shader code in yuv_converter.wgsl in a debug build:
It will reload the shader which has no effect unless you show and hide the respective space views at which point the shader runs again.

(was very very very useful for developing that shader! I ended up having a separate dummy-recording that I switched to and back :D)

emilk · 2024-10-10T10:05:21Z

crates/store/re_types/src/datatypes/pixel_format_ext.rs

+    /// Returns true if the format is a YUV format using
+    /// limited range YUV.
+    ///
+    /// I.e. for 8bit data, Y is valid in [16, 235] and U/V [16, 240], rather than 0-255.
+    pub fn is_limited_yuv_range(&self) -> bool {


I think this would be nicer as fn yuv_range(&self) -> Option<YuvRange> so we can return None for non-yuv formats in the future. Also, enums is nicer than bools.

yeah but I didn't want to introduce this enum a third time (1x each in re_renderer, re_types and re_video -.-)

If it was a good idea in the other places, why isn't it a good idea here?

emilk · 2024-10-10T10:22:14Z

crates/viewer/re_viewer_context/src/tensor/image_stats.rs

+            Some(_) => {
+                // We do the lazy thing here since we convert everything to RGB8 right now anyways.


Maybe it would be smart to prepare ourself for HDR though?

Suggested change

Some(_) => {

// We do the lazy thing here since we convert everything to RGB8 right now anyways.

Some(pixel_format) => {

let max_value = pixel_format.max_component_value();

…

Unfortunately this isn't about the max component value of the pixel format but rather what we show which right now is a bit different. max_component_value would be an interesting one also for limited range YUV stuff 🤔
I'm leaving an additional note on this for the next person!

emilk · 2024-10-10T10:23:33Z

scripts/ci/main.py

+def bgra2y_u_v24(bgra: Any, full_range: bool) -> np.ndarray:
+    if full_range:
+        yvu = cv2.cvtColor(bgra, cv2.COLOR_BGR2YCrCb)
+        y, v, u = cv2.split(yvu)
+    else:
+        yuv = cv2.cvtColor(bgra, cv2.COLOR_BGR2YUV)
+        y, u, v = cv2.split(yuv)
+    y = np.array(y).flatten()
+    u = np.array(u).flatten()
+    v = np.array(v).flatten()
+    yuv24 = np.concatenate((y, u, v))
+    return yuv24.astype(np.uint8)


is there no python library for this? :/

dunno, surely but looking and comparing this would have taken me more time than just writing it out myself

scripts/ci/main.py

Wumpf · 2024-10-10T15:00:09Z

having an eye on the missing testing, but will delay a bit here in order to move on to the next thing!

Wumpf added 🔺 re_renderer affects re_renderer itself include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages labels Oct 9, 2024

Wumpf changed the title ~~New pixel formats: Y_U_V24/Y_U_V16/Y_U_V12_LimitedRange/FullRange~~ New pixel formats: Y_U_V24/Y_U_V16/Y_U_V12 - _LimitedRange/FullRange Oct 9, 2024

Wumpf changed the title ~~New pixel formats: Y_U_V24/Y_U_V16/Y_U_V12 - _LimitedRange/FullRange~~ New planar pixel formats: Y_U_V24/Y_U_V16/Y_U_V12 - _LimitedRange/FullRange Oct 9, 2024

Wumpf force-pushed the andreas/av1-chroma-upsampling-gpu branch from 92cfcda to a25cc74 Compare October 9, 2024 17:09

Wumpf commented Oct 10, 2024

View reviewed changes

crates/store/re_types/src/image.rs Outdated Show resolved Hide resolved

Wumpf commented Oct 10, 2024

View reviewed changes

crates/store/re_types/src/image.rs Show resolved Hide resolved

Wumpf commented Oct 10, 2024

View reviewed changes

crates/store/re_video/src/decode/av1.rs Outdated Show resolved Hide resolved

Wumpf commented Oct 10, 2024

View reviewed changes

crates/viewer/re_renderer/src/resource_managers/yuv_converter.rs Outdated Show resolved Hide resolved

Wumpf and others added 14 commits October 10, 2024 11:29

change naming schema on YuvPixelLayout and extend it

7157262

more crazy notes about yuv ranges

d75562c

iterate on naming scheme

d83bbdc

expose new planar yuv formats, including limited/full range

200163a

handle all these new foramts and test them in chroma_downsample_image…

b6266d4

…. lotsa fixes etc. on the way

move chroma downsampling test to release checklist

886a5b6

typo fixes

2ce1a58

rerun linter fixes

f0b7e16

Fix web viewer

649348b

Expose Y8

a2eb9ae

Fix C++ build

97a2091

remove accidentally added (very large!) pyd file

b97e9a5

squasing and deleting this branch should be enough to remove it from history

fix clippy warning

dc5418e

comment fixes etc.

f296688

Wumpf force-pushed the andreas/av1-chroma-upsampling-gpu branch from f276ba0 to f296688 Compare October 10, 2024 09:40

Wumpf marked this pull request as ready for review October 10, 2024 09:51

emilk self-requested a review October 10, 2024 09:58

emilk approved these changes Oct 10, 2024

View reviewed changes

Wumpf added 2 commits October 10, 2024 14:22

delete mysterious main.py file

d99241e

more clarifying comments

cfe85fd

Wumpf merged commit 06cfa9e into main Oct 10, 2024
39 of 40 checks passed

Wumpf deleted the andreas/av1-chroma-upsampling-gpu branch October 10, 2024 15:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New planar pixel formats: `Y_U_V24`/`Y_U_V16`/`Y_U_V12` - `_LimitedRange`/`FullRange` #7666

New planar pixel formats: `Y_U_V24`/`Y_U_V16`/`Y_U_V12` - `_LimitedRange`/`FullRange` #7666

Wumpf commented Oct 9, 2024 •

edited

Loading

github-actions bot commented Oct 9, 2024 •

edited

Loading

Wumpf Oct 10, 2024

Wumpf Oct 10, 2024

emilk left a comment

emilk Oct 10, 2024

Wumpf Oct 10, 2024

emilk Oct 10, 2024

Wumpf Oct 10, 2024

Wumpf Oct 10, 2024 •

edited

Loading

emilk Oct 10, 2024

Wumpf Oct 10, 2024

emilk Oct 10, 2024

emilk Oct 10, 2024

Wumpf Oct 10, 2024

emilk Oct 10, 2024

Wumpf Oct 10, 2024

Wumpf commented Oct 10, 2024

		// TODO(andreas): This shouldn't be ColorModel::RGB, but our YUV converter can't do anything else right now.
		Self::Y8_LimitedRange \| Self::Y8_FullRange => ColorModel::RGB,

		Some(_) => {
		// We do the lazy thing here since we convert everything to RGB8 right now anyways.

New planar pixel formats: Y_U_V24/Y_U_V16/Y_U_V12 - _LimitedRange/FullRange #7666

New planar pixel formats: Y_U_V24/Y_U_V16/Y_U_V12 - _LimitedRange/FullRange #7666

Conversation

Wumpf commented Oct 9, 2024 • edited Loading

What

Checklist

github-actions bot commented Oct 9, 2024 • edited Loading

Deployed docs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wumpf Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wumpf commented Oct 10, 2024

New planar pixel formats: `Y_U_V24`/`Y_U_V16`/`Y_U_V12` - `_LimitedRange`/`FullRange` #7666

New planar pixel formats: `Y_U_V24`/`Y_U_V16`/`Y_U_V12` - `_LimitedRange`/`FullRange` #7666

Wumpf commented Oct 9, 2024 •

edited

Loading

github-actions bot commented Oct 9, 2024 •

edited

Loading

Wumpf Oct 10, 2024 •

edited

Loading