Resolve media/cast crypto crash, and add crash reporting.

BUG=519022

Review URL: https://codereview.chromium.org/1417873007

Cr-Commit-Position: refs/heads/master@{#357027}

chrome/common/crash_keys.cc

@@ -80,6 +80,8 @@ const char kKaskoEquivalentGuid[] = "kasko-equivalent-guid";
 
 const char kViewCount[] = "view-count";
 
+const char kZeroEncodeDetails[] = "zero-encode-details";
+
 size_t RegisterChromeCrashKeys() {
   // The following keys may be chunked by the underlying crash logging system,
   // but ultimately constitute a single key-value pair.
@@ -143,6 +145,7 @@ size_t RegisterChromeCrashKeys() {
 #endif
     { kBug464926CrashKey, kSmallSize },
     { kViewCount, kSmallSize },
+    { kZeroEncodeDetails, kSmallSize },
   };
 
   // This dynamic set of keys is used for sets of key value pairs when gathering

chrome/common/crash_keys.h

@@ -126,6 +126,10 @@ extern const char kKaskoEquivalentGuid[];
 // Numbers of active views.
 extern const char kViewCount[];
 
+// TEMPORARY: The encoder/frame details at the time a zero-length encoded frame
+// was encountered.  http://crbug.com/519022
+extern const char kZeroEncodeDetails[];
+
 }  // namespace crash_keys
 
 #endif  // CHROME_COMMON_CRASH_KEYS_H_

media/cast/net/cast_transport_sender_impl.cc

@@ -242,7 +242,9 @@ namespace {
 void EncryptAndSendFrame(const EncodedFrame& frame,
                          TransportEncryptionHandler* encryptor,
                          RtpSender* sender) {
-  if (encryptor->is_activated()) {
+  // TODO(miu): We probably shouldn't attempt to send an empty frame, but this
+  // issue is still under investigation.  http://crbug.com/519022
+  if (encryptor->is_activated() && !frame.data.empty()) {
     EncodedFrame encrypted_frame;
     frame.CopyMetadataTo(&encrypted_frame);
     if (encryptor->Encrypt(frame.frame_id, frame.data, &encrypted_frame.data)) {

media/cast/sender/external_video_encoder.cc

@@ -7,11 +7,15 @@
 #include <cmath>
 
 #include "base/bind.h"
+#include "base/debug/crash_logging.h"
+#include "base/debug/dump_without_crashing.h"
+#include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/scoped_vector.h"
 #include "base/memory/shared_memory.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
+#include "base/strings/stringprintf.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_types.h"
 #include "media/base/video_util.h"
@@ -97,7 +101,8 @@ class ExternalVideoEncoder::VEAClientImpl
         key_frame_encountered_(false),
         codec_profile_(media::VIDEO_CODEC_PROFILE_UNKNOWN),
         key_frame_quantizer_parsable_(false),
-        requested_bit_rate_(-1) {}
+        requested_bit_rate_(-1),
+        has_seen_zero_length_encoded_frame_(false) {}
 
   base::SingleThreadTaskRunner* task_runner() const {
     return task_runner_.get();
@@ -306,6 +311,25 @@ class ExternalVideoEncoder::VEAClientImpl
         quantizer_estimator_.Reset();
       }
 
+      // TODO(miu): Determine when/why encoding can produce zero-length data,
+      // which causes crypto crashes.  http://crbug.com/519022
+      if (!has_seen_zero_length_encoded_frame_ && encoded_frame->data.empty()) {
+        has_seen_zero_length_encoded_frame_ = true;
+
+        const char kZeroEncodeDetails[] = "zero-encode-details";
+        const std::string details = base::StringPrintf(
+            "%c/%c,id=%" PRIu32 ",rtp=%" PRIu32 ",br=%d,q=%zu,act=%c,ref=%d",
+            codec_profile_ == media::VP8PROFILE_ANY ? 'V' : 'H',
+            key_frame ? 'K' : 'D', encoded_frame->frame_id,
+            encoded_frame->rtp_timestamp, request.target_bit_rate / 1000,
+            in_progress_frame_encodes_.size(), encoder_active_ ? 'Y' : 'N',
+            static_cast<int>(encoded_frame->referenced_frame_id % 1000));
+        base::debug::SetCrashKeyValue(kZeroEncodeDetails, details);
+        // Please forward crash reports to http://crbug.com/519022:
+        base::debug::DumpWithoutCrashing();
+        base::debug::ClearCrashKey(kZeroEncodeDetails);
+      }
+
       cast_environment_->PostTask(
           CastEnvironment::MAIN,
           FROM_HERE,
@@ -457,6 +481,11 @@ class ExternalVideoEncoder::VEAClientImpl
   // Used to compute utilization metrics for each frame.
   QuantizerEstimator quantizer_estimator_;
 
+  // Set to true once a frame with zero-length encoded data has been
+  // encountered.
+  // TODO(miu): Remove after discovering cause.  http://crbug.com/519022
+  bool has_seen_zero_length_encoded_frame_;
+
   DISALLOW_COPY_AND_ASSIGN(VEAClientImpl);
 };
 

media/cast/sender/vp8_encoder.cc

@@ -4,7 +4,11 @@
 
 #include "media/cast/sender/vp8_encoder.h"
 
+#include "base/debug/crash_logging.h"
+#include "base/debug/dump_without_crashing.h"
+#include "base/format_macros.h"
 #include "base/logging.h"
+#include "base/strings/stringprintf.h"
 #include "media/base/video_frame.h"
 #include "media/cast/cast_defines.h"
 #include "third_party/libvpx_new/source/libvpx/vpx/vp8cx.h"
@@ -31,7 +35,8 @@ Vp8Encoder::Vp8Encoder(const VideoSenderConfig& video_config)
       bitrate_kbit_(cast_config_.start_bitrate / 1000),
       last_encoded_frame_id_(kStartFrameId),
       last_acked_frame_id_(kStartFrameId),
-      undroppable_frames_(0) {
+      undroppable_frames_(0),
+      has_seen_zero_length_encoded_frame_(false) {
   config_.g_timebase.den = 0;  // Not initialized.
 
   for (int i = 0; i < kNumberOfVp8VideoBuffers; ++i) {
@@ -283,6 +288,24 @@ void Vp8Encoder::Encode(const scoped_refptr<media::VideoFrame>& video_frame,
   DCHECK(!encoded_frame->data.empty())
       << "BUG: Encoder must provide data since lagged encoding is disabled.";
 
+  // TODO(miu): Determine when/why encoding can produce zero-length data,
+  // which causes crypto crashes.  http://crbug.com/519022
+  if (!has_seen_zero_length_encoded_frame_ && encoded_frame->data.empty()) {
+    has_seen_zero_length_encoded_frame_ = true;
+
+    const char kZeroEncodeDetails[] = "zero-encode-details";
+    const std::string details = base::StringPrintf(
+        "SV/%c,id=%" PRIu32 ",rtp=%" PRIu32 ",br=%d,kfr=%c",
+        encoded_frame->dependency == EncodedFrame::KEY ? 'K' : 'D',
+        encoded_frame->frame_id, encoded_frame->rtp_timestamp,
+        static_cast<int>(config_.rc_target_bitrate),
+        key_frame_requested_ ? 'Y' : 'N');
+    base::debug::SetCrashKeyValue(kZeroEncodeDetails, details);
+    // Please forward crash reports to http://crbug.com/519022:
+    base::debug::DumpWithoutCrashing();
+    base::debug::ClearCrashKey(kZeroEncodeDetails);
+  }
+
   // Compute deadline utilization as the real-world time elapsed divided by the
   // frame duration.
   const base::TimeDelta processing_time = base::TimeTicks::Now() - start_time;

media/cast/sender/vp8_encoder.h

@@ -114,6 +114,11 @@ class Vp8Encoder : public SoftwareVideoEncoder {
   // This is bound to the thread where Initialize() is called.
   base::ThreadChecker thread_checker_;
 
+  // Set to true once a frame with zero-length encoded data has been
+  // encountered.
+  // TODO(miu): Remove after discovering cause.  http://crbug.com/519022
+  bool has_seen_zero_length_encoded_frame_;
+
   DISALLOW_COPY_AND_ASSIGN(Vp8Encoder);
 };