VideoCapture: Sort VideoPixelFormat in order of preference

content/browser/renderer_host/media/video_capture_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/media/video_capture_controller.h"
 
 #include <map>
 #include <set>
 
 #include "base/bind.h"
@@ skipping 72 matching lines @@
   GLHelper* gl_helper = ImageTransportFactory::GetInstance()->GetGLHelper();
 #endif
   DCHECK(gl_helper);
   // UpdateReleaseSyncPoint() creates a new sync_point using |gl_helper|, so
   // wait the given |sync_point| using |gl_helper|.
   gl_helper->WaitSyncPoint(sync_point);
   SyncPointClientImpl client(gl_helper);
   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 {
   ControllerClient(const VideoCaptureControllerID& id,
                    VideoCaptureControllerEventHandler* handler,
                    base::ProcessHandle render_process,
                    media::VideoCaptureSessionId session_id,
                    const media::VideoCaptureParams& params)
       : controller_id(id),
         event_handler(handler),
@@ skipping 527 matching lines @@
     UMA_HISTOGRAM_COUNTS("Media.VideoCapture.Width",
                          buffer_format.frame_size.width());
     UMA_HISTOGRAM_COUNTS("Media.VideoCapture.Height",
                          buffer_format.frame_size.height());
     UMA_HISTOGRAM_ASPECT_RATIO("Media.VideoCapture.AspectRatio",
                                buffer_format.frame_size.width(),
                                buffer_format.frame_size.height());
     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;
   }
 
   buffer_pool_->HoldForConsumers(buffer->id(), count);
 }
 
 void VideoCaptureController::DoErrorOnIOThread() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   state_ = VIDEO_CAPTURE_STATE_ERROR;
@@ skipping 62 matching lines @@
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   int active_client_count = 0;
   for (ControllerClient* client : controller_clients_) {
     if (!client->paused)
       ++active_client_count;
   }
   return active_client_count;
 }
 
 }  // namespace content