media: Add Rappor reporting in MediaLog

media/filters/gpu_video_decoder.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 "media/filters/gpu_video_decoder.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/cpu.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "gpu/command_buffer/common/mailbox_holder.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/cdm_context.h"
 #include "media/base/decoder_buffer.h"
+#include "media/base/media_log.h"
 #include "media/base/media_switches.h"
 #include "media/base/pipeline_status.h"
 #include "media/base/surface_manager.h"
 #include "media/base/video_decoder_config.h"
 #include "media/renderers/gpu_video_accelerator_factories.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 namespace media {
 
 const char GpuVideoDecoder::kDecoderName[] = "GpuVideoDecoder";
@@ skipping 30 matching lines @@
                                         const gfx::Rect& vr,
                                         const gfx::Size& ns)
     : bitstream_buffer_id(bbid),
       timestamp(ts),
       visible_rect(vr),
       natural_size(ns) {}
 
 GpuVideoDecoder::BufferData::~BufferData() {}
 
 GpuVideoDecoder::GpuVideoDecoder(GpuVideoAcceleratorFactories* factories,
-                                 const RequestSurfaceCB& request_surface_cb)
+                                 const RequestSurfaceCB& request_surface_cb,
+                                 scoped_refptr<MediaLog> media_log)
     : needs_bitstream_conversion_(false),
       factories_(factories),
+      request_surface_cb_(request_surface_cb),
+      media_log_(media_log),

[DaleCurtis, 2016/06/15 00:33:47]

Need to use std::move() if you drop const& I thought.

[xhwang, 2016/06/15 04:48:49]

https://www.chromium.org/developers/coding-style

If the function (at least sometimes) takes a ref on a refcounted object, declare
the param as scoped_refptr<T>. The caller can decide whether it wishes to
transfer ownership (by calling std::move(t) when passing t) or retain its ref
(by simply passing t directly).

So it's up to the caller to decide whether to use std::move(). In our case, the
caller is DefaultRendererFactory, which needs to keep it's own |media_log_| for
other uses, so the caller isn't using std::move either.

       state_(kNormal),
-      request_surface_cb_(request_surface_cb),
       decoder_texture_target_(0),
       pixel_format_(PIXEL_FORMAT_UNKNOWN),
       next_picture_buffer_id_(0),
       next_bitstream_buffer_id_(0),
       available_pictures_(0),
       needs_all_picture_buffers_to_decode_(false),
       supports_deferred_initialization_(false),
       weak_factory_(this) {
   DCHECK(factories_);
 }
@@ skipping 25 matching lines @@
                                  const gfx::Size& max_resolution) {
   return (coded_size.width() <= max_resolution.width() &&
       coded_size.height() <= max_resolution.height() &&
       coded_size.width() >= min_resolution.width() &&
       coded_size.height() >= min_resolution.height());
 }
 
 // Report |success| to UMA and run |cb| with it.  This is super-specific to the
 // UMA stat reported because the UMA_HISTOGRAM_ENUMERATION API requires a
 // callsite to always be called with the same stat name (can't parameterize it).
 static void ReportGpuVideoDecoderInitializeStatusToUMAAndRunCB(

[xhwang, 2016/06/15 00:19:07]

Today this is called for all init cases. When a failure is reported, it could be
due to a lot of reasons, e.g. config not supported (WebM), profile changed
during config change, and so on.

I feel the purpose of this UMA is to catch real VDA init failures (maybe I am
wrong). Does it make sense to have a separate UMA/Rappor that that case?

[DaleCurtis, 2016/06/15 00:33:47]

Well, the problem is we can't tell the difference between a bad configuration /
vda bug / and codec normally not supported... actually, we could rescan the
supported profiles and only log error_not_supported if the decoder should have
been supported?

[xhwang, 2016/06/15 04:48:49]

Yeah, I feel it's nicer if we can at least distinguish between not_supported and
VDA::Init( ) error, where we has passed all kinds of tests and assume the VDA
should support the config.

     const VideoDecoder::InitCB& cb,
+    scoped_refptr<MediaLog> media_log,
     bool success) {
   // TODO(xhwang): Report |success| directly.
   PipelineStatus status = success ? PIPELINE_OK : DECODER_ERROR_NOT_SUPPORTED;
   UMA_HISTOGRAM_ENUMERATION(
       "Media.GpuVideoDecoderInitializeStatus", status, PIPELINE_STATUS_MAX + 1);
+
+  if (!success) {
+    media_log->RecordRapporWithSecurityOrigin(
+        "Media.OriginUrl.GpuVideoDecoderInitFailure");
+  }
+
   cb.Run(success);
 }
 
 // static
 void ReleaseMailboxTrampoline(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
     const VideoFrame::ReleaseMailboxCB& release_mailbox_cb,
     const gpu::SyncToken& release_sync_token) {
   if (task_runner->BelongsToCurrentThread()) {
     release_mailbox_cb.Run(release_sync_token);
@@ skipping 12 matching lines @@
                                  bool /* low_delay */,
                                  CdmContext* cdm_context,
                                  const InitCB& init_cb,
                                  const OutputCB& output_cb) {
   DVLOG(3) << "Initialize()";
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   DCHECK(config.IsValidConfig());
 
   InitCB bound_init_cb =
       base::Bind(&ReportGpuVideoDecoderInitializeStatusToUMAAndRunCB,
-                 BindToCurrentLoop(init_cb));
+                 BindToCurrentLoop(init_cb), media_log_);
 
 #if !defined(OS_ANDROID)
   if (config.is_encrypted()) {
     DVLOG(1) << "Encrypted stream not supported.";
     bound_init_cb.Run(false);
     return;
   }
 #endif
 
   bool previously_initialized = config_.IsValidConfig();
   DVLOG(1) << (previously_initialized ? "Reinitializing" : "Initializing")
-           << "GVD with config: " << config.AsHumanReadableString();
+           << " GVD with config: " << config.AsHumanReadableString();
 
   // TODO(posciak): destroy and create a new VDA on codec/profile change
   // (http://crbug.com/260224).
   if (previously_initialized && (config_.profile() != config.profile())) {
     DVLOG(1) << "Codec or profile changed, cannot reinitialize.";
     bound_init_cb.Run(false);
     return;
   }
 
   VideoDecodeAccelerator::Capabilities capabilities =
@@ skipping 603 matching lines @@
   }
   return false;
 }
 
 void GpuVideoDecoder::DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent()
     const {
   DCHECK(factories_->GetTaskRunner()->BelongsToCurrentThread());
 }
 
 }  // namespace media