Change parameter type for volume to float in AudioMixer

Change-Id: I4da1505ce852505f86f8e5b87f60e8edceeb30e0

4 files changed, 41 insertions, 46 deletions

diff --git a/services/audioflinger/AudioMixer.cpp b/services/audioflinger/AudioMixer.cpp
index 2a8ece6..d73292e 100644
--- a/services/audioflinger/AudioMixer.cpp
+++ b/services/audioflinger/AudioMixer.cpp
@@ -543,7 +543,7 @@ void AudioMixer::disable(int name)
  * set by ramp, which is either 0 for immediate, or typically one state
  * framecount period.
  *
- * @param newValue new volume target in U4.12.
+ * @param newFloatValue new volume target in float [0.0, 1.0].
  * @param ramp number of frames to increment over. ramp is 0 if the volume
  * should be set immediately.
  * @param volume reference to the U4.12 target volume, set on return.
@@ -551,8 +551,15 @@ void AudioMixer::disable(int name)
  * @param volumeInc reference to the increment per output audio frame, set on return.
  * @return true if the volume has changed, false if volume is same.
  */
-static inline bool setVolumeRampVariables(int32_t newValue, int32_t ramp,
+static inline bool setVolumeRampVariables(float newFloatValue, int32_t ramp,
         int16_t &volume, int32_t &prevVolume, int32_t &volumeInc) {
+    int32_t newValue = newFloatValue * AudioMixer::UNITY_GAIN_INT;
+    if (newValue > AudioMixer::UNITY_GAIN_INT) {
+        newValue = AudioMixer::UNITY_GAIN_INT;
+    } else if (newValue < 0) {
+        ALOGE("negative volume %.7g", newFloatValue);
+        newValue = 0; // should never happen, but for safety check.
+    }
     if (newValue == volume) {
         return false;
     }
@@ -668,22 +675,23 @@ void AudioMixer::setParameter(int name, int target, int param, void *value)
         switch (param) {
         case VOLUME0:
         case VOLUME1:
-            if (setVolumeRampVariables(valueInt,
+            if (setVolumeRampVariables(*reinterpret_cast<float*>(value),
                     target == RAMP_VOLUME ? mState.frameCount : 0,
                     track.volume[param - VOLUME0], track.prevVolume[param - VOLUME0],
                     track.volumeInc[param - VOLUME0])) {
                 ALOGV("setParameter(%s, VOLUME%d: %04x)",
-                        target == VOLUME ? "VOLUME" : "RAMP_VOLUME", param - VOLUME0, valueInt);
+                        target == VOLUME ? "VOLUME" : "RAMP_VOLUME", param - VOLUME0,
+                                track.volume[param - VOLUME0]);
                 invalidateState(1 << name);
             }
             break;
         case AUXLEVEL:
             //ALOG_ASSERT(0 <= valueInt && valueInt <= MAX_GAIN_INT, "bad aux level %d", valueInt);
-            if (setVolumeRampVariables(valueInt,
+            if (setVolumeRampVariables(*reinterpret_cast<float*>(value),
                     target == RAMP_VOLUME ? mState.frameCount : 0,
                     track.auxLevel, track.prevAuxLevel, track.auxInc)) {
                 ALOGV("setParameter(%s, AUXLEVEL: %04x)",
-                        target == VOLUME ? "VOLUME" : "RAMP_VOLUME", valueInt);
+                        target == VOLUME ? "VOLUME" : "RAMP_VOLUME", track.auxLevel);
                 invalidateState(1 << name);
             }
             break;
diff --git a/services/audioflinger/AudioMixer.h b/services/audioflinger/AudioMixer.h
index 43fe664..766ff60 100644
--- a/services/audioflinger/AudioMixer.h
+++ b/services/audioflinger/AudioMixer.h
@@ -59,6 +59,7 @@ public:
     static const uint32_t MAX_NUM_CHANNELS_TO_DOWNMIX = 8;
 
     static const uint16_t UNITY_GAIN_INT = 0x1000;
+    static const float    UNITY_GAIN_FLOAT = 1.0f;
 
     enum { // names
 
diff --git a/services/audioflinger/FastMixer.cpp b/services/audioflinger/FastMixer.cpp
index 13b21ec..c486630 100644
--- a/services/audioflinger/FastMixer.cpp
+++ b/services/audioflinger/FastMixer.cpp
@@ -273,10 +273,9 @@ void FastMixer::onStateChange()
                     ALOG_ASSERT(name >= 0);
                     mixer->setBufferProvider(name, bufferProvider);
                     if (fastTrack->mVolumeProvider == NULL) {
-                        mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME0,
-                                (void *) MAX_GAIN_INT);
-                        mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME1,
-                                (void *) MAX_GAIN_INT);
+                        float f = AudioMixer::UNITY_GAIN_FLOAT;
+                        mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME0, &f);
+                        mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME1, &f);
                     }
                     mixer->setParameter(name, AudioMixer::RESAMPLE,
                             AudioMixer::REMOVE, NULL);
@@ -336,12 +335,11 @@ void FastMixer::onWork()
             ALOG_ASSERT(name >= 0);
             if (fastTrack->mVolumeProvider != NULL) {
                 gain_minifloat_packed_t vlr = fastTrack->mVolumeProvider->getVolumeLR();
-                mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME0,
-                        (void *) (uintptr_t)
-                            (float_from_gain(gain_minifloat_unpack_left(vlr)) * MAX_GAIN_INT));
-                mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME1,
-                        (void *) (uintptr_t)
-                            (float_from_gain(gain_minifloat_unpack_right(vlr)) * MAX_GAIN_INT));
+                float vlf = float_from_gain(gain_minifloat_unpack_left(vlr));
+                float vrf = float_from_gain(gain_minifloat_unpack_right(vlr));
+
+                mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME0, &vlf);
+                mixer->setParameter(name, AudioMixer::VOLUME, AudioMixer::VOLUME1, &vrf);
             }
             // FIXME The current implementation of framesReady() for fast tracks
             // takes a tryLock, which can block
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 742163b..89f735b 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -3369,9 +3369,11 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac
             }
 
             // compute volume for this track
-            uint32_t vl, vr, va;
+            uint32_t vl, vr;       // in U8.24 integer format
+            float vlf, vrf, vaf;   // in [0.0, 1.0] float format
             if (track->isPausing() || mStreamTypes[track->streamType()].mute) {
-                vl = vr = va = 0;
+                vl = vr = 0;
+                vlf = vrf = vaf = 0.;
                 if (track->isPausing()) {
                     track->setPaused();
                 }
@@ -3382,8 +3384,8 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac
                 float v = masterVolume * typeVolume;
                 AudioTrackServerProxy *proxy = track->mAudioTrackServerProxy;
                 gain_minifloat_packed_t vlr = proxy->getVolumeLR();
-                float vlf = float_from_gain(gain_minifloat_unpack_left(vlr));
-                float vrf = float_from_gain(gain_minifloat_unpack_right(vlr));
+                vlf = float_from_gain(gain_minifloat_unpack_left(vlr));
+                vrf = float_from_gain(gain_minifloat_unpack_right(vlr));
                 // track volumes come from shared memory, so can't be trusted and must be clamped
                 if (vlf > GAIN_FLOAT_UNITY) {
                     ALOGV("Track left volume out of range: %.3g", vlf);
@@ -3394,20 +3396,22 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac
                     vrf = GAIN_FLOAT_UNITY;
                 }
                 // now apply the master volume and stream type volume
-                // FIXME we're losing the wonderful dynamic range in the minifloat representation
-                float v8_24 = v * (MAX_GAIN_INT * MAX_GAIN_INT);
-                vl = (uint32_t) (v8_24 * vlf);
-                vr = (uint32_t) (v8_24 * vrf);
+                vlf *= v;
+                vrf *= v;
                 // assuming master volume and stream type volume each go up to 1.0,
-                // vl and vr are now in 8.24 format
-
+                // then derive vl and vr as U8.24 versions for the effect chain
+                const float scaleto8_24 = MAX_GAIN_INT * MAX_GAIN_INT;
+                vl = (uint32_t) (scaleto8_24 * vlf);
+                vr = (uint32_t) (scaleto8_24 * vrf);
+                // vl and vr are now in U8.24 format
                 uint16_t sendLevel = proxy->getSendLevel_U4_12();
                 // send level comes from shared memory and so may be corrupt
                 if (sendLevel > MAX_GAIN_INT) {
                     ALOGV("Track send level out of range: %04X", sendLevel);
                     sendLevel = MAX_GAIN_INT;
                 }
-                va = (uint32_t)(v * sendLevel);
+                // vaf is represented as [0.0, 1.0] float by rescaling sendLevel
+                vaf = v * sendLevel * (1. / MAX_GAIN_INT);
             }
 
             // Delegate volume control to effect in track effect chain if needed
@@ -3424,29 +3428,13 @@ AudioFlinger::PlaybackThread::mixer_state AudioFlinger::MixerThread::prepareTrac
                 track->mHasVolumeController = false;
             }
 
-            // FIXME Use float
-            // Convert volumes from 8.24 to 4.12 format
-            // This additional clamping is needed in case chain->setVolume_l() overshot
-            vl = (vl + (1 << 11)) >> 12;
-            if (vl > MAX_GAIN_INT) {
-                vl = MAX_GAIN_INT;
-            }
-            vr = (vr + (1 << 11)) >> 12;
-            if (vr > MAX_GAIN_INT) {
-                vr = MAX_GAIN_INT;
-            }
-
-            if (va > MAX_GAIN_INT) {
-                va = MAX_GAIN_INT;   // va is uint32_t, so no need to check for -
-            }
-
             // XXX: these things DON'T need to be done each time
             mAudioMixer->setBufferProvider(name, track);
             mAudioMixer->enable(name);
 
-            mAudioMixer->setParameter(name, param, AudioMixer::VOLUME0, (void *)(uintptr_t)vl);
-            mAudioMixer->setParameter(name, param, AudioMixer::VOLUME1, (void *)(uintptr_t)vr);
-            mAudioMixer->setParameter(name, param, AudioMixer::AUXLEVEL, (void *)(uintptr_t)va);
+            mAudioMixer->setParameter(name, param, AudioMixer::VOLUME0, &vlf);
+            mAudioMixer->setParameter(name, param, AudioMixer::VOLUME1, &vrf);
+            mAudioMixer->setParameter(name, param, AudioMixer::AUXLEVEL, &vaf);
             mAudioMixer->setParameter(
                 name,
                 AudioMixer::TRACK,