Fix VibratorService waveform accumulated delay

Change VibrateWaveformThread logic around delays while playing a
waveform to amortize processing/scheduling delays into the sleep part of
the thread. This should make the actual waveform duration closer to the
requested one.

Bug: 171133221
Test: atest FrameworksServicesTest:VibratorServiceTest
Change-Id: Iafe7f031444f68cd2cdc16883812874f385cba8e

services/core/java/com/android/server/VibratorService.java

@@ -1794,25 +1794,20 @@ public class VibratorService extends IVibratorService.Stub
             mWakeLock.setWorkSource(mTmpWorkSource);
         }
 
-        private long delayLocked(long duration) {
+        private void delayLocked(long wakeUpTime) {
             Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "delayLocked");
             try {
-                long durationRemaining = duration;
+                long durationRemaining = wakeUpTime - SystemClock.uptimeMillis();
-                if (duration > 0) {
+                while (durationRemaining > 0) {
-                    final long bedtime = duration + SystemClock.uptimeMillis();
+                    try {
-                    do {
+                        this.wait(durationRemaining);
-                        try {
+                    } catch (InterruptedException e) {
-                            this.wait(durationRemaining);
+                    }
-                        }
+                    if (mForceStop) {
-                        catch (InterruptedException e) { }
+                        break;
-                        if (mForceStop) {
+                    }
-                            break;
+                    durationRemaining = wakeUpTime - SystemClock.uptimeMillis();
-                        }
-                        durationRemaining = bedtime - SystemClock.uptimeMillis();
-                    } while (durationRemaining > 0);
-                    return duration - durationRemaining;
                 }
-                return 0;
             } finally {
                 Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR);
             }
@@ -1846,7 +1841,8 @@ public class VibratorService extends IVibratorService.Stub
                     final int repeat = mWaveform.getRepeatIndex();
 
                     int index = 0;
-                    long onDuration = 0;
+                    long nextStepStartTime = SystemClock.uptimeMillis();
+                    long nextVibratorStopTime = 0;
                     while (!mForceStop) {
                         if (index < len) {
                             final int amplitude = amplitudes[index];
@@ -1855,27 +1851,33 @@ public class VibratorService extends IVibratorService.Stub
                                 continue;
                             }
                             if (amplitude != 0) {
-                                if (onDuration <= 0) {
+                                long now = SystemClock.uptimeMillis();
+                                if (nextVibratorStopTime <= now) {
                                     // Telling the vibrator to start multiple times usually causes
                                     // effects to feel "choppy" because the motor resets at every on
                                     // command.  Instead we figure out how long our next "on" period
                                     // is going to be, tell the motor to stay on for the full
                                     // duration, and then wake up to change the amplitude at the
                                     // appropriate intervals.
-                                    onDuration = getTotalOnDuration(timings, amplitudes, index - 1,
+                                    long onDuration = getTotalOnDuration(
-                                            repeat);
+                                            timings, amplitudes, index - 1, repeat);
                                     mVibration.effect = VibrationEffect.createOneShot(
                                             onDuration, amplitude);
                                     doVibratorOn(mVibration);
+                                    nextVibratorStopTime = now + onDuration;
                                 } else {
+                                    // Vibrator is already ON, so just change its amplitude.
                                     doVibratorSetAmplitude(amplitude);
                                 }
                             }
 
-                            long waitTime = delayLocked(duration);
+                            // We wait until the time this waveform step was supposed to end,
-                            if (amplitude != 0) {
+                            // calculated from the time it was supposed to start. All start times
-                                onDuration -= waitTime;
+                            // are calculated from the waveform original start time by adding the
-                            }
+                            // input durations. Any scheduling or processing delay should not affect
+                            // this step's perceived total duration. They will be amortized here.
+                            nextStepStartTime += duration;
+                            delayLocked(nextStepStartTime);
                         } else if (repeat < 0) {
                             break;
                         } else {

services/tests/servicestests/src/com/android/server/VibratorServiceTest.java

@@ -52,6 +52,8 @@ import android.os.PowerManager;
 import android.os.PowerManagerInternal;
 import android.os.PowerSaveState;
 import android.os.Process;
+import android.os.RemoteException;
+import android.os.SystemClock;
 import android.os.UserHandle;
 import android.os.VibrationAttributes;
 import android.os.VibrationEffect;
@@ -76,7 +78,11 @@ import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnit;
 import org.mockito.junit.MockitoRule;
 
+import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 
 /**
  * Tests for {@link VibratorService}.
@@ -443,6 +449,42 @@ public class VibratorServiceTest {
         verify(mNativeWrapperMock).vibratorSetAmplitude(eq(50));
     }
 
+    @Test
+    public void vibrate_withWaveform_totalVibrationTimeRespected() throws Exception {
+        int totalDuration = 10_000; // 10s
+        int stepDuration = 25; // 25ms
+
+        // 25% of the first waveform step will be spent on the native on() call.
+        mockVibratorCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
+        doAnswer(invocation -> {
+            Thread.currentThread().sleep(stepDuration / 4);
+            return null;
+        }).when(mNativeWrapperMock).vibratorOn(anyLong(), anyLong());
+        // 25% of each waveform step will be spent on the native setAmplitude() call..
+        doAnswer(invocation -> {
+            Thread.currentThread().sleep(stepDuration / 4);
+            return null;
+        }).when(mNativeWrapperMock).vibratorSetAmplitude(anyInt());
+
+        VibratorService service = createService();
+
+        int stepCount = totalDuration / stepDuration;
+        long[] timings = new long[stepCount];
+        int[] amplitudes = new int[stepCount];
+        Arrays.fill(timings, stepDuration);
+        Arrays.fill(amplitudes, VibrationEffect.DEFAULT_AMPLITUDE);
+        VibrationEffect effect = VibrationEffect.createWaveform(timings, amplitudes, -1);
+
+        int perceivedDuration = vibrateAndMeasure(service, effect, /* timeoutSecs= */ 15);
+        int delay = Math.abs(perceivedDuration - totalDuration);
+
+        // Allow some delay for thread scheduling and callback triggering.
+        int maxDelay = (int) (0.05 * totalDuration); // < 5% of total duration
+        assertTrue("Waveform with perceived delay of " + delay + "ms,"
+                        + " expected less than " + maxDelay + "ms",
+                delay < maxDelay);
+    }
+
     @Test
     public void vibrate_withOneShotAndNativeCallbackTriggered_finishesVibration() {
         VibratorService service = createService();
@@ -699,16 +741,41 @@ public class VibratorServiceTest {
         vibrate(service, effect, ALARM_ATTRS);
     }
 
-    private void vibrate(VibratorService service, VibrationEffect effect, AudioAttributes attrs) {
-        VibrationAttributes attributes = new VibrationAttributes.Builder(attrs, effect).build();
-        vibrate(service, effect, attributes);
-    }
-
     private void vibrate(VibratorService service, VibrationEffect effect,
             VibrationAttributes attributes) {
         service.vibrate(UID, PACKAGE_NAME, effect, attributes, "some reason", service);
     }
 
+    private int vibrateAndMeasure(
+            VibratorService service, VibrationEffect effect, long timeoutSecs) throws Exception {
+        AtomicLong startTime = new AtomicLong(0);
+        AtomicLong endTime = new AtomicLong(0);
+        CountDownLatch startedCount = new CountDownLatch(1);
+        CountDownLatch finishedCount = new CountDownLatch(1);
+        service.registerVibratorStateListener(new IVibratorStateListener() {
+            @Override
+            public void onVibrating(boolean vibrating) throws RemoteException {
+                if (vibrating) {
+                    startTime.set(SystemClock.uptimeMillis());
+                    startedCount.countDown();
+                } else if (startedCount.getCount() == 0) {
+                    endTime.set(SystemClock.uptimeMillis());
+                    finishedCount.countDown();
+                }
+            }
+
+            @Override
+            public IBinder asBinder() {
+                return mVibratorStateListenerBinderMock;
+            }
+        });
+
+        vibrate(service, effect);
+
+        assertTrue(finishedCount.await(timeoutSecs, TimeUnit.SECONDS));
+        return (int) (endTime.get() - startTime.get());
+    }
+
     private void mockVibratorCapabilities(int capabilities) {
         when(mNativeWrapperMock.vibratorGetCapabilities()).thenReturn((long) capabilities);
     }