Add UMA reporting for RTCVideoDecoder.

content/renderer/media/rtc_video_decoder.cc

Index: content/renderer/media/rtc_video_decoder.cc
diff --git a/content/renderer/media/rtc_video_decoder.cc b/content/renderer/media/rtc_video_decoder.cc
index adce1f61b7e6dcb84bba726ce4050a797da031d0..875023599ea60507dad0c052d93855e8a9a40dc1 100644
--- a/content/renderer/media/rtc_video_decoder.cc
+++ b/content/renderer/media/rtc_video_decoder.cc
@@ -8,6 +8,7 @@
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/metrics/histogram.h"
 #include "base/safe_numerics.h"
 #include "base/stl_util.h"
 #include "base/task_runner_util.h"
@@ -155,13 +156,13 @@ int32_t RTCVideoDecoder::InitDecode(const webrtc::VideoCodec* codecSettings,
   DCHECK_EQ(codecSettings->codecType, webrtc::kVideoCodecVP8);
   if (codecSettings->codecSpecific.VP8.feedbackModeOn) {
     LOG(ERROR) << "Feedback mode not supported";
-    return WEBRTC_VIDEO_CODEC_ERROR;
+    return RecordInitDecodeUMA(WEBRTC_VIDEO_CODEC_ERROR);
   }
 
   base::AutoLock auto_lock(lock_);
   if (state_ == UNINITIALIZED || state_ == DECODE_ERROR) {
     LOG(ERROR) << "VDA is not initialized. state=" << state_;
-    return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
+    return RecordInitDecodeUMA(WEBRTC_VIDEO_CODEC_UNINITIALIZED);
   }
   // Create some shared memory if the queue is empty.
   if (available_shm_segments_.size() == 0) {
@@ -171,7 +172,7 @@ int32_t RTCVideoDecoder::InitDecode(const webrtc::VideoCodec* codecSettings,
                                          kMaxInFlightDecodes,
                                          kSharedMemorySegmentBytes));
   }
-  return WEBRTC_VIDEO_CODEC_OK;
+  return RecordInitDecodeUMA(WEBRTC_VIDEO_CODEC_OK);
 }
 
 int32_t RTCVideoDecoder::Decode(
@@ -461,6 +462,9 @@ void RTCVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
     return;
 
   LOG(ERROR) << "VDA Error:" << error;
+  UMA_HISTOGRAM_ENUMERATION("Media.RTCVideoDecoderError",
+                            error,
+                            media::VideoDecodeAccelerator::LARGEST_ERROR_ENUM);
   DestroyVDA();
 
   base::AutoLock auto_lock(lock_);
@@ -747,4 +751,10 @@ void RTCVideoDecoder::GetBufferData(int32 bitstream_buffer_id,
   NOTREACHED() << "Missing bitstream buffer id: " << bitstream_buffer_id;
 }
 
+int32_t RTCVideoDecoder::RecordInitDecodeUMA(int32_t status) {
+  bool sample = (status == WEBRTC_VIDEO_CODEC_OK) ? true : false;

[Alexei Svitkine (slow), 2013/09/13 14:53:25]

Why not log the actual status as an enum rather than just whether it's an error
or not?

[wuchengli, 2013/09/14 01:40:17]

Success case is more important than error. Most of the time people want to know
if HW or SW has been used for decoding. They can go to chrome://histograms to
check it. Also, InitDecode implementation is very light weight. It's less likely
to report error there. I logged the false case for consistency. Another use case
is that people can see how many times HW decoding is used during a period of
time.

Several ways I've considered:
- Log only the success case of InitDecode.
- Log only when RTCVideoDecoder is successfully created. Fail case is not very
important because most time it fails because HW doesn't support the codec.
- Log the enum of InitDecode. This is fine and just needs more code.
Let me know if you think the current patchset is ok or you prefer another way.

What will happen if one change the type of histogram from boolean to enum after
it runs for some time? Will UMA dashboard be a mess?

[Alexei Svitkine (slow), 2013/09/14 02:37:27]

Okay, that sounds reasonable. Might be worth adding a short comment to that
extent about it.

On 2013/09/14 01:40:17, wuchengli wrote:
A boolean is simply an enum with values 0 and 1. If you keep the same values and
change it to an enum, adding let's say a value 2, it will be compatible with the
old version.

+  UMA_HISTOGRAM_BOOLEAN("Media.RTCVideoDecoderInitDecodeStatus", sample);
+  return status;
+}
+
 }  // namespace content