GMBVFPool: bail if VideoFrame format forces change of OutputFormat

ClusterFuzz encountered an issue if a given VideoFrame comes and
causes a change of OutputFormat: in the bug below, an I420 VF
was received after a series of I420A ones, and caused a crash
when the alpha channel was accessed. This might happen more
generally if the received VideoFrames cause a change of
OutputFormat and we access what we don't have.

This CL addresses that by comparing the current |output_format_|
with what is derived out of the incoming VideoFrame. If they
differ, we just bail.

Bug: 875158, 875670
Change-Id: I961b5bf95651a3888482957ee0b7acd48149d467
Reviewed-on: https://chromium-review.googlesource.com/1180543
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584574}

media/video/gpu_memory_buffer_video_frame_pool.cc

@@ -623,8 +623,13 @@ void GpuMemoryBufferVideoFramePool::PoolImpl::CreateHardwareFrame(
   // Lazily initialize |output_format_| since VideoFrameOutputFormat() has to be
   // called on the media_thread while this object might be instantiated on any.
   const VideoPixelFormat pixel_format = video_frame->format();
-  if (output_format_ == GpuVideoAcceleratorFactories::OutputFormat::UNDEFINED) {
+  if (output_format_ == GpuVideoAcceleratorFactories::OutputFormat::UNDEFINED)
     output_format_ = gpu_factories_->VideoFrameOutputFormat(pixel_format);
+  // Bail if we have a change of GpuVideoAcceleratorFactories::OutputFormat;
+  // such changes should not happen in general (see https://crbug.com/875158).
+  if (output_format_ != gpu_factories_->VideoFrameOutputFormat(pixel_format)) {
+    std::move(frame_ready_cb).Run(video_frame);
+    return;
   }
 
   bool is_software_backed_video_frame = !video_frame->HasTextures();

media/video/gpu_memory_buffer_video_frame_pool_unittest.cc

@@ -681,4 +681,35 @@ TEST_F(GpuMemoryBufferVideoFramePoolTest, AbortCopies) {
   ASSERT_FALSE(frame_2);
 }
 
+// Tests that an I420 VideoFrame after an I420A is ignored, i.e. passed through.
+// See e.g. https://crbug.com/875158.
+TEST_F(GpuMemoryBufferVideoFramePoolTest, VideoFrameChangesPixelFormat) {
+  scoped_refptr<VideoFrame> software_frame_1 = CreateTestYUVAVideoFrame(10);
+  scoped_refptr<VideoFrame> frame_1;
+  mock_gpu_factories_->SetVideoFrameOutputFormat(
+      media::GpuVideoAcceleratorFactories::OutputFormat::RGBA);
+  gpu_memory_buffer_pool_->MaybeCreateHardwareFrame(
+      software_frame_1,
+      base::BindOnce(MaybeCreateHardwareFrameCallback, &frame_1));
+  RunUntilIdle();
+
+  EXPECT_NE(software_frame_1.get(), frame_1.get());
+  EXPECT_EQ(PIXEL_FORMAT_RGB32, frame_1->format());
+  EXPECT_EQ(1u, frame_1->NumTextures());
+  EXPECT_EQ(1u, gles2_->gen_textures_count());
+  EXPECT_TRUE(frame_1->metadata()->IsTrue(
+      media::VideoFrameMetadata::READ_LOCK_FENCES_ENABLED));
+
+  scoped_refptr<VideoFrame> software_frame_2 = CreateTestYUVVideoFrame(10);
+  mock_gpu_factories_->SetVideoFrameOutputFormat(
+      media::GpuVideoAcceleratorFactories::OutputFormat::I420);
+  scoped_refptr<VideoFrame> frame_2;
+  gpu_memory_buffer_pool_->MaybeCreateHardwareFrame(
+      software_frame_2,
+      base::BindOnce(MaybeCreateHardwareFrameCallback, &frame_2));
+  RunUntilIdle();
+
+  EXPECT_EQ(software_frame_2.get(), frame_2.get());
+}
+
 }  // namespace media