Merge "VolumeShaper: Fixes for updated Cts test" into oc-dev
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
diff --git a/media/libaudioclient/AudioTrack.cpp b/media/libaudioclient/AudioTrack.cpp
index 4baf253..38d90bc 100644
--- a/media/libaudioclient/AudioTrack.cpp
+++ b/media/libaudioclient/AudioTrack.cpp
@@ -2268,7 +2268,7 @@
             // For now, we simply advance to the end of the VolumeShaper effect
             // if it has been started.
             if (shaper.isStarted()) {
-                operationToEnd->setXOffset(1.f);
+                operationToEnd->setNormalizedTime(1.f);
             }
             return mAudioTrack->applyVolumeShaper(shaper.mConfiguration, operationToEnd);
         });
diff --git a/media/libaudioclient/include/media/AudioTrack.h b/media/libaudioclient/include/media/AudioTrack.h
index 16eb225..a4c8d53 100644
--- a/media/libaudioclient/include/media/AudioTrack.h
+++ b/media/libaudioclient/include/media/AudioTrack.h
@@ -874,6 +874,10 @@
      */
             bool hasStarted(); // not const
 
+            bool isPlaying() {
+                AutoMutex lock(mLock);
+                return mState == STATE_ACTIVE || mState == STATE_STOPPING;
+            }
 protected:
     /* copying audio tracks is not allowed */
                         AudioTrack(const AudioTrack& other);
diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index cba5cf5..7079ff2 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -2042,7 +2042,7 @@
         // For now, we simply advance to the end of the VolumeShaper effect
         // if it has been started.
         if (shaper.isStarted()) {
-            operationToEnd->setXOffset(1.f);
+            operationToEnd->setNormalizedTime(1.f);
         }
         return t->applyVolumeShaper(shaper.mConfiguration, operationToEnd);
     });
@@ -2301,8 +2301,9 @@
         status = mTrack->applyVolumeShaper(configuration, operation);
         if (status >= 0) {
             (void)mVolumeHandler->applyVolumeShaper(configuration, operation);
-            // TODO: start on exact AudioTrack state (STATE_ACTIVE || STATE_STOPPING)
-            mVolumeHandler->setStarted();
+            if (mTrack->isPlaying()) { // match local AudioTrack to properly restore.
+                mVolumeHandler->setStarted();
+            }
         }
     } else {
         status = mVolumeHandler->applyVolumeShaper(configuration, operation);