Refactoring of media::HDRMetadata struct

media/base/hdr_metadata.h

Index: media/base/hdr_metadata.h
diff --git a/media/base/hdr_metadata.h b/media/base/hdr_metadata.h
index 78a17756ca8665f889e46ef73bd635300eea79a7..ecb7632d4cf7993d34f86cc10987394f7cb88202 100644
--- a/media/base/hdr_metadata.h
+++ b/media/base/hdr_metadata.h
@@ -6,19 +6,16 @@
 #define MEDIA_BASE_HDR_METADATA_H_
 
 #include "media/base/media_export.h"
+#include "ui/gfx/geometry/point_f.h"
 
 namespace media {
 
 // SMPTE ST 2086 mastering metadata.
 struct MEDIA_EXPORT MasteringMetadata {
-  float primary_r_chromaticity_x = 0;
-  float primary_r_chromaticity_y = 0;
-  float primary_g_chromaticity_x = 0;
-  float primary_g_chromaticity_y = 0;
-  float primary_b_chromaticity_x = 0;
-  float primary_b_chromaticity_y = 0;
-  float white_point_chromaticity_x = 0;
-  float white_point_chromaticity_y = 0;
+  gfx::PointF primary_r_chromaticity;
+  gfx::PointF primary_g_chromaticity;
+  gfx::PointF primary_b_chromaticity;
+  gfx::PointF white_point_chromaticity;

[xhwang, 2017/04/07 18:24:59]

I don't feel we should depend on gfx for chromaticity. It's essentially the same
type (a pair of floats), but it's a very different concept. It's a way to define
a color, not a point in a 2D UI.

Also, if you have a Chromaticity type, you don't need the "chromaticity" suffix
for each member variable.

[hubbe, 2017/04/07 18:26:51]

Do you think it would be enough to typedef gfx::PointF ?

[servolk, 2017/04/07 18:30:52]

Well, chromaticities ARE actually 2D points in color space, describing
coordinates of primaries and white points. Is it worth introducing a separate
struct in this case? If we create a new struct, we'll have to add separate
operator== and Mojo definitions (StructTraits, etc) for the new struct. Seems
like too much boilerplate, don't you think?

[servolk, 2017/04/07 18:30:53]

Yes, I guess a typedef gfx::PointF might work. Xiaohan, WDYT?

   float luminance_max = 0;
   float luminance_min = 0;
 
@@ -26,14 +23,10 @@ struct MEDIA_EXPORT MasteringMetadata {
   MasteringMetadata(const MasteringMetadata& rhs);
 
   bool operator==(const MasteringMetadata& rhs) const {
-    return ((primary_r_chromaticity_x == rhs.primary_r_chromaticity_x) &&
-            (primary_r_chromaticity_y == rhs.primary_r_chromaticity_y) &&
-            (primary_g_chromaticity_x == rhs.primary_g_chromaticity_x) &&
-            (primary_g_chromaticity_y == rhs.primary_g_chromaticity_y) &&
-            (primary_b_chromaticity_x == rhs.primary_b_chromaticity_x) &&
-            (primary_b_chromaticity_y == rhs.primary_b_chromaticity_y) &&
-            (white_point_chromaticity_x == rhs.white_point_chromaticity_x) &&
-            (white_point_chromaticity_y == rhs.white_point_chromaticity_y) &&
+    return ((primary_r_chromaticity == rhs.primary_r_chromaticity) &&
+            (primary_g_chromaticity == rhs.primary_g_chromaticity) &&
+            (primary_b_chromaticity == rhs.primary_b_chromaticity) &&
+            (white_point_chromaticity == rhs.white_point_chromaticity) &&
             (luminance_max == rhs.luminance_max) &&
             (luminance_min == rhs.luminance_min));
   }
@@ -44,17 +37,19 @@ struct MEDIA_EXPORT HDRMetadata {
   MasteringMetadata mastering_metadata;
   // Max content light level (CLL), i.e. maximum brightness level present in the
   // stream), in nits.
-  unsigned max_cll = 0;
+  unsigned max_content_light_level = 0;
   // Max frame-average light level (FALL), i.e. maximum average brightness of
   // the brightest frame in the stream), in nits.
-  unsigned max_fall = 0;
+  unsigned max_frame_average_light_level = 0;
 
   HDRMetadata();
   HDRMetadata(const HDRMetadata& rhs);
 
   bool operator==(const HDRMetadata& rhs) const {
-    return ((max_cll == rhs.max_cll) && (max_fall == rhs.max_fall) &&
-            (mastering_metadata == rhs.mastering_metadata));
+    return (
+        (max_content_light_level == rhs.max_content_light_level) &&
+        (max_frame_average_light_level == rhs.max_frame_average_light_level) &&
+        (mastering_metadata == rhs.mastering_metadata));
   }
 };