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.

Committed: https://crrev.com/a54ef7e4e7e234b2bb95f2000b72da718cbbc610
Review-Url: https://codereview.chromium.org/2338213009
Cr-Original-Commit-Position: refs/heads/master@{#419552}
Cr-Commit-Position: refs/heads/master@{#419615}

media/filters/ffmpeg_video_decoder.cc

@@ -144,13 +144,12 @@ int FFmpegVideoDecoder::GetVideoBuffer(struct AVCodecContext* codec_context,
   if (codec_context->color_primaries != AVCOL_PRI_UNSPECIFIED ||
       codec_context->color_trc != AVCOL_TRC_UNSPECIFIED ||
       codec_context->colorspace != AVCOL_SPC_UNSPECIFIED) {
-    video_frame->set_color_space(gfx::ColorSpace(
-        static_cast<gfx::ColorSpace::PrimaryID>(codec_context->color_primaries),
-        static_cast<gfx::ColorSpace::TransferID>(codec_context->color_trc),
-        static_cast<gfx::ColorSpace::MatrixID>(codec_context->colorspace),
-        codec_context->color_range != AVCOL_RANGE_MPEG
-            ? gfx::ColorSpace::RangeID::FULL
-            : gfx::ColorSpace::RangeID::LIMITED));
+    video_frame->set_color_space(
+        gfx::ColorSpace(codec_context->color_primaries,
+                        codec_context->color_trc, codec_context->colorspace,
+                        codec_context->color_range != AVCOL_RANGE_MPEG
+                            ? gfx::ColorSpace::RangeID::FULL
+                            : gfx::ColorSpace::RangeID::LIMITED));
   }
 
   for (size_t i = 0; i < VideoFrame::NumPlanes(video_frame->format()); i++) {

ui/gfx/color_space.cc

@@ -12,6 +12,33 @@
 
 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() {
   memset(custom_primary_matrix_, 0, sizeof(custom_primary_matrix_));
 }
@@ -28,6 +55,15 @@ ColorSpace::ColorSpace(PrimaryID primaries,
   // 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
 ColorSpace ColorSpace::CreateSRGB() {
   return ColorSpace(PrimaryID::BT709, TransferID::IEC61966_2_1, MatrixID::RGB,

ui/gfx/color_space.h

@@ -43,10 +43,12 @@ class GFX_EXPORT ColorSpace {
     SMPTEST431_2 = 11,
     SMPTEST432_1 = 12,
 
+    LAST_STANDARD_VALUE = SMPTEST432_1,
+
     // Chrome-specific values start at 1000.
-    XYZ_D50 = 1000,
-    // TODO(hubbe): We need to store the primaries.
-    CUSTOM = 1001,
+    UNKNOWN = 1000,
+    XYZ_D50,
+    CUSTOM,
     LAST = CUSTOM
   };
 
@@ -72,8 +74,11 @@ class GFX_EXPORT ColorSpace {
     SMPTEST428_1 = 17,
     ARIB_STD_B67 = 18,  // AKA hybrid-log gamma, HLG.
 
+    LAST_STANDARD_VALUE = SMPTEST428_1,
+
     // Chrome-specific values start at 1000.
-    GAMMA24 = 1000,
+    UNKNOWN = 1000,
+    GAMMA24,
 
     // 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
@@ -100,8 +105,11 @@ class GFX_EXPORT ColorSpace {
     BT2020_CL = 10,
     YDZDX = 11,
 
+    LAST_STANDARD_VALUE = YDZDX,
+
     // 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
@@ -120,6 +128,11 @@ class GFX_EXPORT ColorSpace {
              TransferID transfer,
              MatrixID matrix,
              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 CreateXYZD50();

ui/gfx/color_transform.cc

@@ -40,18 +40,18 @@ ColorTransform::TriStim Xy2xyz(float x, float y) {
 void GetPrimaries(ColorSpace::PrimaryID id,
                   ColorTransform::TriStim primaries[4]) {
   switch (id) {
-    default:
-    // If we don't know, assume BT709
+    case ColorSpace::PrimaryID::CUSTOM:
+      NOTREACHED();
 
+    case ColorSpace::PrimaryID::RESERVED0:
+    case ColorSpace::PrimaryID::RESERVED:
+    case ColorSpace::PrimaryID::UNSPECIFIED:
+    case ColorSpace::PrimaryID::UNKNOWN:
     case ColorSpace::PrimaryID::BT709:
-      // 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);
+      // BT709 is our default case. Put it after the switch just
+      // in case we somehow get an id which is not listed in the switch.
+      // (We don't want to use "default", because we want the compiler
+      //  to tell us if we forgot some enum values.)
       break;
 
     case ColorSpace::PrimaryID::BT470M:
@@ -63,7 +63,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.14f, 0.08f);
       // Whitepoint
       primaries[3] = Xy2xyz(0.31f, 0.316f);
-      break;
+      return;
 
     case ColorSpace::PrimaryID::BT470BG:
       // Red
@@ -74,7 +74,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.15f, 0.06f);
       // Whitepoint (D65f)
       primaries[3] = Xy2xyz(0.3127f, 0.3290f);
-      break;
+      return;
 
     case ColorSpace::PrimaryID::SMPTE170M:
     case ColorSpace::PrimaryID::SMPTE240M:
@@ -86,7 +86,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.155f, 0.070f);
       // Whitepoint (D65f)
       primaries[3] = Xy2xyz(0.3127f, 0.3290f);
-      break;
+      return;
 
     case ColorSpace::PrimaryID::FILM:
       // Red
@@ -97,7 +97,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.145f, 0.049f);
       // Whitepoint (Cf)
       primaries[3] = Xy2xyz(0.310f, 0.136f);
-      break;
+      return;
 
     case ColorSpace::PrimaryID::BT2020:
       // Red
@@ -108,7 +108,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.131f, 0.046f);
       // Whitepoint (D65f)
       primaries[3] = Xy2xyz(0.3127f, 0.3290f);
-      break;
+      return;
 
     case ColorSpace::PrimaryID::SMPTEST428_1:
       // X
@@ -119,7 +119,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.0f, 0.0f);
       // Whitepoint (Ef)
       primaries[3] = Xy2xyz(1.0f / 3.0f, 1.0f / 3.0f);
-      break;
+      return;
 
     case ColorSpace::PrimaryID::SMPTEST431_2:
       // Red
@@ -130,7 +130,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.150f, 0.060f);
       // Whitepoint
       primaries[3] = Xy2xyz(0.314f, 0.351f);
-      break;
+      return;
 
     case ColorSpace::PrimaryID::SMPTEST432_1:
       // Red
@@ -141,7 +141,7 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.150f, 0.060f);
       // Whitepoint (D65f)
       primaries[3] = Xy2xyz(0.3127f, 0.3290f);
-      break;
+      return;
 
     case ColorSpace::PrimaryID::XYZ_D50:
       // X
@@ -152,8 +152,17 @@ void GetPrimaries(ColorSpace::PrimaryID id,
       primaries[2] = Xy2xyz(0.0f, 0.0f);
       // D50
       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) {
@@ -191,20 +200,24 @@ GFX_EXPORT Transform GetPrimaryMatrix(ColorSpace::PrimaryID id) {
 
 GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float v) {
   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::SMPTE170M:
     case ColorSpace::TransferID::BT2020_10:
-    case ColorSpace::TransferID::BT2020_12: {
-      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);
-      }
-    }
+    case ColorSpace::TransferID::BT2020_12:
+      // BT709 is our "default" cause, so put the code after the switch
+      // to avoid "control reaches end of non-void function" errors.
+      break;
 
     case ColorSpace::TransferID::GAMMA22:
       v = fmax(0.0f, v);
@@ -305,24 +318,34 @@ GFX_EXPORT float FromLinear(ColorSpace::TransferID id, float v) {
       v = fmax(0.0f, v);
       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) {
   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::SMPTE170M:
     case ColorSpace::TransferID::BT2020_10:
-    case ColorSpace::TransferID::BT2020_12: {
-      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);
-      }
-    }
+    case ColorSpace::TransferID::BT2020_12:
+      // BT709 is our "default" cause, so put the code after the switch
+      // to avoid "control reaches end of non-void function" errors.
+      break;
 
     case ColorSpace::TransferID::GAMMA22:
       v = fmax(0.0f, v);
@@ -432,6 +455,15 @@ GFX_EXPORT float ToLinear(ColorSpace::TransferID id, float v) {
       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 {
@@ -461,7 +493,9 @@ GFX_EXPORT ColorTransform::TriStim ToLinear(ColorSpace::TransferID 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) {
     case ColorSpace::MatrixID::RGB:
       return Transform();
@@ -469,8 +503,8 @@ GFX_EXPORT Transform GetTransferMatrix(ColorSpace::MatrixID id) {
     case ColorSpace::MatrixID::BT709:
     case ColorSpace::MatrixID::UNSPECIFIED:
     case ColorSpace::MatrixID::RESERVED:
-      Kr = 0.2126f;
-      Kb = 0.0722f;
+    case ColorSpace::MatrixID::UNKNOWN:
+      // Default values are already set.
       break;
 
     case ColorSpace::MatrixID::FCC:
@@ -544,6 +578,7 @@ Transform GetRangeAdjustMatrix(ColorSpace::RangeID range,
     case ColorSpace::MatrixID::BT2020_NCL:
     case ColorSpace::MatrixID::BT2020_CL:
     case ColorSpace::MatrixID::YDZDX:
+    case ColorSpace::MatrixID::UNKNOWN:
       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, 0.0f, 255.0f / 224.0f, -15.5f / 224.0f,  // 3