VolumeShaper: Fixes for updated Cts test

1) Limit number of VolumeShapers that can be created.
   a) 16 system VolumeShapers
   b) 16 application/client VolumeShapers
2) Return proper volume before VolumeShaper is started.
3) Consistent xOffset definition used internally:
   a) this is now always the position on the volumeshaper curve
      which can go backwards if in REVERSE.
   b) normalized time is always forward going and is scaled
      to 0.f and 1.f depending on progress relative to
      the curve's duration.
4) Fix replace method.
5) Add comments.

Test: Use updated CTS VolumeShaperTest
Bug: 37536598
Change-Id: I837ab2a481adc0abbd3f1338bfe2cb79831b11fa
diff --git a/include/media/Interpolator.h b/include/media/Interpolator.h
index ad6fb25..45a0585 100644
--- a/include/media/Interpolator.h
+++ b/include/media/Interpolator.h
@@ -294,12 +294,21 @@
 
     std::string toString() const {
         std::stringstream ss;
-        ss << "mInterpolatorType: " << static_cast<int32_t>(mInterpolatorType) << std::endl;
-        ss << "mFirstSlope: " << mFirstSlope << std::endl;
-        ss << "mLastSlope: " << mLastSlope << std::endl;
+        ss << "Interpolator{mInterpolatorType=" << static_cast<int32_t>(mInterpolatorType);
+        ss << ", mFirstSlope=" << mFirstSlope;
+        ss << ", mLastSlope=" << mLastSlope;
+        ss << ", {";
+        bool first = true;
         for (const auto &pt : *this) {
-            ss << pt.first << " " << pt.second << std::endl;
+            if (first) {
+                first = false;
+                ss << "{";
+            } else {
+                ss << ", {";
+            }
+            ss << pt.first << ", " << pt.second << "}";
         }
+        ss << "}}";
         return ss.str();
     }
 
@@ -324,7 +333,7 @@
 
     // spline cubic polynomial coefficient cache
     std::unordered_map<S, std::tuple<S /* c1 */, S /* c2 */, S /* c3 */>> mMemo;
-};
+}; // Interpolator
 
 } // namespace android
 
diff --git a/include/media/VolumeShaper.h b/include/media/VolumeShaper.h
index e646031..e4c0b5b 100644
--- a/include/media/VolumeShaper.h
+++ b/include/media/VolumeShaper.h
@@ -51,31 +51,68 @@
 
 class VolumeShaper {
 public:
-    using S = float;
-    using T = float;
+    // S and T are like template typenames (matching the Interpolator<S, T>)
+    using S = float; // time type
+    using T = float; // volume type
 
-    static const int kSystemIdMax = 16;
+// Curve and dimension information
+// TODO: member static const or constexpr float initialization not permitted in C++11
+#define MIN_CURVE_TIME    0.f  // type S: start of VolumeShaper curve (normalized)
+#define MAX_CURVE_TIME    1.f  // type S: end of VolumeShaper curve (normalized)
+#define MIN_LINEAR_VOLUME 0.f  // type T: silence / mute audio
+#define MAX_LINEAR_VOLUME 1.f  // type T: max volume, unity gain
+#define MAX_LOG_VOLUME    0.f  // type T: max volume, unity gain in dBFS
 
-    // VolumeShaper::Status is equivalent to status_t if negative
-    // but if non-negative represents the id operated on.
-    // It must be expressible as an int32_t for binder purposes.
+    /* kSystemVolumeShapersMax is the maximum number of system VolumeShapers.
+     * Each system VolumeShapers has a predefined Id, which ranges from 0
+     * to kSystemVolumeShapersMax - 1 and is unique for its usage.
+     *
+     * "1" is reserved for system ducking.
+     */
+    static const int kSystemVolumeShapersMax = 16;
+
+    /* kUserVolumeShapersMax is the maximum number of application
+     * VolumeShapers for a player/track.  Application VolumeShapers are
+     * assigned on creation by the client, and have Ids ranging
+     * from kSystemVolumeShapersMax to INT32_MAX.
+     *
+     * The number of user/application volume shapers is independent to the
+     * system volume shapers. If an application tries to create more than
+     * kUserVolumeShapersMax to a player, then the apply() will fail.
+     * This prevents exhausting server side resources by a potentially malicious
+     * application.
+     */
+    static const int kUserVolumeShapersMax = 16;
+
+    /* VolumeShaper::Status is equivalent to status_t if negative
+     * but if non-negative represents the id operated on.
+     * It must be expressible as an int32_t for binder purposes.
+     */
     using Status = status_t;
 
+    // Local definition for clamp as std::clamp is included in C++17 only.
+    // TODO: use the std::clamp version when Android build uses C++17.
+    template<typename R>
+    static constexpr const R &clamp(const R &v, const R &lo, const R &hi) {
+        return (v < lo) ? lo : (hi < v) ? hi : v;
+    }
+
+    /* VolumeShaper.Configuration derives from the Interpolator class and adds
+     * parameters relating to the volume shape.
+     *
+     * This parallels the Java implementation and the enums must match.
+     * See "frameworks/base/media/java/android/media/VolumeShaper.java" for
+     * details on the Java implementation.
+     */
     class Configuration : public Interpolator<S, T>, public RefBase {
     public:
-        /* VolumeShaper.Configuration derives from the Interpolator class and adds
-         * parameters relating to the volume shape.
-         */
-
-        // TODO document as per VolumeShaper.java flags.
-
-        // must match with VolumeShaper.java in frameworks/base
+        // Must match with VolumeShaper.java in frameworks/base.
         enum Type : int32_t {
             TYPE_ID,
             TYPE_SCALE,
         };
 
-        // must match with VolumeShaper.java in frameworks/base
+        // Must match with VolumeShaper.java in frameworks/base.
         enum OptionFlag : int32_t {
             OPTION_FLAG_NONE           = 0,
             OPTION_FLAG_VOLUME_IN_DBFS = (1 << 0),
@@ -84,7 +121,7 @@
             OPTION_FLAG_ALL            = (OPTION_FLAG_VOLUME_IN_DBFS | OPTION_FLAG_CLOCK_TIME),
         };
 
-        // bring to derived class; must match with VolumeShaper.java in frameworks/base
+        // Bring from base class; must match with VolumeShaper.java in frameworks/base.
         using InterpolatorType = Interpolator<S, T>::InterpolatorType;
 
         Configuration()
@@ -138,8 +175,13 @@
             return mDurationMs;
         }
 
-        void setDurationMs(double durationMs) {
-            mDurationMs = durationMs;
+        status_t setDurationMs(double durationMs) {
+            if (durationMs > 0.) {
+                mDurationMs = durationMs;
+                return NO_ERROR;
+            }
+            // zero, negative, or nan. These values not possible from Java.
+            return BAD_VALUE;
         }
 
         int32_t getId() const {
@@ -147,45 +189,46 @@
         }
 
         void setId(int32_t id) {
+            // We permit a negative id here (representing invalid).
             mId = id;
         }
 
+        /* Adjust the volume to be in linear range from MIN_LINEAR_VOLUME to MAX_LINEAR_VOLUME
+         * and compensate for log dbFS volume as needed.
+         */
         T adjustVolume(T volume) const {
             if ((getOptionFlags() & OPTION_FLAG_VOLUME_IN_DBFS) != 0) {
-                const T out = powf(10.f, volume / 10.0f);
+                const T out = powf(10.f, volume / 10.f);
                 VS_LOG("in: %f  out: %f", volume, out);
                 volume = out;
             }
-            // clamp
-            if (volume < 0.f) {
-                volume = 0.f;
-            } else if (volume > 1.f) {
-                volume = 1.f;
-            }
-            return volume;
+            return clamp(volume, MIN_LINEAR_VOLUME /* lo */, MAX_LINEAR_VOLUME /* hi */);
         }
 
-        status_t checkCurve() {
+        /* Check if the existing curve is valid.
+         */
+        status_t checkCurve() const {
             if (mType == TYPE_ID) return NO_ERROR;
             if (this->size() < 2) {
                 ALOGE("curve must have at least 2 points");
                 return BAD_VALUE;
             }
-            if (first().first != 0.f || last().first != 1.f) {
-                ALOGE("curve must start at 0.f and end at 1.f");
+            if (first().first != MIN_CURVE_TIME || last().first != MAX_CURVE_TIME) {
+                ALOGE("curve must start at MIN_CURVE_TIME and end at MAX_CURVE_TIME");
                 return BAD_VALUE;
             }
             if ((getOptionFlags() & OPTION_FLAG_VOLUME_IN_DBFS) != 0) {
                 for (const auto &pt : *this) {
-                    if (!(pt.second <= 0.f) /* handle nan */) {
+                    if (!(pt.second <= MAX_LOG_VOLUME) /* handle nan */) {
                         ALOGE("positive volume dbFS");
                         return BAD_VALUE;
                     }
                 }
             } else {
                 for (const auto &pt : *this) {
-                    if (!(pt.second >= 0.f) || !(pt.second <= 1.f) /* handle nan */) {
-                        ALOGE("volume < 0.f or > 1.f");
+                    if (!(pt.second >= MIN_LINEAR_VOLUME)
+                            || !(pt.second <= MAX_LINEAR_VOLUME) /* handle nan */) {
+                        ALOGE("volume < MIN_LINEAR_VOLUME or > MAX_LINEAR_VOLUME");
                         return BAD_VALUE;
                     }
                 }
@@ -193,19 +236,22 @@
             return NO_ERROR;
         }
 
+        /* Clamps the volume curve in the configuration to
+         * the valid range for log or linear scale.
+         */
         void clampVolume() {
             if ((mOptionFlags & OPTION_FLAG_VOLUME_IN_DBFS) != 0) {
                 for (auto it = this->begin(); it != this->end(); ++it) {
-                    if (!(it->second <= 0.f) /* handle nan */) {
-                        it->second = 0.f;
+                    if (!(it->second <= MAX_LOG_VOLUME) /* handle nan */) {
+                        it->second = MAX_LOG_VOLUME;
                     }
                 }
             } else {
                 for (auto it = this->begin(); it != this->end(); ++it) {
-                    if (!(it->second >= 0.f) /* handle nan */) {
-                        it->second = 0.f;
-                    } else if (!(it->second <= 1.f)) {
-                        it->second = 1.f;
+                    if (!(it->second >= MIN_LINEAR_VOLUME) /* handle nan */) {
+                        it->second = MIN_LINEAR_VOLUME;
+                    } else if (!(it->second <= MAX_LINEAR_VOLUME)) {
+                        it->second = MAX_LINEAR_VOLUME;
                     }
                 }
             }
@@ -226,8 +272,9 @@
             if (endVolume == startVolume) {
                 // match with linear ramp
                 const T offset = volume - startVolume;
+                static const T scale =  1.f / (MAX_CURVE_TIME - MIN_CURVE_TIME); // nominally 1.f
                 for (auto it = this->begin(); it != this->end(); ++it) {
-                    it->second = it->second + offset * (1.f - it->first);
+                    it->second = it->second + offset * (MAX_CURVE_TIME - it->first) * scale;
                 }
             } else {
                 const T  scale = (volume - endVolume) / (startVolume - endVolume);
@@ -264,32 +311,40 @@
                             ?: checkCurve();
         }
 
+        // Returns a string for debug printing.
         std::string toString() const {
             std::stringstream ss;
-            ss << "mType: " << static_cast<int32_t>(mType) << std::endl;
-            ss << "mId: " << mId << std::endl;
+            ss << "VolumeShaper::Configuration{mType=" << static_cast<int32_t>(mType);
+            ss << ", mId=" << mId;
             if (mType != TYPE_ID) {
-                ss << "mOptionFlags: " << static_cast<int32_t>(mOptionFlags) << std::endl;
-                ss << "mDurationMs: " << mDurationMs << std::endl;
-                ss << Interpolator<S, T>::toString().c_str();
+                ss << ", mOptionFlags=" << static_cast<int32_t>(mOptionFlags);
+                ss << ", mDurationMs=" << mDurationMs;
+                ss << ", " << Interpolator<S, T>::toString().c_str();
             }
+            ss << "}";
             return ss.str();
         }
 
     private:
-        Type mType;
-        int32_t mId;
-        OptionFlag mOptionFlags;
-        double mDurationMs;
+        Type mType;              // type of configuration
+        int32_t mId;             // A valid id is >= 0.
+        OptionFlag mOptionFlags; // option flags for the configuration.
+        double mDurationMs;      // duration, must be > 0; default is 1000 ms.
     }; // Configuration
 
-    // must match with VolumeShaper.java in frameworks/base
-    // TODO document per VolumeShaper.java flags.
+    /* VolumeShaper::Operation expresses an operation to perform on the
+     * configuration (either explicitly specified or an id).
+     *
+     * This parallels the Java implementation and the enums must match.
+     * See "frameworks/base/media/java/android/media/VolumeShaper.java" for
+     * details on the Java implementation.
+     */
     class Operation : public RefBase {
     public:
+        // Must match with VolumeShaper.java.
         enum Flag : int32_t {
             FLAG_NONE      = 0,
-            FLAG_REVERSE   = (1 << 0),
+            FLAG_REVERSE   = (1 << 0), // the absence of this indicates "play"
             FLAG_TERMINATE = (1 << 1),
             FLAG_JOIN      = (1 << 2),
             FLAG_DELAY     = (1 << 3),
@@ -334,13 +389,29 @@
         }
 
         void setXOffset(S xOffset) {
-            mXOffset = xOffset;
+            mXOffset = clamp(xOffset, MIN_CURVE_TIME /* lo */, MAX_CURVE_TIME /* hi */);
         }
 
         Flag getFlags() const {
             return mFlags;
         }
 
+        /* xOffset is the position on the volume curve and may go backwards
+         * if you are in reverse mode. This must be in the range from
+         * [MIN_CURVE_TIME, MAX_CURVE_TIME].
+         *
+         * normalizedTime always increases as time or framecount increases.
+         * normalizedTime is nominally from MIN_CURVE_TIME to MAX_CURVE_TIME when
+         * running through the curve, but could be outside this range afterwards.
+         * If you are reversing, this means the position on the curve, or xOffset,
+         * is computed as MAX_CURVE_TIME - normalizedTime, clamped to
+         * [MIN_CURVE_TIME, MAX_CURVE_TIME].
+         */
+        void setNormalizedTime(S normalizedTime) {
+            setXOffset((mFlags & FLAG_REVERSE) != 0
+                    ? MAX_CURVE_TIME - normalizedTime : normalizedTime);
+        }
+
         status_t setFlags(Flag flags) {
             if ((flags & ~FLAG_ALL) != 0) {
                 ALOGE("flags has invalid bits: %#x", flags);
@@ -367,19 +438,26 @@
 
         std::string toString() const {
             std::stringstream ss;
-            ss << "mFlags: " << static_cast<int32_t>(mFlags) << std::endl;
-            ss << "mReplaceId: " << mReplaceId << std::endl;
-            ss << "mXOffset: " << mXOffset << std::endl;
+            ss << "VolumeShaper::Operation{mFlags=" << static_cast<int32_t>(mFlags) ;
+            ss << ", mReplaceId=" << mReplaceId;
+            ss << ", mXOffset=" << mXOffset;
+            ss << "}";
             return ss.str();
         }
 
     private:
-        Flag mFlags;
-        int32_t mReplaceId;
-        S mXOffset;
+        Flag mFlags;        // operation to do
+        int32_t mReplaceId; // if >= 0 the id to remove in a replace operation.
+        S mXOffset;         // position in the curve to set if a valid number (not nan)
     }; // Operation
 
-    // must match with VolumeShaper.java in frameworks/base
+    /* VolumeShaper.State is returned when requesting the last
+     * state of the VolumeShaper.
+     *
+     * This parallels the Java implementation.
+     * See "frameworks/base/media/java/android/media/VolumeShaper.java" for
+     * details on the Java implementation.
+     */
     class State : public RefBase {
     public:
         State(T volume, S xOffset)
@@ -388,7 +466,7 @@
         }
 
         State()
-            : State(-1.f, -1.f) { }
+            : State(NAN, NAN) { }
 
         T getVolume() const {
             return mVolume;
@@ -419,16 +497,18 @@
 
         std::string toString() const {
             std::stringstream ss;
-            ss << "mVolume: " << mVolume << std::endl;
-            ss << "mXOffset: " << mXOffset << std::endl;
+            ss << "VolumeShaper::State{mVolume=" << mVolume;
+            ss << ", mXOffset=" << mXOffset;
+            ss << "}";
             return ss.str();
         }
 
     private:
-        T mVolume;
-        S mXOffset;
+        T mVolume;   // linear volume in the range MIN_LINEAR_VOLUME to MAX_LINEAR_VOLUME
+        S mXOffset;  // position on curve expressed from MIN_CURVE_TIME to MAX_CURVE_TIME
     }; // State
 
+    // Internal helper class to do an affine transform for time and amplitude scaling.
     template <typename R>
     class Translate {
     public:
@@ -459,8 +539,9 @@
 
         std::string toString() const {
             std::stringstream ss;
-            ss << "mOffset: " << mOffset << std::endl;
-            ss << "mScale: " << mScale << std::endl;
+            ss << "VolumeShaper::Translate{mOffset=" << mOffset;
+            ss << ", mScale=" << mScale;
+            ss << "}";
             return ss.str();
         }
 
@@ -484,9 +565,14 @@
         return convertTimespecToUs(tv);
     }
 
-    // TODO: Since we pass configuration and operation as shared pointers
-    // there is a potential risk that the caller may modify these after
-    // delivery.  Currently, we don't require copies made here.
+    /* Native implementation of VolumeShaper.  This is NOT mirrored
+     * on the Java side, so we don't need to mimic Java side layout
+     * and data; furthermore, this isn't refcounted as a "RefBase" object.
+     *
+     * Since we pass configuration and operation as shared pointers (like
+     * Java) there is a potential risk that the caller may modify
+     * these after delivery.
+     */
     VolumeShaper(
             const sp<VolumeShaper::Configuration> &configuration,
             const sp<VolumeShaper::Operation> &operation)
@@ -494,54 +580,58 @@
         , mOperation(operation)         // ditto
         , mStartFrame(-1)
         , mLastVolume(T(1))
-        , mLastXOffset(0.f)
-        , mDelayXOffset(std::numeric_limits<S>::quiet_NaN()) {
+        , mLastXOffset(MIN_CURVE_TIME)
+        , mDelayXOffset(MIN_CURVE_TIME) {
         if (configuration.get() != nullptr
                 && (getFlags() & VolumeShaper::Operation::FLAG_DELAY) == 0) {
             mLastVolume = configuration->first().second;
         }
     }
 
-    void updatePosition(int64_t startFrame, double sampleRate) {
-        double scale = (mConfiguration->last().first - mConfiguration->first().first)
-                        / (mConfiguration->getDurationMs() * 0.001 * sampleRate);
-        const double minScale = 1. / static_cast<double>(INT64_MAX);
-        scale = std::max(scale, minScale);
-        const S xOffset = std::isnan(mDelayXOffset) ? mConfiguration->first().first : mDelayXOffset;
-        VS_LOG("update position: scale %lf  frameCount:%lld, sampleRate:%lf, xOffset:%f",
-                scale, (long long) startFrame, sampleRate, xOffset);
-
-        mXTranslate.setOffset(static_cast<float>(static_cast<double>(startFrame)
-                                                 - static_cast<double>(xOffset) / scale));
-        mXTranslate.setScale(static_cast<float>(scale));
-        VS_LOG("translate: %s", mXTranslate.toString().c_str());
-    }
-
     // We allow a null operation here, though VolumeHandler always provides one.
     VolumeShaper::Operation::Flag getFlags() const {
         return mOperation == nullptr
-                ? VolumeShaper::Operation::FLAG_NONE :mOperation->getFlags();
+                ? VolumeShaper::Operation::FLAG_NONE : mOperation->getFlags();
     }
 
+    /* Returns the last volume and xoffset reported to the AudioFlinger.
+     * If the VolumeShaper has not been started, compute what the volume
+     * should be based on the initial offset specified.
+     */
     sp<VolumeShaper::State> getState() const {
-        return new VolumeShaper::State(mLastVolume, mLastXOffset);
+        if (!isStarted()) {
+            const T volume = computeVolumeFromXOffset(mDelayXOffset);
+            VS_LOG("delayed VolumeShaper, using cached offset:%f for volume:%f",
+                    mDelayXOffset, volume);
+            return new VolumeShaper::State(volume, mDelayXOffset);
+        } else {
+            return new VolumeShaper::State(mLastVolume, mLastXOffset);
+        }
+    }
+
+    S getDelayXOffset() const {
+        return mDelayXOffset;
     }
 
     void setDelayXOffset(S xOffset) {
-        mDelayXOffset = xOffset;
+        mDelayXOffset = clamp(xOffset, MIN_CURVE_TIME /* lo */, MAX_CURVE_TIME /* hi */);
     }
 
     bool isStarted() const {
         return mStartFrame >= 0;
     }
 
+    /* getVolume() updates the last volume/xoffset state so it is not
+     * const, even though logically it may be viewed as const.
+     */
     std::pair<T /* volume */, bool /* active */> getVolume(
             int64_t trackFrameCount, double trackSampleRate) {
         if ((getFlags() & VolumeShaper::Operation::FLAG_DELAY) != 0) {
-            VS_LOG("delayed VolumeShaper, ignoring");
-            mLastVolume = T(1);
-            mLastXOffset = 0.;
-            return std::make_pair(T(1), false);
+            // We haven't had PLAY called yet, so just return the value
+            // as if PLAY were called just now.
+            VS_LOG("delayed VolumeShaper, using cached offset %f", mDelayXOffset);
+            const T volume = computeVolumeFromXOffset(mDelayXOffset);
+            return std::make_pair(volume, false);
         }
         const bool clockTime = (mConfiguration->getOptionFlags()
                 & VolumeShaper::Configuration::OPTION_FLAG_CLOCK_TIME) != 0;
@@ -549,84 +639,110 @@
         const double sampleRate = clockTime ? 1000000 : trackSampleRate;
 
         if (mStartFrame < 0) {
-            updatePosition(frameCount, sampleRate);
+            updatePosition(frameCount, sampleRate, mDelayXOffset);
             mStartFrame = frameCount;
         }
         VS_LOG("frameCount: %lld", (long long)frameCount);
-        S x = mXTranslate((T)frameCount);
-        VS_LOG("translation: %f", x);
+        const S x = mXTranslate((T)frameCount);
+        VS_LOG("translation to normalized time: %f", x);
 
-        // handle reversal of position
-        if (getFlags() & VolumeShaper::Operation::FLAG_REVERSE) {
-            x = 1.f - x;
-            VS_LOG("reversing to %f", x);
-            if (x < mConfiguration->first().first) {
-                mLastXOffset = 1.f;
-                const T volume = mConfiguration->adjustVolume(
-                        mConfiguration->first().second);  // persist last value
-                VS_LOG("persisting volume %f", volume);
-                mLastVolume = volume;
-                return std::make_pair(volume, false);
-            }
-            if (x > mConfiguration->last().first) {
-                mLastXOffset = 0.f;
-                mLastVolume = 1.f;
-                return std::make_pair(T(1), true); // too early
-            }
-        } else {
-            if (x < mConfiguration->first().first) {
-                mLastXOffset = 0.f;
-                mLastVolume = 1.f;
-                return std::make_pair(T(1), true); // too early
-            }
-            if (x > mConfiguration->last().first) {
-                mLastXOffset = 1.f;
-                const T volume = mConfiguration->adjustVolume(
-                        mConfiguration->last().second);  // persist last value
-                VS_LOG("persisting volume %f", volume);
-                mLastVolume = volume;
-                return std::make_pair(volume, false);
-            }
-        }
-        mLastXOffset = x;
-        // x contains the location on the volume curve to use.
-        const T unscaledVolume = mConfiguration->findY(x);
-        const T volume = mConfiguration->adjustVolume(unscaledVolume); // handle log scale
-        VS_LOG("volume: %f  unscaled: %f", volume, unscaledVolume);
-        mLastVolume = volume;
-        return std::make_pair(volume, true);
+        std::tuple<T /* volume */, S /* position */, bool /* active */> vt =
+                computeStateFromNormalizedTime(x);
+
+        mLastVolume = std::get<0>(vt);
+        mLastXOffset = std::get<1>(vt);
+        const bool active = std::get<2>(vt);
+        VS_LOG("rescaled time:%f  volume:%f  xOffset:%f  active:%s",
+                x, mLastVolume, mLastXOffset, active ? "true" : "false");
+        return std::make_pair(mLastVolume, active);
     }
 
     std::string toString() const {
         std::stringstream ss;
-        ss << "StartFrame: " << mStartFrame << std::endl;
-        ss << mXTranslate.toString().c_str();
-        if (mConfiguration.get() == nullptr) {
-            ss << "VolumeShaper::Configuration: nullptr" << std::endl;
-        } else {
-            ss << "VolumeShaper::Configuration:" << std::endl;
-            ss << mConfiguration->toString().c_str();
-        }
-        if (mOperation.get() == nullptr) {
-            ss << "VolumeShaper::Operation: nullptr" << std::endl;
-        } else {
-            ss << "VolumeShaper::Operation:" << std::endl;
-            ss << mOperation->toString().c_str();
-        }
+        ss << "VolumeShaper{mStartFrame=" << mStartFrame;
+        ss << ", mXTranslate=" << mXTranslate.toString().c_str();
+        ss << ", mConfiguration=" <<
+                (mConfiguration.get() == nullptr
+                        ? "nullptr" : mConfiguration->toString().c_str());
+        ss << ", mOperation=" <<
+                (mOperation.get() == nullptr
+                        ? "nullptr" :  mOperation->toString().c_str());
+        ss << "}";
         return ss.str();
     }
 
-    Translate<S> mXTranslate; // x axis translation from frames (in usec for clock time)
+    Translate<S> mXTranslate; // translation from frames (usec for clock time) to normalized time.
     sp<VolumeShaper::Configuration> mConfiguration;
     sp<VolumeShaper::Operation> mOperation;
+
+private:
     int64_t mStartFrame; // starting frame, non-negative when started (in usec for clock time)
     T mLastVolume;       // last computed interpolated volume (y-axis)
     S mLastXOffset;      // last computed interpolated xOffset/time (x-axis)
-    S mDelayXOffset;     // delay xOffset on first volumeshaper start.
+    S mDelayXOffset;     // xOffset to use for first invocation of VolumeShaper.
+
+    // Called internally to adjust mXTranslate for first time start.
+    void updatePosition(int64_t startFrame, double sampleRate, S xOffset) {
+        double scale = (mConfiguration->last().first - mConfiguration->first().first)
+                        / (mConfiguration->getDurationMs() * 0.001 * sampleRate);
+        const double minScale = 1. / static_cast<double>(INT64_MAX);
+        scale = std::max(scale, minScale);
+        VS_LOG("update position: scale %lf  frameCount:%lld, sampleRate:%lf, xOffset:%f",
+                scale, (long long) startFrame, sampleRate, xOffset);
+
+        S normalizedTime = (getFlags() & VolumeShaper::Operation::FLAG_REVERSE) != 0 ?
+                MAX_CURVE_TIME - xOffset : xOffset;
+        mXTranslate.setOffset(static_cast<float>(static_cast<double>(startFrame)
+                                                 - static_cast<double>(normalizedTime) / scale));
+        mXTranslate.setScale(static_cast<float>(scale));
+        VS_LOG("translate: %s", mXTranslate.toString().c_str());
+    }
+
+    T computeVolumeFromXOffset(S xOffset) const {
+        const T unscaledVolume = mConfiguration->findY(xOffset);
+        const T volume = mConfiguration->adjustVolume(unscaledVolume); // handle log scale
+        VS_LOG("computeVolumeFromXOffset %f -> %f -> %f", xOffset, unscaledVolume, volume);
+        return volume;
+    }
+
+    std::tuple<T /* volume */, S /* position */, bool /* active */>
+    computeStateFromNormalizedTime(S x) const {
+        bool active = true;
+        // handle reversal of position
+        if (getFlags() & VolumeShaper::Operation::FLAG_REVERSE) {
+            x = MAX_CURVE_TIME - x;
+            VS_LOG("reversing to %f", x);
+            if (x < MIN_CURVE_TIME) {
+                x = MIN_CURVE_TIME;
+                active = false; // at the end
+            } else if (x > MAX_CURVE_TIME) {
+                x = MAX_CURVE_TIME; //early
+            }
+        } else {
+            if (x < MIN_CURVE_TIME) {
+                x = MIN_CURVE_TIME; // early
+            } else if (x > MAX_CURVE_TIME) {
+                x = MAX_CURVE_TIME;
+                active = false; // at end
+            }
+        }
+        const S xOffset = x;
+        const T volume = computeVolumeFromXOffset(xOffset);
+        return std::make_tuple(volume, xOffset, active);
+    }
 }; // VolumeShaper
 
-// VolumeHandler combines the volume factors of multiple VolumeShapers and handles
-// multiple thread access by synchronizing all public methods.
+/* VolumeHandler combines the volume factors of multiple VolumeShapers associated
+ * with a player.  It is thread safe by synchronizing all public methods.
+ *
+ * This is a native-only implementation.
+ *
+ * The server side VolumeHandler is used to maintain a list of volume handlers,
+ * keep state, and obtain volume.
+ *
+ * The client side VolumeHandler is used to maintain a list of volume handlers,
+ * keep some partial state, and restore if the server dies.
+ */
 class VolumeHandler : public RefBase {
 public:
     using S = float;
@@ -640,13 +756,15 @@
     explicit VolumeHandler(uint32_t sampleRate)
         : mSampleRate((double)sampleRate)
         , mLastFrame(0)
-        , mVolumeShaperIdCounter(VolumeShaper::kSystemIdMax)
+        , mVolumeShaperIdCounter(VolumeShaper::kSystemVolumeShapersMax)
         , mLastVolume(1.f, false) {
     }
 
     VolumeShaper::Status applyVolumeShaper(
             const sp<VolumeShaper::Configuration> &configuration,
-            const sp<VolumeShaper::Operation> &operation) {
+            const sp<VolumeShaper::Operation> &operation_in) {
+        // make a local copy of operation, as we modify it.
+        sp<VolumeShaper::Operation> operation(new VolumeShaper::Operation(operation_in));
         VS_LOG("applyVolumeShaper:configuration: %s", configuration->toString().c_str());
         VS_LOG("applyVolumeShaper:operation: %s", operation->toString().c_str());
         AutoMutex _l(mLock);
@@ -669,14 +787,16 @@
         case VolumeShaper::Configuration::TYPE_SCALE: {
             const int replaceId = operation->getReplaceId();
             if (replaceId >= 0) {
+                VS_LOG("replacing %d", replaceId);
                 auto replaceIt = findId_l(replaceId);
                 if (replaceIt == mVolumeShapers.end()) {
                     ALOGW("cannot find replace id: %d", replaceId);
                 } else {
-                    if ((replaceIt->getFlags() & VolumeShaper::Operation::FLAG_JOIN) != 0) {
+                    if ((operation->getFlags() & VolumeShaper::Operation::FLAG_JOIN) != 0) {
                         // For join, we scale the start volume of the current configuration
                         // to match the last-used volume of the replacing VolumeShaper.
                         auto state = replaceIt->getState();
+                        ALOGD("join: state:%s", state->toString().c_str());
                         if (state->getXOffset() >= 0) { // valid
                             const T volume = state->getVolume();
                             ALOGD("join: scaling start volume to %f", volume);
@@ -698,8 +818,22 @@
                 ALOGW("duplicate id, removing old %d", id);
                 (void)mVolumeShapers.erase(oldIt);
             }
-            // create new VolumeShaper
-            mVolumeShapers.emplace_back(configuration, operation);
+
+            /* Check if too many application VolumeShapers (with id >= kSystemVolumeShapersMax).
+             * We check on the server side to ensure synchronization and robustness.
+             *
+             * This shouldn't fail on a replace command unless the replaced id is
+             * already invalid (which *should* be checked in the Java layer).
+             */
+            if (id >= VolumeShaper::kSystemVolumeShapersMax
+                    && numberOfUserVolumeShapers_l() >= VolumeShaper::kUserVolumeShapersMax) {
+                ALOGW("Too many app VolumeShapers, cannot add to VolumeHandler");
+                return VolumeShaper::Status(INVALID_OPERATION);
+            }
+
+            // create new VolumeShaper with default behavior.
+            mVolumeShapers.emplace_back(configuration, new VolumeShaper::Operation());
+            VS_LOG("after adding, number of volumeShapers:%zu", mVolumeShapers.size());
         }
         // fall through to handle the operation
         HANDLE_TYPE_ID:
@@ -710,7 +844,7 @@
                 VS_LOG("couldn't find id: %d", id);
                 return VolumeShaper::Status(INVALID_OPERATION);
             }
-            if ((it->getFlags() & VolumeShaper::Operation::FLAG_TERMINATE) != 0) {
+            if ((operation->getFlags() & VolumeShaper::Operation::FLAG_TERMINATE) != 0) {
                 VS_LOG("terminate id: %d", id);
                 mVolumeShapers.erase(it);
                 break;
@@ -719,29 +853,37 @@
                     & VolumeShaper::Configuration::OPTION_FLAG_CLOCK_TIME) != 0;
             if ((it->getFlags() & VolumeShaper::Operation::FLAG_REVERSE) !=
                     (operation->getFlags() & VolumeShaper::Operation::FLAG_REVERSE)) {
-                const int64_t frameCount = clockTime ? VolumeShaper::getNowUs() : mLastFrame;
-                const S x = it->mXTranslate((T)frameCount);
-                VS_LOG("reverse translation: %f", x);
-                // reflect position
-                S target = 1.f - x;
-                if (target < it->mConfiguration->first().first) {
-                    VS_LOG("clamp to start - begin immediately");
-                    target = 0.;
+                if (it->isStarted()) {
+                    const int64_t frameCount = clockTime ? VolumeShaper::getNowUs() : mLastFrame;
+                    const S x = it->mXTranslate((T)frameCount);
+                    VS_LOG("reverse normalizedTime: %f", x);
+                    // reflect position
+                    S target = MAX_CURVE_TIME - x;
+                    if (target < MIN_CURVE_TIME) {
+                        VS_LOG("clamp to start - begin immediately");
+                        target = MIN_CURVE_TIME;
+                    }
+                    VS_LOG("reverse normalizedTime target: %f", target);
+                    it->mXTranslate.setOffset(it->mXTranslate.getOffset()
+                            + (x - target) / it->mXTranslate.getScale());
                 }
-                VS_LOG("target reverse: %f", target);
-                it->mXTranslate.setOffset(it->mXTranslate.getOffset()
-                        + (x - target) / it->mXTranslate.getScale());
+                // if not started, the delay offset doesn't change.
             }
             const S xOffset = operation->getXOffset();
             if (!std::isnan(xOffset)) {
-                const int64_t frameCount = clockTime ? VolumeShaper::getNowUs() : mLastFrame;
-                const S x = it->mXTranslate((T)frameCount);
-                VS_LOG("xOffset translation: %f", x);
-                const S target = xOffset; // offset
-                VS_LOG("xOffset target x offset: %f", target);
-                it->mXTranslate.setOffset(it->mXTranslate.getOffset()
-                        + (x - target) / it->mXTranslate.getScale());
-                it->setDelayXOffset(xOffset);
+                if (it->isStarted()) {
+                    const int64_t frameCount = clockTime ? VolumeShaper::getNowUs() : mLastFrame;
+                    const S x = it->mXTranslate((T)frameCount);
+                    VS_LOG("normalizedTime translation: %f", x);
+                    const S target =
+                            (operation->getFlags() & VolumeShaper::Operation::FLAG_REVERSE) != 0 ?
+                                    MAX_CURVE_TIME - xOffset : xOffset;
+                    VS_LOG("normalizedTime target x offset: %f", target);
+                    it->mXTranslate.setOffset(it->mXTranslate.getOffset()
+                            + (x - target) / it->mXTranslate.getScale());
+                } else {
+                    it->setDelayXOffset(xOffset);
+                }
             }
             it->mOperation = operation; // replace the operation
         } break;
@@ -759,28 +901,31 @@
         return it->getState();
     }
 
-    // getVolume() is not const, as it updates internal state.
-    // Once called, any VolumeShapers not already started begin running.
+    /* getVolume() is not const, as it updates internal state.
+     * Once called, any VolumeShapers not already started begin running.
+     */
     std::pair<T /* volume */, bool /* active */> getVolume(int64_t trackFrameCount) {
         AutoMutex _l(mLock);
         mLastFrame = trackFrameCount;
         T volume(1);
         size_t activeCount = 0;
         for (auto it = mVolumeShapers.begin(); it != mVolumeShapers.end();) {
-            std::pair<T, bool> shaperVolume =
+            const std::pair<T, bool> shaperVolume =
                     it->getVolume(trackFrameCount, mSampleRate);
             volume *= shaperVolume.first;
             activeCount += shaperVolume.second;
             ++it;
         }
         mLastVolume = std::make_pair(volume, activeCount != 0);
+        VS_LOG("getVolume: <%f, %s>", mLastVolume.first, mLastVolume.second ? "true" : "false");
         return mLastVolume;
     }
 
-    // Used by a client side VolumeHandler to ensure all the VolumeShapers
-    // indicate that they have been started.  Upon a change in audioserver
-    // output sink, this information is used for restoration of the server side
-    // VolumeHandler.
+    /* Used by a client side VolumeHandler to ensure all the VolumeShapers
+     * indicate that they have been started.  Upon a change in audioserver
+     * output sink, this information is used for restoration of the server side
+     * VolumeHandler.
+     */
     void setStarted() {
         (void)getVolume(mLastFrame);  // getVolume() will start the individual VolumeShapers.
     }
@@ -793,11 +938,19 @@
     std::string toString() const {
         AutoMutex _l(mLock);
         std::stringstream ss;
-        ss << "mSampleRate: " << mSampleRate << std::endl;
-        ss << "mLastFrame: " << mLastFrame << std::endl;
+        ss << "VolumeHandler{mSampleRate=" << mSampleRate;
+        ss << ", mLastFrame=" << mLastFrame;
+        ss << ", mVolumeShapers={";
+        bool first = true;
         for (const auto &shaper : mVolumeShapers) {
+            if (first) {
+                first = false;
+            } else {
+                ss << ", ";
+            }
             ss << shaper.toString().c_str();
         }
+        ss << "}}";
         return ss.str();
     }
 
@@ -817,8 +970,9 @@
         // keep mVolumeShaperIdCounter as is.
     }
 
-    // Sets the configuration id if necessary - This is based on the counter
-    // internal to the VolumeHandler.
+    /* Sets the configuration id if necessary - This is based on the counter
+     * internal to the VolumeHandler.
+     */
     void setIdIfNecessary(const sp<VolumeShaper::Configuration> &configuration) {
         if (configuration->getType() == VolumeShaper::Configuration::TYPE_SCALE) {
             const int id = configuration->getId();
@@ -827,7 +981,7 @@
                 AutoMutex _l(mLock);
                 while (true) {
                     if (mVolumeShaperIdCounter == INT32_MAX) {
-                        mVolumeShaperIdCounter = VolumeShaper::kSystemIdMax;
+                        mVolumeShaperIdCounter = VolumeShaper::kSystemVolumeShapersMax;
                     } else {
                         ++mVolumeShaperIdCounter;
                     }
@@ -853,6 +1007,14 @@
         return it;
     }
 
+    size_t numberOfUserVolumeShapers_l() const {
+        size_t count = 0;
+        for (const auto &shaper : mVolumeShapers) {
+            count += (shaper.mConfiguration->getId() >= VolumeShaper::kSystemVolumeShapersMax);
+        }
+        return count;
+    }
+
     mutable Mutex mLock;
     double mSampleRate; // in samples (frames) per second
     int64_t mLastFrame; // logging purpose only, 0 on start