Add dedicated PipelineStatus UMAs for encrypted playback

content/browser/media/media_internals.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/media/media_internals.h"
 
 #include "base/metrics/histogram.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_ui.h"
 #include "media/audio/audio_parameters.h"
 #include "media/base/media_log_event.h"
+#include "media/filters/decrypting_video_decoder.h"
 #include "media/filters/gpu_video_decoder.h"
 
 namespace {
 
 static base::LazyInstance<content::MediaInternals>::Leaky g_media_internals =
     LAZY_INSTANCE_INITIALIZER;
 
 base::string16 SerializeUpdate(const std::string& function,
                                const base::Value* value) {
   return content::WebUI::GetJavascriptCall(
@@ skipping 161 matching lines @@
 
   // Helper function to save the event payload to RendererPlayerMap.
   void SavePlayerState(const media::MediaLogEvent& event,
                        int render_process_id);
 
  private:
   struct PipelineInfo {
     media::PipelineStatus last_pipeline_status;
     bool has_audio;
     bool has_video;
+    bool video_dds;
+    bool audio_dds;

[xhwang, 2014/12/03 21:39:50]

nit: typically audio goes before video

[prabhur1, 2014/12/03 23:24:31]

Done.

     std::string audio_codec_name;
     std::string video_codec_name;
     std::string video_decoder;
     PipelineInfo()
         : last_pipeline_status(media::PIPELINE_OK),
           has_audio(false),
-          has_video(false) {}
+          has_video(false),
+          video_dds(false),
+          audio_dds(false) {}
   };
 
   // Helper function to report PipelineStatus associated with a player to UMA.
   void ReportUMAForPipelineStatus(const PipelineInfo& player_info);
 
+  // Helper to generate pipelinestatus_UMA_prefix for AudioVideo streams.
+  std::string GetUMANameForAVStream(const PipelineInfo& player_info);
+
   // Key is playerid
   typedef std::map<int, PipelineInfo> PlayerInfoMap;
 
   // Key is renderer id
   typedef std::map<int, PlayerInfoMap> RendererPlayerMap;
 
   // Stores player information per renderer
   RendererPlayerMap renderer_info_;
 
   NotificationRegistrar registrar_;
@@ skipping 56 matching lines @@
                                &player_info[event.id].audio_codec_name);
       }
       if (event.params.HasKey("video_codec_name")) {
         event.params.GetString("video_codec_name",
                                &player_info[event.id].video_codec_name);
       }
       if (event.params.HasKey("video_decoder")) {
         event.params.GetString("video_decoder",
                                &player_info[event.id].video_decoder);
       }
+      if (event.params.HasKey("video_dds")) {
+        event.params.GetBoolean("video_dds", &player_info[event.id].video_dds);
+      }
+      if (event.params.HasKey("audio_dds")) {
+        event.params.GetBoolean("audio_dds", &player_info[event.id].audio_dds);
+      }
       break;
     default:
       break;
   }
   return;
 }
 
+std::string MediaInternals::MediaInternalsUMAHandler::GetUMANameForAVStream(
+    const PipelineInfo& player_info) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  std::string pipeline_uma_prefix = "Media.PipelineStatus.AudioVideo.";
+  if (player_info.video_codec_name == "vp8") {
+    pipeline_uma_prefix += "VP8.";
+  } else if (player_info.video_codec_name == "vp9") {
+    pipeline_uma_prefix += "VP9.";
+  } else if (player_info.video_codec_name == "h264") {
+    pipeline_uma_prefix += "H264.";
+  } else {
+    return pipeline_uma_prefix + "Other";
+  }
+  if (player_info.video_decoder ==
+      media::DecryptingVideoDecoder::kDecoderName) {
+    pipeline_uma_prefix += "DVD";
+  } else if (player_info.video_dds || player_info.audio_dds) {

[xhwang, 2014/12/03 21:39:49]

Now AudioVideo.VP8.DDS could be because we use audio DDS, which will be very
confusing. Since we are not checking DecryptingAudioDecoder(DAD) either, I
suggest we don't check audio_dds as well.

[prabhur1, 2014/12/03 23:24:31]

Done.

+    pipeline_uma_prefix += "DDS";

[xhwang, 2014/12/03 21:39:50]

When we use DDS, we are not checking HW vs SW anymore. Actually it'll be
interesting to check how often we use DDS with SW. This should be rare, but if
we see that a lot, then something is broken.

So is it possible we could have?
codec.HW
codec.SW
codec.DVD
codec.DDS.HW
codec.DDS.SW

[prabhur1, 2014/12/03 23:24:31]

Done.

+  } else if (player_info.video_decoder ==
+             media::GpuVideoDecoder::kDecoderName) {
+    pipeline_uma_prefix += "HW";
+  } else {
+    pipeline_uma_prefix += "SW";
+  }
+  return pipeline_uma_prefix;

[xhwang, 2014/12/03 21:39:50]

Returning a uma prefix seems weird. How about something like:

static const char kPipelineUmaPrefix[] = "Media.PipelineStatus.AudioVideo.";

std::string uma_name = kPipelineUmaPrefix;
uma_name += ...

return uma_name;

[prabhur1, 2014/12/03 23:24:31]

Done.

+}
+
 void MediaInternals::MediaInternalsUMAHandler::ReportUMAForPipelineStatus(
     const PipelineInfo& player_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (player_info.has_video && player_info.has_audio) {
-    if (player_info.video_codec_name == "vp8") {
-      UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.VP8.SW",
-                                player_info.last_pipeline_status,
-                                media::PIPELINE_STATUS_MAX + 1);
-    } else if (player_info.video_codec_name == "vp9") {
-      UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.VP9.SW",
-                                player_info.last_pipeline_status,
-                                media::PIPELINE_STATUS_MAX + 1);
-    } else if (player_info.video_codec_name == "h264") {
-      if (player_info.video_decoder == media::GpuVideoDecoder::kDecoderName) {
-        UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.H264.HW",
-                                  player_info.last_pipeline_status,
-                                  media::PIPELINE_STATUS_MAX + 1);
-      } else {
-        UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.H264.SW",
-                                  player_info.last_pipeline_status,
-                                  media::PIPELINE_STATUS_MAX + 1);
-      }
-    } else {
-      UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.Other",
-                                player_info.last_pipeline_status,
-                                media::PIPELINE_STATUS_MAX + 1);
-    }
+    base::LinearHistogram::FactoryGet(

[DaleCurtis, 2014/12/02 23:21:17]

Is this the (new?) hotness for dynamic UMA names?

[prabhur1, 2014/12/03 02:04:32]

Done.

[prabhur1, 2014/12/03 02:04:32]

Yep. I think this API has been around for sometime.

+        GetUMANameForAVStream(player_info), media::PIPELINE_OK,

[xhwang, 2014/12/03 21:39:50]

media::PIPELINE_OK is 0, which is not recommended. This is a common pitfall in
the base::LinearHistogram::FactoryGet():

https://code.google.com/p/chromium/codesearch#chromium/src/base/metrics/histo...

https://code.google.com/p/chromium/codesearch#chromium/src/base/metrics/histo...

[prabhur1, 2014/12/03 23:24:31]

Done.

+        media::PIPELINE_STATUS_MAX, media::PIPELINE_STATUS_MAX + 1,
+        base::HistogramBase::kUmaTargetedHistogramFlag)
+        ->Add(player_info.last_pipeline_status);
   } else if (player_info.has_audio) {
     UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioOnly",
                               player_info.last_pipeline_status,
                               media::PIPELINE_STATUS_MAX + 1);
   } else if (player_info.has_video) {
     UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.VideoOnly",
                               player_info.last_pipeline_status,
                               media::PIPELINE_STATUS_MAX + 1);
   } else {
     UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.Unsupported",
@@ skipping 159 matching lines @@
     const std::string& function,
     const base::DictionaryValue* value) {
   SendUpdate(SerializeUpdate(function, value));
 
   base::AutoLock auto_lock(lock_);
   scoped_ptr<base::Value> out_value;
   CHECK(audio_streams_cached_data_.Remove(cache_key, &out_value));
 }
 
 }  // namespace content