Add UMA reporting for RTCVideoDecoder.

content/renderer/media/rtc_video_decoder.cc

 // Copyright 2013 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/renderer/media/rtc_video_decoder.h"
 
 #include "base/bind.h"
 #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"
 #include "content/child/child_thread.h"
 #include "content/renderer/media/native_handle_impl.h"
 #include "media/base/bind_to_loop.h"
 #include "media/filters/gpu_video_accelerator_factories.h"
 #include "third_party/webrtc/common_video/interface/texture_video_frame.h"
 #include "third_party/webrtc/system_wrappers/interface/ref_count.h"
 
@@ skipping 127 matching lines @@
   }
   return decoder.Pass();
 }
 
 int32_t RTCVideoDecoder::InitDecode(const webrtc::VideoCodec* codecSettings,
                                     int32_t /*numberOfCores*/) {
   DVLOG(2) << "InitDecode";
   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) {
     vda_loop_proxy_->PostTask(FROM_HERE,
                               base::Bind(&RTCVideoDecoder::CreateSHM,
                                          weak_this_,
                                          kMaxInFlightDecodes,
                                          kSharedMemorySegmentBytes));
   }
-  return WEBRTC_VIDEO_CODEC_OK;
+  return RecordInitDecodeUMA(WEBRTC_VIDEO_CODEC_OK);
 }
 
 int32_t RTCVideoDecoder::Decode(
     const webrtc::EncodedImage& inputImage,
     bool missingFrames,
     const webrtc::RTPFragmentationHeader* /*fragmentation*/,
     const webrtc::CodecSpecificInfo* /*codecSpecificInfo*/,
     int64_t /*renderTimeMs*/) {
   DVLOG(3) << "Decode";
 
@@ skipping 269 matching lines @@
   // Send the pending buffers for decoding.
   RequestBufferDecode();
 }
 
 void RTCVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
   DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
   if (!vda_)
     return;
 
   LOG(ERROR) << "VDA Error:" << error;
+  UMA_HISTOGRAM_ENUMERATION("Media.RTCVideoDecoderError",
+                            error,
+                            media::VideoDecodeAccelerator::LARGEST_ERROR_ENUM);
   DestroyVDA();
 
   base::AutoLock auto_lock(lock_);
   state_ = DECODE_ERROR;
 }
 
 void RTCVideoDecoder::WillDestroyCurrentMessageLoop() {
   DVLOG(2) << "WillDestroyCurrentMessageLoop";
   DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
   factories_->Abort();
@@ skipping 266 matching lines @@
     if (it->bitstream_buffer_id != bitstream_buffer_id)
       continue;
     *timestamp = it->timestamp;
     *width = it->width;
     *height = it->height;
     return;
   }
   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