VideoCapture: Sort VideoPixelFormat in order of preference

content/browser/renderer_host/media/video_capture_controller.cc

Index: content/browser/renderer_host/media/video_capture_controller.cc
diff --git a/content/browser/renderer_host/media/video_capture_controller.cc b/content/browser/renderer_host/media/video_capture_controller.cc
index 07c9ca141088b9f7d9ad7fd968277409b5767a59..ab90665a35860dfa4e441cf2cbc643a09effe45c 100644
--- a/content/browser/renderer_host/media/video_capture_controller.cc
+++ b/content/browser/renderer_host/media/video_capture_controller.cc
@@ -90,6 +90,49 @@ void ReturnVideoFrame(const scoped_refptr<media::VideoFrame>& video_frame,
   video_frame->UpdateReleaseSyncPoint(&client);
 }
 
+// Get the correct UMA int label for the pixel format.
+static int GetUmaLabel(media::VideoPixelFormat format) {

[mcasas, 2014/10/19 13:13:14]

const media::VideoPixelFormat format ?

[magjed_chromium, 2014/10/19 13:30:16]

I haven't seen anyone else make a const non-ref, I have only seen ints and enums
being passed as non-const values.

[mcasas, 2014/10/20 09:06:19]

Ah, true, VPF is an enum. That's the evil with typedef.
Your call.

+  // This enum must be in sync with CapturePixelFormat defined in
+  // histograms.xml.
+  enum UmaPixelFormat {
+    UNKNOWN,
+    I420,
+    YUY2,
+    UYVY,
+    RGB24,
+    ARGB,
+    MJPEG,
+    NV21,
+    YV12,
+    TEXTURE
+  };
+  switch (format) {

[mcasas, 2014/10/20 09:06:19]

Paraphrasing tommi@, I'd like to see this data driven instead of
multiple conditionals:

struct {
  VideoPixelFormat pixel_format;
  UmaPixelFormat uma_pixel_format;
} pixel_formats[] = {
  { PIXEL_FORMAT_UNKNOWN, UNKNOWN },
  { PIXEL_FORMAT_I420, I420 },
  ...
};

for (size_t i = 0; i < ARRAYSIZE_UNSAFE(pixel_formats); ++i) {
  if (format == pixel_formats[i].pixel_format)
    return pixel_formats[i].uma_pixel_format;
}

So things don't need to be updated in two places.

+    case media::PIXEL_FORMAT_I420:
+      return I420;
+    case media::PIXEL_FORMAT_YV12:
+      return YV12;
+    case media::PIXEL_FORMAT_NV21:
+      return NV21;
+    case media::PIXEL_FORMAT_UYVY:
+      return UYVY;
+    case media::PIXEL_FORMAT_YUY2:
+      return YUY2;
+    case media::PIXEL_FORMAT_RGB24:
+      return RGB24;
+    case media::PIXEL_FORMAT_ARGB:
+      return ARGB;
+    case media::PIXEL_FORMAT_MJPEG:
+      return MJPEG;
+    case media::PIXEL_FORMAT_TEXTURE:
+      return TEXTURE;
+    case media::PIXEL_FORMAT_UNKNOWN:
+      return UNKNOWN;
+    default:
+      NOTIMPLEMENTED();
+      return 0;
+  };
+}
+
 }  // anonymous namespace
 
 struct VideoCaptureController::ControllerClient {
@@ -637,7 +680,7 @@ void VideoCaptureController::DoIncomingCapturedVideoFrameOnIOThread(
     UMA_HISTOGRAM_COUNTS("Media.VideoCapture.FrameRate",
                          buffer_format.frame_rate);
     UMA_HISTOGRAM_ENUMERATION("Media.VideoCapture.PixelFormat",
-                              buffer_format.pixel_format,
+                              GetUmaLabel(buffer_format.pixel_format),
                               media::PIXEL_FORMAT_MAX);
     has_received_frames_ = true;
   }