Skip to content

Commit

Permalink
Document colorimetry aspects of video decoding, fix mixing up primari…
Browse files Browse the repository at this point in the history
…es & matrix coefficients (#7707)

### What

Learned a lot more stuff recently which is now documented better.
Previously incorrectly called things out as color primaries when
_actually_ the difference is not the primaries (they are the same unless
you go hairsplitting with PAL & NTSC), but the "Matrix Coefficients"

There's a small functional change in this pr - use dav1d's
`matrix_coefficients` instead of `color_primaries` - but since we're
very likely to still almost always choose Bt709 coefficients I expect
this to be a very very minor change in practice.

Meaning to say the meat of this is the docs & renames ;-)

### 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/7707?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/7707?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/7707)
- [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`.
  • Loading branch information
Wumpf authored Oct 14, 2024
1 parent b8ba3d2 commit e845174
Show file tree
Hide file tree
Showing 13 changed files with 237 additions and 142 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
namespace rerun.datatypes;

// TODO(andreas): Clarify relationship to color primaries. Right now there's some hardcoded differences between formats.
// TODO(andreas): Clarify relationship to color primaries & yuv matrix coefficients.
// Right now there's some hardcoded differences between formats.
// See `image_to_gpu.rs`
// Suggestion: guides heuristic but doesn't specify it unless noted.

Expand Down
13 changes: 7 additions & 6 deletions crates/store/re_types/src/datatypes/pixel_format_ext.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::image::{rgb_from_yuv, ColorPrimaries};
use crate::image::{rgb_from_yuv, YuvMatrixCoefficients};

use super::{ChannelDatatype, ColorModel, PixelFormat};

Expand Down Expand Up @@ -187,9 +187,10 @@ impl PixelFormat {
}
}

/// Color primaries used by this format.
/// Yuv matrix coefficients used by this format.
// TODO(andreas): Expose this in the API separately and document it better.
pub fn color_primaries(&self) -> ColorPrimaries {
#[allow(clippy::unnecessary_wraps)]
pub fn yuv_matrix_coefficients(&self) -> YuvMatrixCoefficients {
match self {
Self::Y_U_V24_LimitedRange
| Self::Y_U_V24_FullRange
Expand All @@ -199,9 +200,9 @@ impl PixelFormat {
| Self::Y_U_V16_FullRange
// TODO(andreas): Y8 isn't really color, does this even make sense?
| Self::Y8_FullRange
| Self::Y8_LimitedRange => ColorPrimaries::Bt709,
| Self::Y8_LimitedRange => YuvMatrixCoefficients::Bt709,

Self::NV12 | Self::YUY2 => ColorPrimaries::Bt601,
Self::NV12 | Self::YUY2 => YuvMatrixCoefficients::Bt601,
}
}

Expand All @@ -216,7 +217,7 @@ impl PixelFormat {
u,
v,
self.is_limited_yuv_range(),
self.color_primaries(),
self.yuv_matrix_coefficients(),
))
}
}
27 changes: 18 additions & 9 deletions crates/store/re_types/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,22 @@ fn test_find_non_empty_dim_indices() {

// ----------------------------------------------------------------------------

// TODO(andreas): Expose this in the API.
/// Type of color primaries a given image is in.
// TODO(andreas): Expose this in the API?
/// Yuv matrix coefficients that determine how a YUV image is meant to be converted to RGB.
///
/// This applies both to YUV and RGB formats, but if not specified otherwise
/// we assume BT.709 primaries for all RGB(A) 8bits per channel content.
/// A rigorious definition of the yuv conversion matrix would still require to define
/// the transfer characteristics & color primaries of the resulting RGB space.
/// See [`re_video::decode`]'s documentation.
///
/// However, at this point we generally assume that no further processing is needed after the transform.
/// This is acceptable for most non-HDR content because of the following properties of `Bt709`/`Bt601`/ sRGB:
/// * Bt709 & sRGB primaries are practically identical
/// * Bt601 PAL & Bt709 color primaries are the same (with some slight differences for Bt709 NTSC)
/// * Bt709 & sRGB transfer function are almost identical (and the difference is widely ignored)
/// (sources: <https://en.wikipedia.org/wiki/Rec._709>, <https://en.wikipedia.org/wiki/Rec._601>)
/// …which means for the moment we pretty much only care about the (actually quite) different YUV conversion matrices!
#[derive(Clone, Copy, Debug)]
pub enum ColorPrimaries {
pub enum YuvMatrixCoefficients {
/// BT.601 (aka. SDTV, aka. Rec.601)
///
/// Wiki: <https://en.wikipedia.org/wiki/YCbCr#ITU-R_BT.601_conversion/>
Expand Down Expand Up @@ -310,7 +319,7 @@ pub fn rgb_from_yuv(
u: u8,
v: u8,
limited_range: bool,
primaries: ColorPrimaries,
coefficients: YuvMatrixCoefficients,
) -> [u8; 3] {
let (mut y, mut u, mut v) = (y as f32, u as f32, v as f32);

Expand All @@ -332,15 +341,15 @@ pub fn rgb_from_yuv(
let g;
let b;

match primaries {
ColorPrimaries::Bt601 => {
match coefficients {
YuvMatrixCoefficients::Bt601 => {
// BT.601 (aka. SDTV, aka. Rec.601). wiki: https://en.wikipedia.org/wiki/YCbCr#ITU-R_BT.601_conversion
r = y + 1.402 * v;
g = y - 0.344 * u - 0.714 * v;
b = y + 1.772 * u;
}

ColorPrimaries::Bt709 => {
YuvMatrixCoefficients::Bt709 => {
// BT.709 (aka. HDTV, aka. Rec.709). wiki: https://en.wikipedia.org/wiki/YCbCr#ITU-R_BT.709_conversion
r = y + 1.575 * v;
g = y - 0.187 * u - 0.468 * v;
Expand Down
85 changes: 44 additions & 41 deletions crates/store/re_video/src/decode/av1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::Time;
use dav1d::{PixelLayout, PlanarImageComponent};

use super::{
Chunk, ColorPrimaries, Error, Frame, OutputCallback, PixelFormat, Result, SyncDecoder,
Chunk, Error, Frame, OutputCallback, PixelFormat, Result, SyncDecoder, YuvMatrixCoefficients,
YuvPixelLayout, YuvRange,
};

Expand Down Expand Up @@ -115,11 +115,6 @@ fn output_picture(
picture: &dav1d::Picture,
on_output: &(dyn Fn(Result<Frame>) + Send + Sync),
) {
// TODO(jan): support other parameters?
// What do these even do:
// - matrix_coefficients
// - transfer_characteristics

let data = {
re_tracing::profile_scope!("copy_picture_data");

Expand Down Expand Up @@ -218,7 +213,7 @@ fn output_picture(
dav1d::pixel::YUVRange::Limited => YuvRange::Limited,
dav1d::pixel::YUVRange::Full => YuvRange::Full,
},
primaries: color_primaries(debug_name, picture),
coefficients: yuv_matrix_coefficients(debug_name, picture),
};

let frame = Frame {
Expand All @@ -232,68 +227,76 @@ fn output_picture(
on_output(Ok(frame));
}

fn color_primaries(debug_name: &str, picture: &dav1d::Picture) -> ColorPrimaries {
fn yuv_matrix_coefficients(debug_name: &str, picture: &dav1d::Picture) -> YuvMatrixCoefficients {
// Quotes are from https://wiki.x266.mov/docs/colorimetry/matrix (if not noted otherwise)
#[allow(clippy::match_same_arms)]
match picture.color_primaries() {
dav1d::pixel::ColorPrimaries::Reserved
| dav1d::pixel::ColorPrimaries::Reserved0
| dav1d::pixel::ColorPrimaries::Unspecified => {
match picture.matrix_coefficients() {
// TODO(andreas) This one we should probably support! Afaik this means to just interpret YUV as RGB (or is there a swizzle?).
dav1d::pixel::MatrixCoefficients::Identity => YuvMatrixCoefficients::Bt709,

dav1d::pixel::MatrixCoefficients::BT709 => YuvMatrixCoefficients::Bt709,

dav1d::pixel::MatrixCoefficients::Unspecified
| dav1d::pixel::MatrixCoefficients::Reserved => {
// This happens quite often. Don't issue a warning, that would be noise!

if picture.transfer_characteristic() == dav1d::pixel::TransferCharacteristic::SRGB {
// If the transfer characteristic is sRGB, assume BT.709 primaries, would be quite odd otherwise.
// TODO(andreas): Other transfer characteristics may also hint at primaries.
ColorPrimaries::Bt709
YuvMatrixCoefficients::Bt709
} else {
// Best guess: If the picture is 720p+ assume Bt709 because Rec709
// is the "HDR" standard.
// TODO(#7594): 4k/UHD material should probably assume Bt2020?
// else if picture.height() >= 720 {
// ColorPrimaries::Bt709
// YuvMatrixCoefficients::Bt709
// } else {
// ColorPrimaries::Bt601
// YuvMatrixCoefficients::Bt601
// }
//
// This is also what the mpv player does (and probably others):
// https://wiki.x266.mov/docs/colorimetry/primaries#2-unspecified
// https://wiki.x266.mov/docs/colorimetry/matrix#2-unspecified
// (and similar for primaries! https://wiki.x266.mov/docs/colorimetry/primaries#2-unspecified)
//
// …then again, eyeballing VLC it looks like it just always assumes BT.709.
// The handwavy test case employed here was the same video in low & high resolution
// without specified primaries. Both looked the same.
ColorPrimaries::Bt709
YuvMatrixCoefficients::Bt709
}
}

dav1d::pixel::ColorPrimaries::BT709 => ColorPrimaries::Bt709,

// NTSC standard. Close enough to BT.601 for now. TODO(andreas): Is it worth warning?
dav1d::pixel::ColorPrimaries::BT470M => ColorPrimaries::Bt601,

// PAL standard. Close enough to BT.601 for now. TODO(andreas): Is it worth warning?
dav1d::pixel::ColorPrimaries::BT470BG => ColorPrimaries::Bt601,

// These are both using BT.2020 primaries.
dav1d::pixel::ColorPrimaries::ST170M | dav1d::pixel::ColorPrimaries::ST240M => {
ColorPrimaries::Bt601
dav1d::pixel::MatrixCoefficients::BT470M => {
// "BT.470M is a standard that was used in analog television systems in the United States."
// I guess Bt601 will do!
YuvMatrixCoefficients::Bt601
}
dav1d::pixel::MatrixCoefficients::BT470BG | dav1d::pixel::MatrixCoefficients::ST170M => {
// This is PAL & NTSC standards, both are part of Bt.601.
YuvMatrixCoefficients::Bt601
}
dav1d::pixel::MatrixCoefficients::ST240M => {
// "SMPTE 240M was an interim standard used during the early days of HDTV (1988-1998)."
// Not worth the effort: HD -> Bt709 🤷
YuvMatrixCoefficients::Bt709
}

// Is st428 also HDR? Not sure.
// BT2020 and P3 variants definitely are ;)
dav1d::pixel::ColorPrimaries::BT2020
| dav1d::pixel::ColorPrimaries::ST428
| dav1d::pixel::ColorPrimaries::P3DCI
| dav1d::pixel::ColorPrimaries::P3Display => {
// TODO(#7594): HDR support.
dav1d::pixel::MatrixCoefficients::BT2020NonConstantLuminance
| dav1d::pixel::MatrixCoefficients::BT2020ConstantLuminance
| dav1d::pixel::MatrixCoefficients::ICtCp
| dav1d::pixel::MatrixCoefficients::ST2085 => {
// TODO(#7594): HDR support (we'll probably only care about `BT2020NonConstantLuminance`?)
re_log::warn_once!("Video {debug_name:?} specified HDR color primaries. Rerun doesn't handle HDR colors correctly yet. Color artifacts may be visible.");
ColorPrimaries::Bt709
YuvMatrixCoefficients::Bt709
}

dav1d::pixel::ColorPrimaries::Film | dav1d::pixel::ColorPrimaries::Tech3213 => {
dav1d::pixel::MatrixCoefficients::ChromaticityDerivedNonConstantLuminance
| dav1d::pixel::MatrixCoefficients::ChromaticityDerivedConstantLuminance
| dav1d::pixel::MatrixCoefficients::YCgCo => {
re_log::warn_once!(
"Video {debug_name:?} specified unsupported color primaries {:?}. Color artifacts may be visible.",
picture.color_primaries()
);
ColorPrimaries::Bt709
"Video {debug_name:?} specified unsupported matrix coefficients {:?}. Color artifacts may be visible.",
picture.matrix_coefficients()
);
YuvMatrixCoefficients::Bt709
}
}
}
86 changes: 82 additions & 4 deletions crates/store/re_video/src/decode/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,82 @@
//! Video frame decoding.
//! =========================
//!
//! Whirlwind tour of how to interpret picture data (from a Video perspective)
//! ---------------------------------------------------------------------------------
//!
//! Extracted from the [av1 codec wiki](https://wiki.x266.mov/docs/colorimetry/intro) and other sources.
//! Follows the trail of information we get from our AV1 decoder.
//!
//! ### How to get from YUV to RGB?
//!
//! Things to know about the incoming yuv data:
//! * `picture.bit_depth()`
//! * is either 8 or 16
//! * that's how the decoder stores for us but the per component we have either 8 or 10 or 12 bits -> see `picture.bits_per_component()`
//! * `picture.pixel_layout()`
//! * `4:0:0` greyscale
//! * `4:2:0` half horizontal and half vertical resolution for chroma
//! * `4:2:2` half horizontal resolution for chroma
//! * `4:4:4` full resolution for chroma
//! * note that the AV1 decoder gives us always (!) planar data
//! * `picture.color_range()`
//! * yuv data range may be either `limited` or `full`
//! * `full` is what you'd naively expect, just full use up the entire 8/10/12 bits!
//! * `limited` means that only a certain range of values is valid
//! * weirdly enough, DO NOT CLAMP! a lot of software may say it's limited but then use the so-called foot and head space anyways to go outside the regular colors
//! * reportedly (read this on some forums ;-)) some players _do_ clamp, so let's not get too concerned about this
//! * it's a remnant of the analog age, but it's still very common!
//! * TODO(andreas): It may actually be non-downsampled RGB as well, so skip the entire YUV conversion step! Haven't figured out yet how to determine that.
//!
//!
//! ### Given a normalized YUV triplet, how do we get color?
//!
//! * `picture.matrix_coefficients()` (see <https://wiki.x266.mov/docs/colorimetry/matrix>)
//! * this tells us what to multiply the incoming YUV data with to get SOME RGB data
//! * there's various standards of how to do this, but the most common is BT.709
//! * `picture.primaries()`
//! * now we have RGB but we kinda have no idea what that means!
//! * the color primaries tell us which space we're in
//! * ...meaning that if the primaries are anything else we'd have to do some conversion BUT
//! it also means that we have no chance of displaying the picture perfectly on a screen taking in sRGB (or any other not-matching color space)
//! * [Wikipedia says](https://en.wikipedia.org/wiki/Rec._709#Relationship_to_sRGB) sRGB uses the same primaries as BT.709
//! * but I also found other sources (e.g. [this forum post](https://forum.doom9.org/showthread.php?p=1640342#post1640342))
//! clamining that they're just close enough to be considered the same for practical purposes
//! * `picture.transfer_characteristics()`
//! * until this point everything is "gamma compressed", or more accurately, went through Opto Electric Transfer Function (OETF)
//! * i.e. measure of light in, electronic signal out
//! * we have to keep in mind the EOTF that our screen at the other end will use which for today's renderpipeline is always sRGB
//! (meaning it's a 2.2 gamma curve with a small linear part)
//! * Similar to the primaries, BT.709 uses a _similar_ transfer function as sRGB, but not exactly the same
//! <https://www.image-engineering.de/library/technotes/714-color-spaces-rec-709-vs-srgb>
//! * There's reason to believe players just ignore this:
//! * From a [VLC issue](https://code.videolan.org/videolan/vlc/-/issues/26999):
//! > We do not support transfers or primaries anyway, so it does not matter
//! > (we do support HDR transfer functions PQ and HLG, not SDR ones and we support BT.2020 primaries, but not SMPTE C (which is what BT.601 NTSC is))."
//! * …I'm sure I found a report of other video players ignoring this and most of everything except `matrix_coefficients` but I can't find it anymore :(
//!
//! All of the above are completely optional for a video to specify and there's sometimes some interplay of relationships with those.
//! (a standard would often specify several things at once, there's typical and less typical combinations)
//! So naturally, people will use terms sloppily and interchangeably,
//! If anything is lacking a video player has to make a guess.
//! … and as discussed above, even it's there, often video players tend to ignore some settings!
//!
//! With all this out of the way…
//!
//! ### What's the state of us making use of all these things?
//!
//! * ❌ `picture.bit_depth()`
//! * TODO(#7594): ignored, we just pretend everything is 8 bits
//! * ✅ `picture.pixel_layout()`
//! * ✅ `picture.color_range()`
//! * 🟧 `picture.matrix_coefficients()`
//! * we try to figure out whether to use `BT.709` or `BT.601` coefficients, using other characteristics for guessing if nothing else is available.
//! * ❌ `picture.primaries()`
//! * ❌ `picture.transfer_characteristics()`
//!
//! We'll very likely be good with this until either we get specific feature requests and/or we'll start
//! supporting HDR content at which point more properties will be important!
//!
#[cfg(feature = "av1")]
#[cfg(not(target_arch = "wasm32"))]
Expand Down Expand Up @@ -70,7 +148,7 @@ pub enum PixelFormat {
range: YuvRange,
// TODO(andreas): color primaries should also apply to RGB data,
// but for now we just always assume RGB to be BT.709 ~= sRGB.
primaries: ColorPrimaries,
coefficients: YuvMatrixCoefficients,
},
}

Expand All @@ -95,11 +173,11 @@ pub enum YuvRange {
Full,
}

/// Color primaries used by [`PixelFormat::Yuv`].
/// Yuv matrix coefficients used by [`PixelFormat::Yuv`].
///
/// For details see `re_renderer`'s `ColorPrimaries` type.
/// For details see `re_renderer`'s `YuvMatrixCoefficients` type.
#[derive(Debug)]
pub enum ColorPrimaries {
pub enum YuvMatrixCoefficients {
Bt601,
Bt709,
}
Loading

0 comments on commit e845174

Please sign in to comment.