From 934559ace989f0153cb02e92f417d1d4b52e8850 Mon Sep 17 00:00:00 2001 From: Lais Andrade Date: Thu, 22 Oct 2020 18:35:24 +0000 Subject: [PATCH] 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 --- .../com/android/server/VibratorService.java | 50 ++++++------ .../android/server/VibratorServiceTest.java | 77 +++++++++++++++++-- 2 files changed, 98 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/VibratorService.java b/services/core/java/com/android/server/VibratorService.java index afddd650c46c2..74fba0441be80 100644 --- a/services/core/java/com/android/server/VibratorService.java +++ b/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; - if (duration > 0) { - final long bedtime = duration + SystemClock.uptimeMillis(); - do { - try { - this.wait(durationRemaining); - } - catch (InterruptedException e) { } - if (mForceStop) { - break; - } - durationRemaining = bedtime - SystemClock.uptimeMillis(); - } while (durationRemaining > 0); - return duration - durationRemaining; + long durationRemaining = wakeUpTime - SystemClock.uptimeMillis(); + while (durationRemaining > 0) { + try { + this.wait(durationRemaining); + } catch (InterruptedException e) { + } + if (mForceStop) { + break; + } + durationRemaining = wakeUpTime - SystemClock.uptimeMillis(); } - 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, - repeat); + long onDuration = getTotalOnDuration( + 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); - if (amplitude != 0) { - onDuration -= waitTime; - } + // We wait until the time this waveform step was supposed to end, + // calculated from the time it was supposed to start. All start times + // 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 { diff --git a/services/tests/servicestests/src/com/android/server/VibratorServiceTest.java b/services/tests/servicestests/src/com/android/server/VibratorServiceTest.java index 8d60de9abe99f..fba321e072072 100644 --- a/services/tests/servicestests/src/com/android/server/VibratorServiceTest.java +++ b/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); }