Don't delay video frame submission when frame size changes

Upload upstream commit:
> Don't delay video frame submission when frame size changes.
>
> We end up racing with the relayout if we defer frame submission
> until after the compositor ack for size changes.
>
> It also changes VRI to almost always paint the video frame before
> notifying of frame size changes. Only the CanReadWithoutStalling()
> path will still relayout before the frame size changes, but that
> will only effect the first layout (nothing -> black square of the
> right size).
>
> R=​liberato
>
> Fixed: 41475959
> Change-Id: I1598f9bb05a0bd73bb7a92a81eda0cc6f76c908f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5393428
> Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Commit-Queue: Frank Liberato <liberato@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1281615}

Bug: https://jira-eu.sec.samsung.net/browse/VDGAME-698
Change-Id: I2ec4a063a58b2c5a40713e54f9cb456e8e638826
Signed-off-by: Jakub Gajownik <j.gajownik2@samsung.com>

diff --git a/media/renderers/video_renderer_impl.cc b/media/renderers/video_renderer_impl.cc
index 362a7824fbb97f404f5792c3119153e6d2c8bf2e..eb17b90e9217ce7b272cd8b5c000dda27b62d5da 100644 (file)
--- a/media/renderers/video_renderer_impl.cc
+++ b/media/renderers/video_renderer_impl.cc
@@ -1077,8 +1077,8 @@ void VideoRendererImpl::PaintFirstFrame() {
   auto first_frame =
       algorithm_->Render(base::TimeTicks(), base::TimeTicks(), nullptr);
   DCHECK(first_frame);
-  CheckForMetadataChanges(first_frame->format(), first_frame->natural_size());
   sink_->PaintSingleFrame(first_frame);
+  CheckForMetadataChanges(first_frame->format(), first_frame->natural_size());
   painted_first_frame_ = true;
   paint_first_frame_cb_.Cancel();
 }

diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc b/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
index b0eeb217ebf459c91fc1e53529eba9a2deebdc6f..9c73a38bfbadad98b4d16cf7afc261f2879d79fd 100644 (file)
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
@@ -316,7 +316,7 @@ void VideoFrameSubmitter::OnContextLost() {
   if (context_provider_)
     context_provider_->RemoveObserver(this);
 
-  waiting_for_compositor_ack_ = false;
+  waiting_for_compositor_ack_ = 0;
   last_frame_id_.reset();
 
   if (video_frame_provider_)
@@ -340,7 +340,15 @@ void VideoFrameSubmitter::DidReceiveCompositorFrameAck(
     WTF::Vector<viz::ReturnedResource> resources) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   ReclaimResources(std::move(resources));
-  waiting_for_compositor_ack_ = false;
+
+  // `waiting_for_compositor_ack_` may be set to zero during SubmitEmptyFrame()
+  // or upon ContextLost().
+  if (waiting_for_compositor_ack_ == 0) {
+    DCHECK(!last_frame_id_);
+    return;
+  }
+
+  --waiting_for_compositor_ack_;
 }
 
 void VideoFrameSubmitter::OnBeginFrame(
@@ -704,17 +712,17 @@ bool VideoFrameSubmitter::SubmitFrame(
   }
 #endif  // defined(TIZEN_TV_UPSTREAM_MULTIMEDIA)
 
-  if (!compositor_frame_sink_ || !ShouldSubmit())
+  if (!compositor_frame_sink_ || !ShouldSubmit()) {
     return false;
+  }
 
   // Not submitting a frame when waiting for a previous ack saves memory by
   // not building up unused remote side resources. See https://crbug.com/830828.
   //
   // Similarly we don't submit the same frame multiple times.
-  if (waiting_for_compositor_ack_ || last_frame_id_ == video_frame->unique_id())
+  if (last_frame_id_ == video_frame->unique_id()) {
     return false;
-
-  last_frame_id_ = video_frame->unique_id();
+  }
 
   gfx::Size frame_size(video_frame->natural_size());
 
@@ -733,12 +741,22 @@ bool VideoFrameSubmitter::SubmitFrame(
     return false;
   }
 
+  bool frame_size_changed = false;
   if (frame_size_ != frame_size) {
     if (!frame_size_.IsEmpty())
       GenerateNewSurfaceId();
     frame_size_ = frame_size;
+    frame_size_changed = true;
+  }
+
+  // We can't delay frame size changes even if we have a pending compositor ACK
+  // because a relayout signal is already in flight on the main thread.
+  if (waiting_for_compositor_ack_ > 0 && !frame_size_changed) {
+    return false;
   }
 
+  last_frame_id_ = video_frame->unique_id();
+
   auto frame_token = ++next_frame_token_;
   auto source_id = begin_frame_ack.frame_id.source_id;
   if (source_id != viz::BeginFrameArgs::kManualSourceId) {
@@ -769,7 +787,7 @@ bool VideoFrameSubmitter::SubmitFrame(
                                     last_begin_frame_args_);
   resource_provider_->ReleaseFrameResources();
 
-  waiting_for_compositor_ack_ = true;
+  ++waiting_for_compositor_ack_;
   return true;
 }
 
@@ -796,8 +814,9 @@ void VideoFrameSubmitter::SubmitEmptyFrame() {
   frame_trackers_.NotifySubmitFrame(frame_token, false, begin_frame_ack,
                                     last_begin_frame_args_);
 
-  // We don't set |waiting_for_compositor_ack_| here since we want to allow a
+  // We set `waiting_for_compositor_ack_` to zero here since we want to allow a
   // subsequent real frame to replace it at any time if needed.
+  waiting_for_compositor_ack_ = 0;
 }
 
 void VideoFrameSubmitter::SubmitSingleFrame() {

diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter.h b/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
index edd2621b20b059a7a33c69ca85bbe4a30490fa35..45343b698df7347e9ce13dcd12bce28e68d19a94 100644 (file)
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
@@ -164,7 +164,7 @@ class PLATFORM_EXPORT VideoFrameSubmitter
   mojo::Receiver<viz::mojom::blink::CompositorFrameSinkClient> receiver_{this};
   WebContextProviderCallback context_provider_callback_;
   std::unique_ptr<VideoFrameResourceProvider> resource_provider_;
-  bool waiting_for_compositor_ack_ = false;
+  int waiting_for_compositor_ack_ = 0;
 
   // When UseVideoFrameSinkBundle is enabled, this is initialized to a local
   // implementation which batches outgoing Viz requests with those from other

diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc b/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
index 514b8c0f5d64af00982503a086e41f747dc31770..47f0d9d2df6862650c9406a7ac3d36536f14f438 100644 (file)
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
@@ -175,7 +175,7 @@ class VideoFrameSubmitterTest : public testing::Test,
         begin_frame_source_(new viz::FakeExternalBeginFrameSource(0.f, false)),
         video_frame_provider_(new StrictMock<MockVideoFrameProvider>()),
         context_provider_(viz::TestContextProvider::Create()) {
-    if (GetParam()) {
+    if (HasBeginFrameAcks()) {
       scoped_feature_list_.InitAndEnableFeature(features::kOnBeginFrameAcks);
     } else {
       scoped_feature_list_.InitAndDisableFeature(features::kOnBeginFrameAcks);
@@ -185,6 +185,8 @@ class VideoFrameSubmitterTest : public testing::Test,
     task_environment_.RunUntilIdle();
   }
 
+  bool HasBeginFrameAcks() const { return GetParam(); }
+
   void MakeSubmitter() { MakeSubmitter(base::DoNothing()); }
 
   void MakeSubmitter(
@@ -253,7 +255,7 @@ class VideoFrameSubmitterTest : public testing::Test,
       const WTF::HashMap<uint32_t, viz::FrameTimingDetails>& timing_details,
       bool frame_ack,
       WTF::Vector<viz::ReturnedResource> resources) {
-    if (GetParam()) {
+    if (HasBeginFrameAcks() && frame_ack) {
       EXPECT_CALL(*resource_provider_, ReceiveReturnsFromParent(_));
     }
     submitter_->OnBeginFrame(args, timing_details, frame_ack,
@@ -330,7 +332,7 @@ TEST_P(VideoFrameSubmitterTest, StopRenderingSkipsUpdateCurrentFrame) {
   EXPECT_SUBMISSION(SubmissionType::kBeginFrame);
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
   AckSubmittedFrame();
 
@@ -345,7 +347,7 @@ TEST_P(VideoFrameSubmitterTest, StopRenderingSkipsUpdateCurrentFrame) {
   EXPECT_CALL(*sink_, DidNotProduceFrame(_));
   args = begin_frame_source_->CreateBeginFrameArgs(BEGINFRAME_FROM_HERE,
                                                    now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 }
 
@@ -500,7 +502,6 @@ TEST_P(VideoFrameSubmitterTest, RotationInformationPassedToResourceProvider) {
   EXPECT_CALL(*sink_, SetNeedsBeginFrame(true));
   submitter_->StartRendering();
   task_environment_.RunUntilIdle();
-  AckSubmittedFrame();
 
   EXPECT_CALL(*video_frame_provider_, UpdateCurrentFrame(_, _))
       .WillOnce(Return(true));
@@ -520,7 +521,7 @@ TEST_P(VideoFrameSubmitterTest, RotationInformationPassedToResourceProvider) {
 
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
   AckSubmittedFrame();
 
@@ -545,7 +546,7 @@ TEST_P(VideoFrameSubmitterTest, RotationInformationPassedToResourceProvider) {
 
   args = begin_frame_source_->CreateBeginFrameArgs(BEGINFRAME_FROM_HERE,
                                                    now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 }
 
@@ -575,7 +576,6 @@ TEST_P(VideoFrameSubmitterTest, FrameTransformTakesPrecedent) {
   EXPECT_CALL(*sink_, SetNeedsBeginFrame(true));
   submitter_->StartRendering();
   task_environment_.RunUntilIdle();
-  AckSubmittedFrame();
 
   auto frame = media::VideoFrame::CreateFrame(
       media::PIXEL_FORMAT_YV12, gfx::Size(8, 8), gfx::Rect(gfx::Size(8, 8)),
@@ -596,7 +596,7 @@ TEST_P(VideoFrameSubmitterTest, FrameTransformTakesPrecedent) {
 
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
   AckSubmittedFrame();
 }
@@ -610,7 +610,7 @@ TEST_P(VideoFrameSubmitterTest, OnBeginFrameSubmitsFrame) {
   EXPECT_SUBMISSION(SubmissionType::kBeginFrame);
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 }
 
@@ -620,7 +620,7 @@ TEST_P(VideoFrameSubmitterTest, MissedFrameArgDoesNotProduceFrame) {
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
   args.type = viz::BeginFrameArgs::MISSED;
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 }
 
@@ -631,7 +631,7 @@ TEST_P(VideoFrameSubmitterTest, MissingProviderDoesNotProduceFrame) {
 
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 }
 
@@ -645,7 +645,7 @@ TEST_P(VideoFrameSubmitterTest, NoUpdateOnFrameDoesNotProduceFrame) {
 
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 }
 
@@ -658,7 +658,7 @@ TEST_P(VideoFrameSubmitterTest, NotRenderingDoesNotProduceFrame) {
 
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 }
 
@@ -678,7 +678,7 @@ TEST_P(VideoFrameSubmitterTest, WaitingForAckPreventsNewFrame) {
   EXPECT_SUBMISSION(SubmissionType::kBeginFrame);
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 
   // DidNotProduceFrame should be called because no frame will be submitted
@@ -872,7 +872,7 @@ TEST_P(VideoFrameSubmitterTest, VideoRotationOutputRect) {
 
     viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
         BEGINFRAME_FROM_HERE, now_src_.get());
-    OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+    OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
     task_environment_.RunUntilIdle();
 
     EXPECT_EQ(sink_->last_submitted_compositor_frame().size_in_pixels(),
@@ -902,7 +902,7 @@ TEST_P(VideoFrameSubmitterTest, VideoRotationOutputRect) {
 
     viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
         BEGINFRAME_FROM_HERE, now_src_.get());
-    OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+    OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
     task_environment_.RunUntilIdle();
 
     // 180 deg rotation has same size.
@@ -933,7 +933,7 @@ TEST_P(VideoFrameSubmitterTest, VideoRotationOutputRect) {
 
     viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
         BEGINFRAME_FROM_HERE, now_src_.get());
-    OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+    OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
     task_environment_.RunUntilIdle();
 
     EXPECT_EQ(sink_->last_submitted_compositor_frame().size_in_pixels(),
@@ -980,7 +980,7 @@ TEST_P(VideoFrameSubmitterTest, PreferredInterval) {
   EXPECT_SUBMISSION(SubmissionType::kBeginFrame);
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 
   EXPECT_EQ(sink_->last_submitted_compositor_frame()
@@ -1008,7 +1008,7 @@ TEST_P(VideoFrameSubmitterTest, NoDuplicateFramesOnBeginFrame) {
   EXPECT_CALL(*resource_provider_, ReleaseFrameResources());
   viz::BeginFrameArgs args = begin_frame_source_->CreateBeginFrameArgs(
       BEGINFRAME_FROM_HERE, now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
   AckSubmittedFrame();
 
@@ -1020,7 +1020,7 @@ TEST_P(VideoFrameSubmitterTest, NoDuplicateFramesOnBeginFrame) {
   EXPECT_CALL(*sink_, DidNotProduceFrame(_));
   args = begin_frame_source_->CreateBeginFrameArgs(BEGINFRAME_FROM_HERE,
                                                    now_src_.get());
-  OnBeginFrame(args, {}, true, WTF::Vector<viz::ReturnedResource>());
+  OnBeginFrame(args, {}, false, WTF::Vector<viz::ReturnedResource>());
   task_environment_.RunUntilIdle();
 }
 
@@ -1108,7 +1108,7 @@ TEST_P(VideoFrameSubmitterTest, ProcessTimingDetails) {
 
     auto args = begin_frame_source_->CreateBeginFrameArgs(BEGINFRAME_FROM_HERE,
                                                           now_src_.get());
-    OnBeginFrame(args, timing_details, true,
+    OnBeginFrame(args, timing_details, false,
                  WTF::Vector<viz::ReturnedResource>());
     task_environment_.RunUntilIdle();
     AckSubmittedFrame();
@@ -1122,7 +1122,7 @@ INSTANTIATE_TEST_SUITE_P(,
                          testing::Bool(),
                          [](auto& info) {
                            return info.param ? "BeginFrameAcks"
-                                             : "CompositoFrameAcks";
+                                             : "CompositorFrameAcks";
                          });
 
 }  // namespace blink