Android: Fix random crash in HW encode accelerator

media/gpu/android_video_encode_accelerator.cc

Index: media/gpu/android_video_encode_accelerator.cc
diff --git a/media/gpu/android_video_encode_accelerator.cc b/media/gpu/android_video_encode_accelerator.cc
index b16f933222866fe2bb9bcae46c1883d6a9eeecb7..0dfebf8740638529f189c1cdf15b7de0cbbc8b93 100644
--- a/media/gpu/android_video_encode_accelerator.cc
+++ b/media/gpu/android_video_encode_accelerator.cc
@@ -49,8 +49,8 @@ enum PixelFormat {
       DLOG(ERROR) << log;                                      \
       if (client_ptr_factory_->GetWeakPtr()) {                 \

[watk, 2016/12/28 01:01:51]

It's not possible for this to be null AFAICT. Should probably change it to if
(!error_occurred).

[braveyao, 2016/12/28 01:28:18]

Done.

         client_ptr_factory_->GetWeakPtr()->NotifyError(error); \
-        client_ptr_factory_.reset();                           \
       }                                                        \
+      error_occurred_ = true;                                  \
       return;                                                  \
     }                                                          \
   } while (0)
@@ -94,7 +94,7 @@ static bool GetSupportedColorFormatForMime(const std::string& mime,
 }
 
 AndroidVideoEncodeAccelerator::AndroidVideoEncodeAccelerator()
-    : num_buffers_at_codec_(0), last_set_bitrate_(0) {}
+    : num_buffers_at_codec_(0), last_set_bitrate_(0), error_occurred_(false) {}
 
 AndroidVideoEncodeAccelerator::~AndroidVideoEncodeAccelerator() {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -152,6 +152,7 @@ bool AndroidVideoEncodeAccelerator::Initialize(
   DCHECK(thread_checker_.CalledOnValidThread());
 
   client_ptr_factory_.reset(new base::WeakPtrFactory<Client>(client));

[watk, 2016/12/28 01:01:51]

I'd suggest adding a DCHECK(client) or if (!client) return false; to make it
clear that client can't be null.

[braveyao, 2016/12/28 01:28:18]

Done.

+  error_occurred_ = false;

[watk, 2016/12/28 01:01:51]

Initialize() cannot be called more than once so this is unnecessary.

[braveyao, 2016/12/28 01:28:18]

Done.

 
   if (!(MediaCodecUtil::SupportsSetParameters() &&
         format == PIXEL_FORMAT_I420)) {
@@ -285,6 +286,7 @@ void AndroidVideoEncodeAccelerator::Destroy() {
   DVLOG(3) << __PRETTY_FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
   client_ptr_factory_.reset();
+  error_occurred_ = false;

[watk, 2016/12/28 01:01:51]

No need to reset this, we're deleting below. (The .reset() above is also 
unnecessary, but I can see it adding clarity).

[braveyao, 2016/12/28 01:28:18]

Done.

   if (media_codec_) {
     if (io_timer_.IsRunning())
       io_timer_.Stop();
@@ -301,7 +303,7 @@ void AndroidVideoEncodeAccelerator::DoIOTask() {
 }
 
 void AndroidVideoEncodeAccelerator::QueueInput() {
-  if (!client_ptr_factory_->GetWeakPtr() || pending_frames_.empty())
+  if (error_occurred_ || pending_frames_.empty())
     return;
 
   int input_buf_index = 0;
@@ -368,8 +370,8 @@ void AndroidVideoEncodeAccelerator::QueueInput() {
 }
 
 void AndroidVideoEncodeAccelerator::DequeueOutput() {
-  if (!client_ptr_factory_->GetWeakPtr() ||
-      available_bitstream_buffers_.empty() || num_buffers_at_codec_ == 0) {
+  if (error_occurred_ || available_bitstream_buffers_.empty() ||
+      num_buffers_at_codec_ == 0) {
     return;
   }