Revert "[MediaFoundation] Request next frame during onPlaying event"

This reverts commit 395b2b1.

Reason for revert: the following test cases fail
- MediaFoundationRendererIntegrationTest.BasicPlayback
- MediaFoundationRendererIntegrationTest.BasicPlayback_MediaSource

on Windows 10/Asan

with the failure log:

[ RUN      ] MediaFoundationRendererIntegrationTest.BasicPlayback
=================================================================
==644==ERROR: AddressSanitizer: access-violation on unknown address 0x000000000008 (pc 0x7ff6889a1e92 bp 0x00c26c4fe5f0 sp 0x00c26c4fe560 T0)
==644==The signal is caused by a READ memory access.
==644==Hint: address points to the zero page.
==644==*** WARNING: Failed to initialize DbgHelp!              ***
==644==*** Most likely this means that the app is already      ***
==644==*** using DbgHelp, possibly with incompatible flags.    ***
==644==*** Due to technical reasons, symbolization might crash ***
==644==*** or produce wrong results.                           ***
    #0 0x7ff6889a1e91 in base::RepeatingCallback<void (const base::UnguessableToken &, const gfx::Size &, base::TimeDelta)>::Run C:\b\s\w\ir\cache\builder\src\base\callback.h:263
    #1 0x7ff6889a1e91 in media::MediaFoundationRenderer::RequestNextFrame(void) C:\b\s\w\ir\cache\builder\src\media\renderers\win\media_foundation_renderer.cc:1022:23
    #2 0x7ff68899bed8 in media::MediaFoundationRenderer::OnPlaying(void) C:\b\s\w\ir\cache\builder\src\media\renderers\win\media_foundation_renderer.cc:838:3
    #3 0x7ff68a04bb3a in base::OnceCallback<void ()>::Run C:\b\s\w\ir\cache\builder\src\base\callback.h:145
    #4 0x7ff68a04bb3a in base::TaskAnnotator::RunTaskImpl(struct base::PendingTask &) C:\b\s\w\ir\cache\builder\src\base\task\common\task_annotator.cc:133:32
    #5 0x7ff68ac91059 in base::TaskAnnotator::RunTask C:\b\s\w\ir\cache\builder\src\base\task\common\task_annotator.h:72
    #6 0x7ff68ac91059 in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(class base::LazyNow *) C:\b\s\w\ir\cache\builder\src\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:422:21
    #7 0x7ff68ac90046 in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork(void) C:\b\s\w\ir\cache\builder\src\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:292:41
    #8 0x7ff68acc53da in base::MessagePumpDefault::Run(class base::MessagePump::Delegate *) C:\b\s\w\ir\cache\builder\src\base\message_loop\message_pump_default.cc:39:55
    #9 0x7ff68ac9316b in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, class base::TimeDelta) C:\b\s\w\ir\cache\builder\src\base\task\sequence_manager\thread_controller_with_message_pump_impl.cc:575:12
    #10 0x7ff68960a00c in base::RunLoop::Run(class base::Location const &) C:\b\s\w\ir\cache\builder\src\base\run_loop.cc:141:14
    #11 0x7ff68942c9a7 in media::PipelineIntegrationTestBase::RunUntilQuitOrError(class base::RunLoop *) C:\b\s\w\ir\cache\builder\src\media\test\pipeline_integration_test_base.cc:697:13
    #12 0x7ff689424636 in media::PipelineIntegrationTestBase::RunUntilQuitOrEndedOrError(class base::RunLoop *) C:\b\s\w\ir\cache\builder\src\media\test\pipeline_integration_test_base.cc:709:3
    #13 0x7ff689424449 in media::PipelineIntegrationTestBase::WaitUntilEndedOrError(void) C:\b\s\w\ir\cache\builder\src\media\test\pipeline_integration_test_base.cc:226:5
    #14 0x7ff689423ac4 in media::PipelineIntegrationTestBase::WaitUntilOnEnded(void) C:\b\s\w\ir\cache\builder\src\media\test\pipeline_integration_test_base.cc:217:27
    #15 0x7ff686ce4597 in media::MediaFoundationRendererIntegrationTest_BasicPlayback_Test::TestBody(void) C:\b\s\w\ir\cache\builder\src\media\renderers\win\media_foundation_renderer_integration_test.cc:92:3
    #16 0x7ff6870b0c77 in testing::Test::Run(void) C:\b\s\w\ir\cache\builder\src\third_party\googletest\src\googletest\src\gtest.cc:2670:5
    #17 0x7ff6870b2c3b in testing::TestInfo::Run(void) C:\b\s\w\ir\cache\builder\src\third_party\googletest\src\googletest\src\gtest.cc:2849:11
    #18 0x7ff6870b498e in testing::TestSuite::Run(void) C:\b\s\w\ir\cache\builder\src\third_party\googletest\src\googletest\src\gtest.cc:3008:30
    #19 0x7ff6870d772f in testing::internal::UnitTestImpl::RunAllTests(void) C:\b\s\w\ir\cache\builder\src\third_party\googletest\src\googletest\src\gtest.cc:5866:44
    #20 0x7ff6870d6bd5 in testing::UnitTest::Run(void) C:\b\s\w\ir\cache\builder\src\third_party\googletest\src\googletest\src\gtest.cc:5440:10
    #21 0x7ff6896d7a69 in RUN_ALL_TESTS C:\b\s\w\ir\cache\builder\src\third_party\googletest\src\googletest\include\gtest\gtest.h:2284
    #22 0x7ff6896d7a69 in base::TestSuite::Run(void) C:\b\s\w\ir\cache\builder\src\base\test\test_suite.cc:463:16
    #23 0x7ff6896dc4ed in base::OnceCallback<int ()>::Run C:\b\s\w\ir\cache\builder\src\base\callback.h:145
    #24 0x7ff6896dc4ed in base::`anonymous namespace'::LaunchUnitTestsInternal C:\b\s\w\ir\cache\builder\src\base\test\launcher\unit_test_launcher.cc:181:38
    #25 0x7ff6896dc0ba in base::LaunchUnitTests(int, char **, class base::OnceCallback<(void)>, unsigned __int64) C:\b\s\w\ir\cache\builder\src\base\test\launcher\unit_test_launcher.cc:272:10
    #26 0x7ff686dcc7f9 in main C:\b\s\w\ir\cache\builder\src\media\test\run_all_unittests.cc:52:10
    #27 0x7ff68c67712b in invoke_main d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #28 0x7ff68c67712b in __scrt_common_main_seh d:\a01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #29 0x7fff384a2773  (C:\Windows\System32\KERNEL32.DLL+0x180012773)
    #30 0x7fff386e0d50  (C:\Windows\SYSTEM32\ntdll.dll+0x180070d50)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: access-violation C:\b\s\w\ir\cache\builder\src\base\callback.h:263 in base::RepeatingCallback<void (const base::UnguessableToken &, const gfx::Size &, base::TimeDelta)>::Run
==644==ABORTING

Original change's description:
> [MediaFoundation] Request next frame during onPlaying event
>
> OS: Win10, Win11
>
> During OnPlaying event, a frame request should be called to get
> the first frame to output at the earliest possible time. This
> is the earliest time when a frame is available. Current
> implementation waits for render to be called, while
> StartPlayingFrom's call for RequestNextFrameBetweenTimestamps
> may not output a result if the media engine is not yet ready
> to output a frame.
>
> Bug: 1355520
> Change-Id: Ice60ac41ca4b8cae9b0687626e93017d0a4406f0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3852409
> Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
> Commit-Queue: Daoyuan Li <daoyuanli@microsoft.com>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1041097}

Bug: 1355520
Change-Id: Ic483e314ce14e3f187691df772515eacea387cb1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3863075
Commit-Queue: Asami Doi <asamidoi@chromium.org>
Owners-Override: Asami Doi <asamidoi@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Asami Doi <asamidoi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041256}

media/mojo/clients/win/media_foundation_renderer_client.cc

@@ -225,6 +225,14 @@ void MediaFoundationRendererClient::StartPlayingFrom(base::TimeDelta time) {
   SignalMediaPlayingStateChange(true);
   next_video_frame_.reset();
   mojo_renderer_->StartPlayingFrom(time);
+  // Request the first frame (if we are not in frame server mode this just
+  // gets dropped).
+  base::TimeTicks request_min = base::TimeTicks::Now();
+  base::TimeTicks request_max =
+      base::TimeTicks::Now() + GetPreferredRenderInterval();
+
+  renderer_extension_->RequestNextFrameBetweenTimestamps(request_min,
+                                                         request_max);
 }
 
 void MediaFoundationRendererClient::SetPlaybackRate(double playback_rate) {
@@ -348,15 +356,22 @@ scoped_refptr<VideoFrame> MediaFoundationRendererClient::Render(
     return nullptr;
   }
 
+  base::TimeTicks next_request_min = deadline_max;
+  base::TimeTicks next_request_max =
+      deadline_max + GetPreferredRenderInterval();
+
   auto callback =
-      [](base::WeakPtr<MediaFoundationRendererClient> renderer_client) {
-        if (renderer_client) {
-          renderer_client->renderer_extension_->RequestNextFrame();
+      [](base::TimeTicks deadline_min, base::TimeTicks deadline_max,
+         base::WeakPtr<MediaFoundationRendererClient> renderer_client) {
+        if (renderer_client.MaybeValid()) {
+          renderer_client->renderer_extension_
+              ->RequestNextFrameBetweenTimestamps(deadline_min, deadline_max);
         }
       };
 
   media_task_runner_->PostTask(
-      FROM_HERE, base::BindOnce(callback, weak_factory_.GetWeakPtr()));
+      FROM_HERE, base::BindOnce(callback, next_request_min, next_request_max,
+                                weak_factory_.GetWeakPtr()));
 
   // TODO(crbug.com/1298093): Need to report underflow when we don't have a
   // frame ready for presentation by calling OnBufferingStateChange

media/mojo/clients/win/media_foundation_renderer_client.h

@@ -140,9 +140,9 @@ class MediaFoundationRendererClient
   void OnOverlayStateChanged(const gpu::Mailbox& mailbox, bool promoted);
   void UpdateRenderMode();
 
-  // This class is constructed on the main thread. Hence we store
-  // PendingRemotes so we can bind the Remotes on the media task
-  // runner during/after Initialize().
+  // This class is constructed on the main thread and used exclusively on the
+  // media thread. Hence we store PendingRemotes so we can bind the Remotes
+  // on the media task runner during/after Initialize().
   scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
   std::unique_ptr<MediaLog> media_log_;
   std::unique_ptr<MojoRenderer> mojo_renderer_;

media/mojo/mojom/renderer_extensions.mojom

@@ -125,10 +125,12 @@ interface MediaFoundationRendererExtension {
   // Notify that the frame has been displayed and can be reused.
   NotifyFrameReleased(mojo_base.mojom.UnguessableToken frame_token);
 
-  // Request a frame from the media engine if it is available
+  // Request a frame from the media engine if it is available for a specific
+  // time.
   // The frame will be returned async via the
   // MediaFoundationRendererClientExtension::OnFrameAvailable callback.
-  RequestNextFrame();
+  RequestNextFrameBetweenTimestamps(mojo_base.mojom.TimeTicks deadline_min,
+    mojo_base.mojom.TimeTicks deadline_max);
 
   // Notify which rendering mode to be using for future video frames.
   SetMediaFoundationRenderingMode(MediaFoundationRenderingMode mode);

media/mojo/services/media_foundation_renderer_wrapper.cc

@@ -230,8 +230,10 @@ void MediaFoundationRendererWrapper::NotifyFrameReleased(
   renderer_->NotifyFrameReleased(frame_token);
 }
 
-void MediaFoundationRendererWrapper::RequestNextFrame() {
-  renderer_->RequestNextFrame();
+void MediaFoundationRendererWrapper::RequestNextFrameBetweenTimestamps(
+    base::TimeTicks deadline_min,
+    base::TimeTicks deadline_max) {
+  renderer_->RequestNextFrameBetweenTimestamps(deadline_min, deadline_max);
 }
 
 void MediaFoundationRendererWrapper::SetMediaFoundationRenderingMode(

media/mojo/services/media_foundation_renderer_wrapper.h

@@ -63,7 +63,8 @@ class MediaFoundationRendererWrapper final
   void SetOutputRect(const gfx::Rect& output_rect,
                      SetOutputRectCallback callback) override;
   void NotifyFrameReleased(const base::UnguessableToken& frame_token) override;
-  void RequestNextFrame() override;
+  void RequestNextFrameBetweenTimestamps(base::TimeTicks deadline_min,
+                                         base::TimeTicks deadline_max) override;
   void SetMediaFoundationRenderingMode(
       MediaFoundationRenderingMode mode) override;
 

media/renderers/win/media_foundation_renderer.cc

@@ -833,10 +833,6 @@ void MediaFoundationRenderer::OnPlaying() {
   OnBufferingStateChange(
       BufferingState::BUFFERING_HAVE_ENOUGH,
       BufferingStateChangeReason::BUFFERING_CHANGE_REASON_UNKNOWN);
-
-  // Earliest time to request first frame to screen
-  RequestNextFrame();
-
   // The OnPlaying callback from MediaEngineNotifyImpl lets us know that an
   // MF_MEDIA_ENGINE_EVENT_PLAYING message has been received. At this point we
   // can safely start sending Statistics as any asynchronous Flush action in
@@ -972,7 +968,9 @@ void MediaFoundationRenderer::OnError(PipelineStatus status,
     renderer_client_->OnError(new_status);
 }
 
-void MediaFoundationRenderer::RequestNextFrame() {
+void MediaFoundationRenderer::RequestNextFrameBetweenTimestamps(
+    base::TimeTicks deadline_min,
+    base::TimeTicks deadline_max) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
   if (rendering_mode_ != MediaFoundationRenderingMode::FrameServer) {
     return;

media/renderers/win/media_foundation_renderer.h

@@ -94,7 +94,8 @@ class MEDIA_EXPORT MediaFoundationRenderer
   void SetOutputRect(const gfx::Rect& output_rect,
                      SetOutputRectCB callback) override;
   void NotifyFrameReleased(const base::UnguessableToken& frame_token) override;
-  void RequestNextFrame() override;
+  void RequestNextFrameBetweenTimestamps(base::TimeTicks deadline_min,
+                                         base::TimeTicks deadline_max) override;
   void SetMediaFoundationRenderingMode(
       MediaFoundationRenderingMode render_mode) override;
 

media/renderers/win/media_foundation_renderer_extension.h

@@ -42,7 +42,9 @@ class MEDIA_EXPORT MediaFoundationRendererExtension {
       const base::UnguessableToken& frame_token) = 0;
 
   // Request a new frame to be provided to the client.
-  virtual void RequestNextFrame() = 0;
+  virtual void RequestNextFrameBetweenTimestamps(
+      base::TimeTicks deadline_min,
+      base::TimeTicks deadline_max) = 0;
 
   // Change which mode we are using for video frame rendering.
   virtual void SetMediaFoundationRenderingMode(