Ensure flush support is properly known before using it.

VEA flush support isn't known until Initialize() completes, which
happens by way of RequireBitstreamBuffers(). The old code uses the
value of |flush_support_| during InitCompleted() despite it not
being set until after InitCompleted() runs.

The fix is to ensure we set |flush_support_| correctly before we
use it and to switch to a base::Optional for its value to prevent
accidental incorrect usage in the future.

R=sandersd

Fixed: 1163673
Test: New unittest.
Change-Id: I69cec2f954ffc68773984ab59227b2cd10079820
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2613670
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840877}

media/video/video_encode_accelerator_adapter.cc

@@ -424,7 +424,7 @@ void VideoEncodeAcceleratorAdapter::FlushOnAcceleratorThread(StatusCB done_cb) {
 
   // If flush is not supported FlushCompleted() will be called by
   // BitstreamBufferReady() when |active_encodes_| is empty.
-  if (flush_support_ && state_ == State::kFlushing) {
+  if (state_ == State::kFlushing && flush_support_.value()) {
     accelerator_->Flush(
         base::BindOnce(&VideoEncodeAcceleratorAdapter::FlushCompleted,
                        base::Unretained(this)));
@@ -453,7 +453,6 @@ void VideoEncodeAcceleratorAdapter::RequireBitstreamBuffers(
   accelerator_->UseOutputBitstreamBuffer(
       BitstreamBuffer(buffer_id, region->Duplicate(), region->GetSize()));
   InitCompleted(Status());
-  flush_support_ = accelerator_->IsFlushSupported();
 }
 
 void VideoEncodeAcceleratorAdapter::BitstreamBufferReady(
@@ -531,7 +530,7 @@ void VideoEncodeAcceleratorAdapter::BitstreamBufferReady(
     }
   }
   output_cb_.Run(std::move(result), std::move(desc));
-  if (active_encodes_.empty() && !flush_support_) {
+  if (active_encodes_.empty() && !flush_support_.value()) {
     // Manually call FlushCompleted(), since |accelerator_| won't do it for us.
     FlushCompleted(true);
   }
@@ -586,6 +585,7 @@ void VideoEncodeAcceleratorAdapter::InitCompleted(Status status) {
   }
 
   state_ = State::kReadyToEncode;
+  flush_support_ = accelerator_->IsFlushSupported();
 
   // Send off the encodes that came in while we were waiting for initialization.
   for (auto& encode : pending_encodes_) {
@@ -598,7 +598,7 @@ void VideoEncodeAcceleratorAdapter::InitCompleted(Status status) {
   // all the pending encodes have been sent.
   if (pending_flush_) {
     state_ = State::kFlushing;
-    if (flush_support_) {
+    if (flush_support_.value()) {
       accelerator_->Flush(
           base::BindOnce(&VideoEncodeAcceleratorAdapter::FlushCompleted,
                          base::Unretained(this)));

media/video/video_encode_accelerator_adapter.h

@@ -11,6 +11,7 @@
 #include "base/containers/circular_deque.h"
 #include "base/containers/queue.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
 #include "base/synchronization/lock.h"
 #include "media/base/media_export.h"
 #include "media/base/video_encoder.h"
@@ -135,7 +136,7 @@ class MEDIA_EXPORT VideoEncodeAcceleratorAdapter
   scoped_refptr<base::SequencedTaskRunner> callback_task_runner_;
 
   State state_ = State::kNotInitialized;
-  bool flush_support_ = false;
+  base::Optional<bool> flush_support_;
 
   struct PendingEncode {
     PendingEncode();

media/video/video_encode_accelerator_adapter_test.cc

@@ -213,6 +213,36 @@ TEST_F(VideoEncodeAcceleratorAdapterTest, InitializeAfterFirstFrame) {
   EXPECT_EQ(outputs_count, 1);
 }
 
+TEST_F(VideoEncodeAcceleratorAdapterTest, FlushDuringInitialize) {
+  VideoEncoder::Options options;
+  options.frame_size = gfx::Size(640, 480);
+  int outputs_count = 0;
+  auto pixel_format = PIXEL_FORMAT_I420;
+  VideoEncoder::OutputCB output_cb = base::BindLambdaForTesting(
+      [&](VideoEncoderOutput, base::Optional<VideoEncoder::CodecDescription>) {
+        outputs_count++;
+      });
+
+  vea()->SetEncodingCallback(base::BindLambdaForTesting(
+      [&](BitstreamBuffer&, bool keyframe, scoped_refptr<VideoFrame> frame) {
+        EXPECT_EQ(keyframe, true);
+        EXPECT_EQ(frame->format(), pixel_format);
+        EXPECT_EQ(frame->coded_size(), options.frame_size);
+        return BitstreamBufferMetadata(1, keyframe, frame->timestamp());
+      }));
+  adapter()->Initialize(profile_, options, std::move(output_cb),
+                        ValidatingStatusCB());
+
+  auto frame = CreateGreenFrame(options.frame_size, pixel_format,
+                                base::TimeDelta::FromMilliseconds(1));
+  adapter()->Encode(frame, true, ValidatingStatusCB());
+  adapter()->Flush(base::BindLambdaForTesting([&](Status s) {
+    EXPECT_TRUE(s.is_ok());
+    EXPECT_EQ(outputs_count, 1);
+  }));
+  RunUntilIdle();
+}
+
 TEST_F(VideoEncodeAcceleratorAdapterTest, InitializationError) {
   VideoEncoder::Options options;
   options.frame_size = gfx::Size(640, 480);