Add PipelineStatus UMA logging to media_internals

content/browser/media/media_internals.cc

Index: content/browser/media/media_internals.cc
diff --git a/content/browser/media/media_internals.cc b/content/browser/media/media_internals.cc
index 4d600851783452b708edd2a04cb59edf7c2397e6..b59aafae120dbb3ff27b76ab1ebf940cd8eb4931 100644
--- a/content/browser/media/media_internals.cc
+++ b/content/browser/media/media_internals.cc
@@ -4,13 +4,16 @@
 
 #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_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.h"
 #include "media/base/media_log_event.h"
 
 namespace {
@@ -174,9 +177,37 @@ MediaInternals* MediaInternals::GetInstance() {
   return g_media_internals.Pointer();
 }
 
-MediaInternals::MediaInternals() : owner_ids_() {}
+MediaInternals::MediaInternals() : owner_ids_() {
+  registrar_.Add(this,
+                 NOTIFICATION_RENDERER_PROCESS_TERMINATED,
+                 NotificationService::AllBrowserContextsAndSources());
+}
+
 MediaInternals::~MediaInternals() {}
 
+void MediaInternals::Observe(int type,
+                             const NotificationSource& source,
+                             const NotificationDetails& details) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(type, NOTIFICATION_RENDERER_PROCESS_TERMINATED);
+  RenderProcessHost* process = Source<RenderProcessHost>(source).ptr();
+  LogAndClearPlayersInRenderer(process->GetID());
+}
+
+void MediaInternals::LogAndClearPlayersInRenderer(int render_process_id) {
+  auto players_it = renderer_info.find(render_process_id);
+  if (players_it == renderer_info.end())
+    return;
+
+  auto it = players_it->second.begin();
+  while (it != players_it->second.end()) {
+    LogUMAForPipelineStatus(*(it->second));
+    //delete memory allocated for PipelineStatus
+    delete it->second;
+    players_it->second.erase(it++);
+  }
+}
+
 void MediaInternals::OnMediaEvents(
     int render_process_id, const std::vector<media::MediaLogEvent>& events) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -195,6 +226,95 @@ void MediaInternals::OnMediaEvents(
     dict.SetDouble("ticksMillis", ticks_millis);
     dict.Set("params", event->params.DeepCopy());
     SendUpdate(SerializeUpdate("media.onMediaEvent", &dict));
+    SavePlayerState(*event, render_process_id);
+  }
+}
+
+void MediaInternals::SavePlayerState(const media::MediaLogEvent& event,
+                                     int render_process_id) {
+  PlayerInfoMap& player_info = renderer_info[render_process_id];
+  switch (event.type) {
+    case media::MediaLogEvent::WEBMEDIAPLAYER_CREATED: {
+      auto it = player_info.find(event.id);
+      if (it == player_info.end()) {
+        player_info[event.id] = new PipelineInfo();
+      }
+      break;
+    }
+    case media::MediaLogEvent::PIPELINE_ERROR: {
+      std::string status = "";

[DaleCurtis, 2014/11/06 23:49:08]

No need for =""

[prabhur1, 2014/11/07 23:43:23]

Done.

+      event.params.GetString("pipeline_error", &status);
+      media::MediaLog::StringToPipelineStatus(
+          status, player_info[event.id]->last_pipeline_status);
+      break;
+    }
+    case media::MediaLogEvent::PROPERTY_CHANGE:
+      if (event.params.HasKey("found_audio_stream")) {
+        event.params.GetBoolean("found_audio_stream",
+                                &player_info[event.id]->has_audio);
+      }
+      if (event.params.HasKey("found_video_stream")) {
+        event.params.GetBoolean("found_video_stream",
+                                &player_info[event.id]->has_video);
+      }
+      if (event.params.HasKey("audio_codec_name")) {
+        event.params.GetString("audio_codec_name",
+                               &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);

[prabhur1, 2014/11/06 21:17:51]

Just noticed the formatting. Will fix it in the next iteration.

[prabhur1, 2014/11/07 23:43:23]

Done.

+      }
+      break;
+    default:
+      break;
+  }
+  return;
+}
+
+void MediaInternals::LogUMAForPipelineStatus(const PipelineInfo& player_info) {
+  if (player_info.has_video && player_info.has_audio) {
+    if (player_info.video_codec_name == "vp8") {
+      UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.VP8",
+                                player_info.last_pipeline_status,
+                                media::PIPELINE_STATUS_MAX + 1);
+    } else if (player_info.video_codec_name == "vp9") {

[prabhur1, 2014/11/06 21:17:51]

This code is not getting hit while playing a vp9 video. It is because
media_internals is not getting the corresponding "video_decode_name" event.
Possibly a bug ?

[DaleCurtis, 2014/11/06 23:49:08]

Yeah sounds like it, can you fix or file a bug?

[prabhur1, 2014/11/07 23:43:23]

Assigned to me : https://code.google.com/p/chromium/issues/detail?id=431443

Done.

+      UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.VP9",
+                                player_info.last_pipeline_status,
+                                media::PIPELINE_STATUS_MAX + 1);
+    } else if (player_info.video_codec_name == "h264") {
+
+      if (player_info.video_decoder == "gpu") {
+        UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.HW.H264",
+                                  player_info.last_pipeline_status,
+                                  media::PIPELINE_STATUS_MAX + 1);
+      }
+      else {
+      UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo.SW.H264",
+                                player_info.last_pipeline_status,
+                                media::PIPELINE_STATUS_MAX + 1);
+      }
+    } else {
+      UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.AudioVideo",
+                                player_info.last_pipeline_status,
+                                media::PIPELINE_STATUS_MAX + 1);
+    }
+  } else if (player_info.has_audio) {
+    UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.Audio",
+                              player_info.last_pipeline_status,
+                              media::PIPELINE_STATUS_MAX + 1);
+  } else if (player_info.has_video) {
+    UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.Video",
+                              player_info.last_pipeline_status,
+                              media::PIPELINE_STATUS_MAX + 1);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("Media.PipelineStatus.Unsupported",

[prabhur1, 2014/11/06 21:17:51]

Youtube videos end up showing as Unsupported because a lot of media_log events
are not received. Could be because those videos use MSE and chunk_demuxer doesnt
currently have any media_log eventing enabled.
Should the chunk demuxer do the same thing as ffmegdemuxer - w.r.t sending
media_log events ?

[DaleCurtis, 2014/11/06 23:49:08]

Hmm, yeah it looks like MSE is sending the raw video codec and audio codec IDs
through error strings. :| Probably we should add some code to MSE to log the
codec_ids as reported to AudioDecoderConfig/VideoDecoderConfig.

[prabhur1, 2014/11/07 23:43:23]

Assigned to me: https://code.google.com/p/chromium/issues/detail?id=431447

[prabhur1, 2014/11/07 23:43:23]

Done.

+                              player_info.last_pipeline_status,
+                              media::PIPELINE_STATUS_MAX + 1);
   }
 }