Add UMA metrics for VideoCodec.MP4 and VideoCodec.WebM

media/filters/ffmpeg_glue.cc

Index: media/filters/ffmpeg_glue.cc
diff --git a/media/filters/ffmpeg_glue.cc b/media/filters/ffmpeg_glue.cc
index c1a2145d4c1f343635b884861d9a45c8ad0f0eb7..bcc4df00d2452eb0473ea4141460531dd398df2f 100644
--- a/media/filters/ffmpeg_glue.cc
+++ b/media/filters/ffmpeg_glue.cc
@@ -100,8 +100,7 @@ void FFmpegGlue::InitializeFFmpeg() {
   CHECK(initialized);
 }
 
-FFmpegGlue::FFmpegGlue(FFmpegURLProtocol* protocol)
-    : open_called_(false) {
+FFmpegGlue::FFmpegGlue(FFmpegURLProtocol* protocol) {
   InitializeFFmpeg();
 
   // Initialize an AVIOContext using our custom read and seek operations.  Don't
@@ -157,41 +156,40 @@ bool FFmpegGlue::OpenContext() {
     if (num_read < container_names::kMinimumContainerSize)
       return false;
 
-    UMA_HISTOGRAM_SPARSE_SLOWLY(
-        "Media.DetectedContainer",
-        container_names::DetermineContainer(buffer.data(), num_read));
+    container_ = container_names::DetermineContainer(buffer.data(), num_read);
+    UMA_HISTOGRAM_SPARSE_SLOWLY("Media.DetectedContainer", container_);
     return false;
   } else if (ret < 0) {
     return false;
   }
 
   // Rely on ffmpeg's parsing if we're able to succesfully open the file.
-  container_names::MediaContainerName container =
-      container_names::CONTAINER_UNKNOWN;
   if (strcmp(format_context_->iformat->name, "mov,mp4,m4a,3gp,3g2,mj2") == 0)
-    container = container_names::CONTAINER_MOV;
+    container_ = container_names::CONTAINER_MOV;
   else if (strcmp(format_context_->iformat->name, "flac") == 0)
-    container = container_names::CONTAINER_FLAC;
+    container_ = container_names::CONTAINER_FLAC;
   else if (strcmp(format_context_->iformat->name, "matroska,webm") == 0)
-    container = container_names::CONTAINER_WEBM;
+    container_ = container_names::CONTAINER_WEBM;
   else if (strcmp(format_context_->iformat->name, "ogg") == 0)
-    container = container_names::CONTAINER_OGG;
+    container_ = container_names::CONTAINER_OGG;
   else if (strcmp(format_context_->iformat->name, "wav") == 0)
-    container = container_names::CONTAINER_WAV;
+    container_ = container_names::CONTAINER_WAV;
   else if (strcmp(format_context_->iformat->name, "aac") == 0)
-    container = container_names::CONTAINER_AAC;
+    container_ = container_names::CONTAINER_AAC;
   else if (strcmp(format_context_->iformat->name, "mp3") == 0)
-    container = container_names::CONTAINER_MP3;
+    container_ = container_names::CONTAINER_MP3;
   else if (strcmp(format_context_->iformat->name, "amr") == 0)
-    container = container_names::CONTAINER_AMR;
+    container_ = container_names::CONTAINER_AMR;
   else if (strcmp(format_context_->iformat->name, "avi") == 0)
-    container = container_names::CONTAINER_AVI;
+    container_ = container_names::CONTAINER_AVI;
   // TODO(jrummell): Remove GSM detection. http://crbug.com/711774
   else if (strcmp(format_context_->iformat->name, "gsm") == 0)
-    container = container_names::CONTAINER_GSM;
+    container_ = container_names::CONTAINER_GSM;
+  else

[DaleCurtis, 2017/04/27 21:51:02]

No need since default is unknown?

[kqyang, 2017/04/27 22:04:57]

Sure.

(I am more comfortable to have an extra guarantee. I wasn't sure how FfmpegGlue
would be used, e.g. would the same object be used on multiple different files?)

[DaleCurtis, 2017/04/27 22:10:49]

This can only be called once and UNKNOWN should never be a valid value here.

+    container_ = container_names::CONTAINER_UNKNOWN;
 
-  DCHECK_NE(container, container_names::CONTAINER_UNKNOWN);
-  UMA_HISTOGRAM_SPARSE_SLOWLY("Media.DetectedContainer", container);
+  DCHECK_NE(container_, container_names::CONTAINER_UNKNOWN);
+  UMA_HISTOGRAM_SPARSE_SLOWLY("Media.DetectedContainer", container_);
 
   return true;
 }