Skip to content

GitLab

T
tangled
Commit a54ef7e4 authored by hubbe's avatar hubbe Committed by Commit bot
Browse files
Options

Always use valid enum values in gfx::ColorSpace 

As a bonus, we get warnings if you add a new enum value without
updating gfx::ColorTransform.

Review-Url: https://codereview.chromium.org/2338213009
Cr-Commit-Position: refs/heads/master@{#419552}
parent c8affb87
Hide whitespace changes
Inline Side-by-side
Showing with 140 additions and 57 deletions
media/filters/ffmpeg_video_decoder.cc
View file @ a54ef7e4
...@@ -144,13 +144,12 @@ int FFmpegVideoDecoder::GetVideoBuffer(struct AVCodecContext* codec_context, ...@@ -144,13 +144,12 @@ int FFmpegVideoDecoder::GetVideoBuffer(struct AVCodecContext* codec_context,
if (codec_context->color_primaries != AVCOL_PRI_UNSPECIFIED || if (codec_context->color_primaries != AVCOL_PRI_UNSPECIFIED ||
codec_context->color_trc != AVCOL_TRC_UNSPECIFIED || codec_context->color_trc != AVCOL_TRC_UNSPECIFIED ||
codec_context->colorspace != AVCOL_SPC_UNSPECIFIED) { codec_context->colorspace != AVCOL_SPC_UNSPECIFIED) {
video_frame->set_color_space(gfx::ColorSpace( video_frame->set_color_space(
static_cast<gfx::ColorSpace::PrimaryID>(codec_context->color_primaries), gfx::ColorSpace(codec_context->color_primaries,
static_cast<gfx::ColorSpace::TransferID>(codec_context->color_trc), codec_context->color_trc, codec_context->colorspace,
static_cast<gfx::ColorSpace::MatrixID>(codec_context->colorspace), codec_context->color_range != AVCOL_RANGE_MPEG
codec_context->color_range != AVCOL_RANGE_MPEG ? gfx::ColorSpace::RangeID::FULL
? gfx::ColorSpace::RangeID::FULL : gfx::ColorSpace::RangeID::LIMITED));
: gfx::ColorSpace::RangeID::LIMITED));
} }
for (size_t i = 0; i < VideoFrame::NumPlanes(video_frame->format()); i++) { for (size_t i = 0; i < VideoFrame::NumPlanes(video_frame->format()); i++) {
......
ui/gfx/color_space.cc
View file @ a54ef7e4
...@@ -12,6 +12,33 @@ ...@@ -12,6 +12,33 @@
namespace gfx { namespace gfx {
ColorSpace::PrimaryID ColorSpace::PrimaryIDFromInt(int primary_id) {
if (primary_id < 0 || primary_id > static_cast<int>(PrimaryID::LAST))
return PrimaryID::UNKNOWN;
if (primary_id > static_cast<int>(PrimaryID::LAST_STANDARD_VALUE) &&
primary_id < 1000)
return PrimaryID::UNKNOWN;
return static_cast<PrimaryID>(primary_id);
}
ColorSpace::TransferID ColorSpace::TransferIDFromInt(int transfer_id) {
if (transfer_id < 0 || transfer_id > static_cast<int>(TransferID::LAST))
return TransferID::UNKNOWN;
if (transfer_id > static_cast<int>(TransferID::LAST_STANDARD_VALUE) &&
transfer_id < 1000)
return TransferID::UNKNOWN;
return static_cast<TransferID>(transfer_id);
}
ColorSpace::MatrixID ColorSpace::MatrixIDFromInt(int matrix_id) {
if (matrix_id < 0 || matrix_id > static_cast<int>(MatrixID::LAST))
return MatrixID::UNKNOWN;
if (matrix_id > static_cast<int>(MatrixID::LAST_STANDARD_VALUE) &&
matrix_id < 1000)
return MatrixID::UNKNOWN;
return static_cast<MatrixID>(matrix_id);
}
ColorSpace::ColorSpace() { ColorSpace::ColorSpace() {
memset(custom_primary_matrix_, 0, sizeof(custom_primary_matrix_)); memset(custom_primary_matrix_, 0, sizeof(custom_primary_matrix_));
} }
...@@ -28,6 +55,15 @@ ColorSpace::ColorSpace(PrimaryID primaries, ...@@ -28,6 +55,15 @@ ColorSpace::ColorSpace(PrimaryID primaries,
// TODO: Set profile_id_ // TODO: Set profile_id_
} }
ColorSpace::ColorSpace(int primaries, int transfer, int matrix, RangeID range)
: primaries_(PrimaryIDFromInt(primaries)),
transfer_(TransferIDFromInt(transfer)),
matrix_(MatrixIDFromInt(matrix)),
range_(range) {
memset(custom_primary_matrix_, 0, sizeof(custom_primary_matrix_));
// TODO: Set profile_id_
}
// static // static
ColorSpace ColorSpace::CreateSRGB() { ColorSpace ColorSpace::CreateSRGB() {
return ColorSpace(PrimaryID::BT709, TransferID::IEC61966_2_1, MatrixID::RGB, return ColorSpace(PrimaryID::BT709, TransferID::IEC61966_2_1, MatrixID::RGB,
......
ui/gfx/color_space.h
View file @ a54ef7e4
...@@ -43,11 +43,13 @@ class GFX_EXPORT ColorSpace { ...@@ -43,11 +43,13 @@ class GFX_EXPORT ColorSpace {
SMPTEST431_2 = 11, SMPTEST431_2 = 11,
SMPTEST432_1 = 12, SMPTEST432_1 = 12,
LAST_STANDARD_VALUE = SMPTEST432_1,
// Chrome-specific values start at 1000. // Chrome-specific values start at 1000.
XYZ_D50 = 1000, UNKNOWN = 1000,
// TODO(hubbe): We need to store the primaries. XYZ_D50,
CUSTOM = 1001, CUSTOM,
LAST = CUSTOM LAST = UNKNOWN
}; };
enum class TransferID : uint16_t { enum class TransferID : uint16_t {
...@@ -72,8 +74,11 @@ class GFX_EXPORT ColorSpace { ...@@ -72,8 +74,11 @@ class GFX_EXPORT ColorSpace {
SMPTEST428_1 = 17, SMPTEST428_1 = 17,
ARIB_STD_B67 = 18, // AKA hybrid-log gamma, HLG. ARIB_STD_B67 = 18, // AKA hybrid-log gamma, HLG.
LAST_STANDARD_VALUE = SMPTEST428_1,
// Chrome-specific values start at 1000. // Chrome-specific values start at 1000.
GAMMA24 = 1000, UNKNOWN = 1000,
GAMMA24,
// This is an ad-hoc transfer function that decodes SMPTE 2084 content // This is an ad-hoc transfer function that decodes SMPTE 2084 content
// into a 0-1 range more or less suitable for viewing on a non-hdr // into a 0-1 range more or less suitable for viewing on a non-hdr
...@@ -100,8 +105,11 @@ class GFX_EXPORT ColorSpace { ...@@ -100,8 +105,11 @@ class GFX_EXPORT ColorSpace {
BT2020_CL = 10, BT2020_CL = 10,
YDZDX = 11, YDZDX = 11,
LAST_STANDARD_VALUE = YDZDX,
// Chrome-specific values start at 1000 // Chrome-specific values start at 1000
LAST = YDZDX, UNKNOWN = 1000,
LAST = UNKNOWN,
}; };
// The h264 spec declares this as bool, so only the the first two values // The h264 spec declares this as bool, so only the the first two values
...@@ -120,6 +128,11 @@ class GFX_EXPORT ColorSpace { ...@@ -120,6 +128,11 @@ class GFX_EXPORT ColorSpace {
TransferID transfer, TransferID transfer,
MatrixID matrix, MatrixID matrix,
RangeID full_range); RangeID full_range);
ColorSpace(int primaries, int transfer, int matrix, RangeID full_range);
static PrimaryID PrimaryIDFromInt(int primary_id);
static TransferID TransferIDFromInt(int transfer_id);
static MatrixID MatrixIDFromInt(int matrix_id);
static ColorSpace CreateSRGB(); static ColorSpace CreateSRGB();
static ColorSpace CreateXYZD50(); static ColorSpace CreateXYZD50();
......
ui/gfx/color_transform.cc
View file @ a54ef7e4
...@@ -40,18 +40,18 @@ ColorTransform::TriStim Xy2xyz(float x, float y) { ...@@ -40,18 +40,18 @@ ColorTransform::TriStim Xy2xyz(float x, float y) {
void GetPrimaries(ColorSpace::PrimaryID id, void GetPrimaries(ColorSpace::PrimaryID id,
ColorTransform::TriStim primaries[4]) { ColorTransform::TriStim primaries[4]) {
switch (id) { switch (id) {
default: case ColorSpace::PrimaryID::CUSTOM:
// If we don't know, assume BT709 NOTREACHED();
case ColorSpace::PrimaryID::RESERVED0:
case ColorSpace::PrimaryID::RESERVED:
case ColorSpace::PrimaryID::UNSPECIFIED:
case ColorSpace::PrimaryID::UNKNOWN:
case ColorSpace::PrimaryID::BT709: case ColorSpace::PrimaryID::BT709:
// Red // BT709 is our default case. Put it after the switch just
primaries[0] = Xy2xyz(0.640f, 0.330f); // in case we somehow get an id which is not listed in the switch.
// Green // (We don't want to use "default", because we want the compiler
primaries[1] = Xy2xyz(0.300f, 0.600f); // to tell us if we forgot some enum values.)
// Blue
primaries[2] = Xy2xyz(0.150f, 0.060f);
// Whitepoint (D65f)
primaries[3] = Xy2xyz(0.3127f, 0.3290f);
break; break;
case ColorSpace::PrimaryID::BT470M: case ColorSpace::PrimaryID::BT470M:
...@@ -63,7 +63,7 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -63,7 +63,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.14f, 0.08f); primaries[2] = Xy2xyz(0.14f, 0.08f);
// Whitepoint // Whitepoint
primaries[3] = Xy2xyz(0.31f, 0.316f); primaries[3] = Xy2xyz(0.31f, 0.316f);
break; return;
case ColorSpace::PrimaryID::BT470BG: case ColorSpace::PrimaryID::BT470BG:
// Red // Red
...@@ -74,7 +74,7 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -74,7 +74,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.15f, 0.06f); primaries[2] = Xy2xyz(0.15f, 0.06f);
// Whitepoint (D65f) // Whitepoint (D65f)
primaries[3] = Xy2xyz(0.3127f, 0.3290f); primaries[3] = Xy2xyz(0.3127f, 0.3290f);
break; return;
case ColorSpace::PrimaryID::SMPTE170M: case ColorSpace::PrimaryID::SMPTE170M:
case ColorSpace::PrimaryID::SMPTE240M: case ColorSpace::PrimaryID::SMPTE240M:
...@@ -86,7 +86,7 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -86,7 +86,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.155f, 0.070f); primaries[2] = Xy2xyz(0.155f, 0.070f);
// Whitepoint (D65f) // Whitepoint (D65f)
primaries[3] = Xy2xyz(0.3127f, 0.3290f); primaries[3] = Xy2xyz(0.3127f, 0.3290f);
break; return;
case ColorSpace::PrimaryID::FILM: case ColorSpace::PrimaryID::FILM:
// Red // Red
...@@ -97,7 +97,7 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -97,7 +97,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.145f, 0.049f); primaries[2] = Xy2xyz(0.145f, 0.049f);
// Whitepoint (Cf) // Whitepoint (Cf)
primaries[3] = Xy2xyz(0.310f, 0.136f); primaries[3] = Xy2xyz(0.310f, 0.136f);
break; return;
case ColorSpace::PrimaryID::BT2020: case ColorSpace::PrimaryID::BT2020:
// Red // Red
...@@ -108,7 +108,7 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -108,7 +108,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.131f, 0.046f); primaries[2] = Xy2xyz(0.131f, 0.046f);
// Whitepoint (D65f) // Whitepoint (D65f)
primaries[3] = Xy2xyz(0.3127f, 0.3290f); primaries[3] = Xy2xyz(0.3127f, 0.3290f);
break; return;
case ColorSpace::PrimaryID::SMPTEST428_1: case ColorSpace::PrimaryID::SMPTEST428_1:
// X // X
...@@ -119,7 +119,7 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -119,7 +119,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.0f, 0.0f); primaries[2] = Xy2xyz(0.0f, 0.0f);
// Whitepoint (Ef) // Whitepoint (Ef)
primaries[3] = Xy2xyz(1.0f / 3.0f, 1.0f / 3.0f); primaries[3] = Xy2xyz(1.0f / 3.0f, 1.0f / 3.0f);
break; return;
case ColorSpace::PrimaryID::SMPTEST431_2: case ColorSpace::PrimaryID::SMPTEST431_2:
// Red // Red
...@@ -130,7 +130,7 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -130,7 +130,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.150f, 0.060f); primaries[2] = Xy2xyz(0.150f, 0.060f);
// Whitepoint // Whitepoint
primaries[3] = Xy2xyz(0.314f, 0.351f); primaries[3] = Xy2xyz(0.314f, 0.351f);
break; return;
case ColorSpace::PrimaryID::SMPTEST432_1: case ColorSpace::PrimaryID::SMPTEST432_1:
// Red // Red
...@@ -141,7 +141,7 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -141,7 +141,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.150f, 0.060f); primaries[2] = Xy2xyz(0.150f, 0.060f);
// Whitepoint (D65f) // Whitepoint (D65f)
primaries[3] = Xy2xyz(0.3127f, 0.3290f); primaries[3] = Xy2xyz(0.3127f, 0.3290f);
break; return;
case ColorSpace::PrimaryID::XYZ_D50: case ColorSpace::PrimaryID::XYZ_D50:
// X // X
...@@ -152,8 +152,17 @@ void GetPrimaries(ColorSpace::PrimaryID id, ...@@ -152,8 +152,17 @@ void GetPrimaries(ColorSpace::PrimaryID id,
primaries[2] = Xy2xyz(0.0f, 0.0f); primaries[2] = Xy2xyz(0.0f, 0.0f);
// D50 // D50
primaries[3] = Xy2xyz(0.34567f, 0.35850f); primaries[3] = Xy2xyz(0.34567f, 0.35850f);
break; return;
} }
// Red
primaries[0] = Xy2xyz(0.640f, 0.330f);
// Green
primaries[1] = Xy2xyz(0.300f, 0.600f);
// Blue
primaries[2] = Xy2xyz(0.150f, 0.060f);
// Whitepoint (D65f)
primaries[3] = Xy2xyz(0.3127f, 0.3290f);
} }
GFX_EXPORT Transform GetPrimaryMatrix(ColorSpace::PrimaryID id) { GFX_EXPORT Transform GetPrimaryMatrix(ColorSpace::PrimaryID id) {
...@@ -191,20 +200,24 @@ GFX_EXPORT Transform GetPrimaryMatrix(ColorSpace::PrimaryID id) { ...@@ -191,20 +200,24 @@ GFX_EXPORT Transform GetPrimaryMatrix(ColorSpace::PrimaryID id) {
GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float v) { GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float v) {
switch (id) { switch (id) {
default: case ColorSpace::TransferID::SMPTEST2084_NON_HDR:
// Should already be handled.
NOTREACHED();
case ColorSpace::TransferID::CUSTOM:
// TODO(hubbe): Actually implement custom transfer functions.
case ColorSpace::TransferID::RESERVED0:
case ColorSpace::TransferID::RESERVED:
case ColorSpace::TransferID::UNSPECIFIED:
case ColorSpace::TransferID::UNKNOWN:
// All unknown values default to BT709
case ColorSpace::TransferID::BT709: case ColorSpace::TransferID::BT709:
case ColorSpace::TransferID::SMPTE170M: case ColorSpace::TransferID::SMPTE170M:
case ColorSpace::TransferID::BT2020_10: case ColorSpace::TransferID::BT2020_10:
case ColorSpace::TransferID::BT2020_12: { case ColorSpace::TransferID::BT2020_12:
v = fmax(0.0f, v); // BT709 is our "default" cause, so put the code after the switch
float a = 1.099296826809442f; // to avoid "control reaches end of non-void function" errors.
float b = 0.018053968510807f; break;
if (v <= b) {
return 4.5f * v;
} else {
return a * powf(v, 0.45f) - (a - 1.0f);
}
}
case ColorSpace::TransferID::GAMMA22: case ColorSpace::TransferID::GAMMA22:
v = fmax(0.0f, v); v = fmax(0.0f, v);
...@@ -305,24 +318,34 @@ GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float v) { ...@@ -305,24 +318,34 @@ GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float v) {
v = fmax(0.0f, v); v = fmax(0.0f, v);
return powf(v, 1.0f / 2.4f); return powf(v, 1.0f / 2.4f);
} }
v = fmax(0.0f, v);
float a = 1.099296826809442f;
float b = 0.018053968510807f;
if (v <= b) {
return 4.5f * v;
} else {
return a * powf(v, 0.45f) - (a - 1.0f);
}
} }
GFX_EXPORT float ToLinear(ColorSpace::TransferID id, float v) { GFX_EXPORT float ToLinear(ColorSpace::TransferID id, float v) {
switch (id) { switch (id) {
default: case ColorSpace::TransferID::CUSTOM:
// TODO(hubbe): Actually implement custom transfer functions.
case ColorSpace::TransferID::RESERVED0:
case ColorSpace::TransferID::RESERVED:
case ColorSpace::TransferID::UNSPECIFIED:
case ColorSpace::TransferID::UNKNOWN:
// All unknown values default to BT709
case ColorSpace::TransferID::BT709: case ColorSpace::TransferID::BT709:
case ColorSpace::TransferID::SMPTE170M: case ColorSpace::TransferID::SMPTE170M:
case ColorSpace::TransferID::BT2020_10: case ColorSpace::TransferID::BT2020_10:
case ColorSpace::TransferID::BT2020_12: { case ColorSpace::TransferID::BT2020_12:
v = fmax(0.0f, v); // BT709 is our "default" cause, so put the code after the switch
float a = 1.099296826809442f; // to avoid "control reaches end of non-void function" errors.
float b = 0.018053968510807f; break;
if (v < FromLinear(ColorSpace::TransferID::BT709, b)) {
return v / 4.5f;
} else {
return powf((v + a - 1.0f) / a, 1.0f / 0.45f);
}
}
case ColorSpace::TransferID::GAMMA22: case ColorSpace::TransferID::GAMMA22:
v = fmax(0.0f, v); v = fmax(0.0f, v);
...@@ -432,6 +455,15 @@ GFX_EXPORT float ToLinear(ColorSpace::TransferID id, float v) { ...@@ -432,6 +455,15 @@ GFX_EXPORT float ToLinear(ColorSpace::TransferID id, float v) {
return v_ / Lmax; return v_ / Lmax;
} }
} }
v = fmax(0.0f, v);
float a = 1.099296826809442f;
float b = 0.018053968510807f;
if (v < FromLinear(ColorSpace::TransferID::BT709, b)) {
return v / 4.5f;
} else {
return powf((v + a - 1.0f) / a, 1.0f / 0.45f);
}
} }
namespace { namespace {
...@@ -461,7 +493,9 @@ GFX_EXPORT ColorTransform::TriStim ToLinear(ColorSpace::TransferID id, ...@@ -461,7 +493,9 @@ GFX_EXPORT ColorTransform::TriStim ToLinear(ColorSpace::TransferID id,
} }
GFX_EXPORT Transform GetTransferMatrix(ColorSpace::MatrixID id) { GFX_EXPORT Transform GetTransferMatrix(ColorSpace::MatrixID id) {
float Kr = 0.0f, Kb = 0.0f; // Default values for BT709;
float Kr = 0.2126f;
float Kb = 0.0722f;
switch (id) { switch (id) {
case ColorSpace::MatrixID::RGB: case ColorSpace::MatrixID::RGB:
return Transform(); return Transform();
...@@ -469,8 +503,8 @@ GFX_EXPORT Transform GetTransferMatrix(ColorSpace::MatrixID id) { ...@@ -469,8 +503,8 @@ GFX_EXPORT Transform GetTransferMatrix(ColorSpace::MatrixID id) {
case ColorSpace::MatrixID::BT709: case ColorSpace::MatrixID::BT709:
case ColorSpace::MatrixID::UNSPECIFIED: case ColorSpace::MatrixID::UNSPECIFIED:
case ColorSpace::MatrixID::RESERVED: case ColorSpace::MatrixID::RESERVED:
Kr = 0.2126f; case ColorSpace::MatrixID::UNKNOWN:
Kb = 0.0722f; // Default values are already set.
break; break;
case ColorSpace::MatrixID::FCC: case ColorSpace::MatrixID::FCC:
...@@ -544,6 +578,7 @@ Transform GetRangeAdjustMatrix(ColorSpace::RangeID range, ...@@ -544,6 +578,7 @@ Transform GetRangeAdjustMatrix(ColorSpace::RangeID range,
case ColorSpace::MatrixID::BT2020_NCL: case ColorSpace::MatrixID::BT2020_NCL:
case ColorSpace::MatrixID::BT2020_CL: case ColorSpace::MatrixID::BT2020_CL:
case ColorSpace::MatrixID::YDZDX: case ColorSpace::MatrixID::YDZDX:
case ColorSpace::MatrixID::UNKNOWN:
return Transform(255.0f / 219.0f, 0.0f, 0.0f, -16.0f / 219.0f, // 1 return Transform(255.0f / 219.0f, 0.0f, 0.0f, -16.0f / 219.0f, // 1
0.0f, 255.0f / 224.0f, 0.0f, -15.5f / 224.0f, // 2 0.0f, 255.0f / 224.0f, 0.0f, -15.5f / 224.0f, // 2
0.0f, 0.0f, 255.0f / 224.0f, -15.5f / 224.0f, // 3 0.0f, 0.0f, 255.0f / 224.0f, -15.5f / 224.0f, // 3
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment