[Fuchsia] Improve underflow handling in FuchsiaAudioOutputDevice

Previously FuchsiaAudioOutputDevice was checking for underflows when
scheduling PumpSamples() timer and then in PumpSamples() itself. When
trying to recover it was also delaying the next packet by
kLeadTimeExtra, which is not necessary. This change moves underflow
logic to PumpSamples(), which allows to simplify that logic and reduce
the effects of missed timer deadline.

Bug: 1066203
Change-Id: I617ec7f7a248360fd0a17575d7f36f59f7f77eb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2989095
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Commit-Queue: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#896121}

media/fuchsia/audio/fuchsia_audio_output_device.cc

@@ -385,46 +385,33 @@ void FuchsiaAudioOutputDevice::SchedulePumpSamples() {
 
   base::TimeTicks now = base::TimeTicks::Now();
 
-  int skipped_frames = 0;
-
   // Target time for when PumpSamples() should run.
   base::TimeTicks target_time = playback_time - min_lead_time_ - kLeadTimeExtra;
 
-  // Check if it's too late to send the next packet. If it is, then advance
-  // current stream position, adding kLeadTimeExtra to ensure the next packet
-  // doesn't miss the deadline.
-  auto lead_time = playback_time - now;
-  if (lead_time < min_lead_time_) {
-    auto new_playback_time = now + min_lead_time_ + kLeadTimeExtra;
-    auto skipped_time = new_playback_time - playback_time;
-    skipped_frames =
-        AudioTimestampHelper::TimeToFrames(skipped_time, params_.sample_rate());
-    media_pos_frames_ += skipped_frames;
-    target_time = now;
-    playback_time += skipped_time;
-  }
-
   base::TimeDelta delay = target_time - now;
   pump_samples_timer_.Start(
       FROM_HERE, delay,
       base::BindOnce(&FuchsiaAudioOutputDevice::PumpSamples, this,
-                     playback_time, skipped_frames));
+                     playback_time));
 }
 
-void FuchsiaAudioOutputDevice::PumpSamples(base::TimeTicks playback_time,
-                                           int frames_skipped) {
+void FuchsiaAudioOutputDevice::PumpSamples(base::TimeTicks playback_time) {
   DCHECK(CurrentThreadIsRenderingThread());
 
   auto now = base::TimeTicks::Now();
 
-  // Check if the timer has missed the deadline. It doesn't make sense to try
-  // sending the packet in that case (it's likely to arrive too late).
-  // Reschedule the timer. In this case SchedulePumpSamples() is expected to
-  // schedule PumpSamples() to run immediately with frames_skipped > 0.
+  int skipped_frames = 0;
+
+  // Check if it's too late to send the next packet. If it is, then advance
+  // current stream position.
   auto lead_time = playback_time - now;
   if (lead_time < min_lead_time_) {
-    SchedulePumpSamples();
-    return;
+    auto new_playback_time = now + min_lead_time_;
+    auto skipped_time = new_playback_time - playback_time;
+    skipped_frames =
+        AudioTimestampHelper::TimeToFrames(skipped_time, params_.sample_rate());
+    media_pos_frames_ += skipped_frames;
+    playback_time += skipped_time;
   }
 
   int frames_filled;
@@ -436,7 +423,7 @@ void FuchsiaAudioOutputDevice::PumpSamples(base::TimeTicks playback_time,
     if (!callback_)
       return;
 
-    frames_filled = callback_->Render(playback_time - now, now, frames_skipped,
+    frames_filled = callback_->Render(playback_time - now, now, skipped_frames,
                                       audio_bus_.get());
   }
 

media/fuchsia/audio/fuchsia_audio_output_device.h

@@ -96,7 +96,7 @@ class MEDIA_EXPORT FuchsiaAudioOutputDevice : public AudioRendererSink {
 
   // Pumps a single packet to AudioConsumer and calls SchedulePumpSamples() to
   // pump the next packet.
-  void PumpSamples(base::TimeTicks playback_time, int frames_skipped);
+  void PumpSamples(base::TimeTicks playback_time);
 
   // Callback for StreamSink::SendPacket().
   void OnStreamSendDone(size_t buffer_index);

media/fuchsia/audio/fuchsia_audio_output_device_test.cc

@@ -98,8 +98,8 @@ class FuchsiaAudioOutputDeviceTest : public testing::Test {
   }
 
   void CallPumpSamples() {
-    output_device_->PumpSamples(
-        base::TimeTicks::Now() + base::TimeDelta::FromMilliseconds(200), 0);
+    output_device_->PumpSamples(base::TimeTicks::Now() +
+                                base::TimeDelta::FromMilliseconds(200));
   }
 
   void ValidatePresentationTime() {
@@ -194,8 +194,8 @@ TEST_F(FuchsiaAudioOutputDeviceTest, Underflow) {
   // Advance time by 100ms, causing some frames to be skipped.
   task_environment_.AdvanceClock(kPeriod * 10);
   task_environment_.RunUntilIdle();
-  EXPECT_EQ(renderer_.frames_rendered(), kFramesPerPeriod);
-  EXPECT_EQ(renderer_.frames_skipped(), kFramesPerPeriod * 9);
+  EXPECT_EQ(renderer_.frames_rendered(), kFramesPerPeriod * 3);
+  EXPECT_EQ(renderer_.frames_skipped(), kFramesPerPeriod * 7);
   renderer_.reset_frames_rendered();
 
   ValidatePresentationTime();