Merge "VolumeShaper: Enable xOffset from Java" into oc-dev
diff --git a/media/java/android/media/VolumeAutomation.java b/media/java/android/media/VolumeAutomation.java
index dff8801..ff2e645 100644
--- a/media/java/android/media/VolumeAutomation.java
+++ b/media/java/android/media/VolumeAutomation.java
@@ -31,9 +31,9 @@
      * @param configuration the {@link VolumeShaper.Configuration configuration}
      *        that specifies the curve and duration to use.
      * @return a {@code VolumeShaper} object
-     * @throws IllegalArgumentException if the configuration is not allowed by the player.
-     * @throws IllegalStateException if too many VolumeShapers are requested or the state of
-     * the player does not permit its creation (e.g. player is released).
+     * @throws IllegalArgumentException if the {@code configuration} is not allowed by the player.
+     * @throws IllegalStateException if too many {@code VolumeShaper}s are requested
+     *         or the state of the player does not permit its creation (e.g. player is released).
      */
     public @NonNull VolumeShaper createVolumeShaper(
             @NonNull VolumeShaper.Configuration configuration);
diff --git a/media/java/android/media/VolumeShaper.java b/media/java/android/media/VolumeShaper.java
index 8f4721f..3068706 100644
--- a/media/java/android/media/VolumeShaper.java
+++ b/media/java/android/media/VolumeShaper.java
@@ -31,8 +31,16 @@
 /**
  * The {@code VolumeShaper} class is used to automatically control audio volume during media
  * playback, allowing simple implementation of transition effects and ducking.
+ * It is created from implementations of {@code VolumeAutomation},
+ * such as {@code MediaPlayer} and {@code AudioTrack} (referred to as "players" below),
+ * by {@link MediaPlayer#createVolumeShaper} or {@link AudioTrack#createVolumeShaper}.
  *
- * The {@link VolumeShaper} appears as an additional scaling on the audio output,
+ * A {@code VolumeShaper} is intended for short volume changes.
+ * If the audio output sink changes during
+ * a {@code VolumeShaper} transition, the precise curve position may be lost, and the
+ * {@code VolumeShaper} may advance to the end of the curve for the new audio output sink.
+ *
+ * The {@code VolumeShaper} appears as an additional scaling on the audio output,
  * and adjusts independently of track or stream volume controls.
  */
 public final class VolumeShaper implements AutoCloseable {
@@ -52,7 +60,19 @@
 
     /**
      * Applies the {@link VolumeShaper.Operation} to the {@code VolumeShaper}.
+     *
+     * Applying {@link VolumeShaper.Operation#PLAY} after {@code PLAY}
+     * or {@link VolumeShaper.Operation#REVERSE} after
+     * {@code REVERSE} has no effect.
+     *
+     * Applying {@link VolumeShaper.Operation#PLAY} when the player
+     * hasn't started will synchronously start the {@code VolumeShaper} when
+     * playback begins.
+     *
      * @param operation the {@code operation} to apply.
+     * @throws IllegalStateException if the player is uninitialized or if there
+     *         is a critical failure. In that case, the {@code VolumeShaper} should be
+     *         recreated.
      */
     public void apply(@NonNull Operation operation) {
         /* void */ applyPlayer(new VolumeShaper.Configuration(mId), operation);
@@ -65,11 +85,24 @@
      * This allows the user to change the volume shape
      * while the existing {@code VolumeShaper} is in effect.
      *
+     * The effect of {@code replace()} is similar to an atomic close of
+     * the existing {@code VolumeShaper} and creation of a new {@code VolumeShaper}.
+     *
+     * If the {@code operation} is {@link VolumeShaper.Operation#PLAY} then the
+     * new curve starts immediately.
+     *
+     * If the {@code operation} is
+     * {@link VolumeShaper.Operation#REVERSE}, then the new curve will
+     * be delayed until {@code PLAY} is applied.
+     *
      * @param configuration the new {@code configuration} to use.
-     * @param operation the operation to apply to the {@code VolumeShaper}
+     * @param operation the {@code operation} to apply to the {@code VolumeShaper}
      * @param join if true, match the start volume of the
      *             new {@code configuration} to the current volume of the existing
      *             {@code VolumeShaper}, to avoid discontinuity.
+     * @throws IllegalStateException if the player is uninitialized or if there
+     *         is a critical failure. In that case, the {@code VolumeShaper} should be
+     *         recreated.
      */
     public void replace(
             @NonNull Configuration configuration, @NonNull Operation operation, boolean join) {
@@ -81,7 +114,14 @@
     /**
      * Returns the current volume scale attributable to the {@code VolumeShaper}.
      *
+     * This is the last volume from the {@code VolumeShaper} used for the player,
+     * or the initial volume if the {@code VolumeShaper} hasn't been started with
+     * {@link VolumeShaper.Operation#PLAY}.
+     *
      * @return the volume, linearly represented as a value between 0.f and 1.f.
+     * @throws IllegalStateException if the player is uninitialized or if there
+     *         is a critical failure.  In that case, the {@code VolumeShaper} should be
+     *         recreated.
      */
     public float getVolume() {
         return getStatePlayer(mId).getVolume();
@@ -89,7 +129,14 @@
 
     /**
      * Releases the {@code VolumeShaper} object; any volume scale due to the
-     * {@code VolumeShaper} is removed.
+     * {@code VolumeShaper} is removed after closing.
+     *
+     * If the volume does not reach 1.f when the {@code VolumeShaper} is closed
+     * (or finalized), there may be an abrupt change of volume.
+     *
+     * {@code close()} may be safely called after a prior {@code close()}.
+     * This class implements the Java {@code AutoClosable} interface and
+     * may be used with try-with-resources.
      */
     @Override
     public void close() {
@@ -107,11 +154,11 @@
 
     @Override
     protected void finalize() {
-        close(); // ensure we remove the native volume shaper
+        close(); // ensure we remove the native VolumeShaper
     }
 
     /**
-     * Internal call to apply the configuration and operation to the Player.
+     * Internal call to apply the {@code configuration} and {@code operation} to the player.
      * Returns a valid shaper id or throws the appropriate exception.
      * @param configuration
      * @param operation
@@ -137,7 +184,7 @@
             // Due to RPC handling, we translate integer codes to exceptions right before
             // delivering to the user.
             if (id == VOLUME_SHAPER_INVALID_OPERATION) {
-                throw new IllegalStateException("player or volume shaper deallocated");
+                throw new IllegalStateException("player or VolumeShaper deallocated");
             } else {
                 throw new IllegalArgumentException("invalid configuration or operation: " + id);
             }
@@ -146,9 +193,9 @@
     }
 
     /**
-     * Internal call to retrieve the current VolumeShaper state.
+     * Internal call to retrieve the current {@code VolumeShaper} state.
      * @param id
-     * @return the current {@vode VolumeShaper.State}
+     * @return the current {@code VolumeShaper.State}
      * @throws IllegalStateException if the player has been deallocated or is uninitialized.
      */
     private @NonNull VolumeShaper.State getStatePlayer(int id) {
@@ -180,6 +227,9 @@
      * by {@link VolumeShaper#replace(Configuration, Operation, boolean)
      * VolumeShaper.replace(Configuration, Operation, boolean)}
      * to replace an existing {@code configuration}.
+     * <p>
+     * The {@link AudioTrack} and {@link MediaPlayer} classes implement
+     * the {@link VolumeAutomation} interface.
      */
     public static final class Configuration implements Parcelable {
         private static final int MAXIMUM_CURVE_POINTS = 16;
@@ -485,7 +535,7 @@
 
         /**
          * @hide
-         * Constructs a volume shaper from an id.
+         * Constructs a {@code VolumeShaper} from an id.
          *
          * This is an opaque handle for controlling a {@code VolumeShaper} that has
          * already been sent to a player.  The {@code id} is returned from the
@@ -756,7 +806,7 @@
             /**
              * Sets the interpolator type.
              *
-             * If omitted the interplator type is {@link #INTERPOLATOR_TYPE_CUBIC}.
+             * If omitted the default interpolator type is {@link #INTERPOLATOR_TYPE_CUBIC}.
              *
              * @param interpolatorType method of interpolation used for the volume curve.
              *        One of {@link #INTERPOLATOR_TYPE_STEP},
@@ -802,7 +852,7 @@
             }
 
             /**
-             * Sets the volume shaper duration in milliseconds.
+             * Sets the {@code VolumeShaper} duration in milliseconds.
              *
              * If omitted, the default duration is 1 second.
              *
@@ -1059,13 +1109,13 @@
 
         /**
          * Defer playback until next operation is sent. This is used
-         * when starting a VolumeShaper effect.
+         * when starting a {@code VolumeShaper} effect.
          */
         private static final int FLAG_DEFER = 1 << 3;
 
         /**
          * Use the id specified in the configuration, creating
-         * VolumeShaper as needed; the configuration should be
+         * {@code VolumeShaper} as needed; the configuration should be
          * TYPE_SCALE.
          */
         private static final int FLAG_CREATE_IF_NEEDED = 1 << 4;
@@ -1074,18 +1124,20 @@
 
         private final int mFlags;
         private final int mReplaceId;
+        private final float mXOffset;
 
         @Override
         public String toString() {
             return "VolumeShaper.Operation{"
                     + "mFlags = 0x" + Integer.toHexString(mFlags).toUpperCase()
                     + ", mReplaceId = " + mReplaceId
+                    + ", mXOffset = " + mXOffset
                     + "}";
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(mFlags, mReplaceId);
+            return Objects.hash(mFlags, mReplaceId, mXOffset);
         }
 
         @Override
@@ -1093,10 +1145,10 @@
             if (!(o instanceof Operation)) return false;
             if (o == this) return true;
             final Operation other = (Operation) o;
-            // if xOffset (native field only) is brought into Java
-            // we need to do proper NaN comparison as that is allowed.
+
             return mFlags == other.mFlags
-                    && mReplaceId == other.mReplaceId;
+                    && mReplaceId == other.mReplaceId
+                    && Float.compare(mXOffset, other.mXOffset) == 0;
         }
 
         @Override
@@ -1109,7 +1161,7 @@
             // this needs to match the native VolumeShaper.Operation parceling
             dest.writeInt(mFlags);
             dest.writeInt(mReplaceId);
-            dest.writeFloat(Float.NaN); // xOffset (ignored at Java level)
+            dest.writeFloat(mXOffset);
         }
 
         public static final Parcelable.Creator<VolumeShaper.Operation> CREATOR
@@ -1119,11 +1171,12 @@
                 // this needs to match the native VolumeShaper.Operation parceling
                 final int flags = p.readInt();
                 final int replaceId = p.readInt();
-                final float xOffset = p.readFloat(); // ignored at Java level
+                final float xOffset = p.readFloat();
 
                 return new VolumeShaper.Operation(
                         flags
-                        , replaceId);
+                        , replaceId
+                        , xOffset);
             }
 
             @Override
@@ -1132,9 +1185,10 @@
             }
         };
 
-        private Operation(@Flag int flags, int replaceId) {
+        private Operation(@Flag int flags, int replaceId, float xOffset) {
             mFlags = flags;
             mReplaceId = replaceId;
+            mXOffset = xOffset;
         }
 
         /**
@@ -1146,6 +1200,7 @@
         public static final class Builder {
             int mFlags;
             int mReplaceId;
+            float mXOffset;
 
             /**
              * Constructs a new {@code Builder} with the defaults.
@@ -1153,23 +1208,27 @@
             public Builder() {
                 mFlags = 0;
                 mReplaceId = -1;
+                mXOffset = Float.NaN;
             }
 
             /**
-             * Constructs a new Builder from a given {@code VolumeShaper.Operation}
+             * Constructs a new {@code Builder} from a given {@code VolumeShaper.Operation}
              * @param operation the {@code VolumeShaper.operation} whose data will be
-             *        reused in the new Builder.
+             *        reused in the new {@code Builder}.
              */
             public Builder(@NonNull VolumeShaper.Operation operation) {
                 mReplaceId = operation.mReplaceId;
                 mFlags = operation.mFlags;
+                mXOffset = operation.mXOffset;
             }
 
             /**
-             * Replaces the previous {@code VolumeShaper} specified by id.
-             * It has no other effect if the {@code VolumeShaper} is
-             * already expired.
-             * @param id the id of the previous {@code VolumeShaper}.
+             * Replaces the previous {@code VolumeShaper} specified by {@code id}.
+             *
+             * The {@code VolumeShaper} specified by the {@code id} is removed
+             * if it exists. The configuration should be TYPE_SCALE.
+             *
+             * @param id the {@code id} of the previous {@code VolumeShaper}.
              * @param join if true, match the volume of the previous
              * shaper to the start volume of the new {@code VolumeShaper}.
              * @return the same {@code Builder} instance.
@@ -1194,8 +1253,9 @@
             }
 
             /**
-             * Terminates the VolumeShaper.
-             * Do not call directly, use {@link VolumeShaper#release()}.
+             * Terminates the {@code VolumeShaper}.
+             *
+             * Do not call directly, use {@link VolumeShaper#close()}.
              * @return the same {@code Builder} instance.
              */
             public @NonNull Builder terminate() {
@@ -1214,8 +1274,12 @@
 
             /**
              * Use the id specified in the configuration, creating
-             * VolumeShaper as needed; the configuration should be
+             * {@code VolumeShaper} only as needed; the configuration should be
              * TYPE_SCALE.
+             *
+             * If the {@code VolumeShaper} with the same id already exists
+             * then the operation has no effect.
+             *
              * @return the same {@code Builder} instance.
              */
             public @NonNull Builder createIfNeeded() {
@@ -1224,6 +1288,28 @@
             }
 
             /**
+             * Sets the {@code xOffset} to use for the {@code VolumeShaper}.
+             *
+             * The {@code xOffset} is the position on the volume curve,
+             * and setting takes effect when the {@code VolumeShaper} is used next.
+             *
+             * @param xOffset a value between (or equal to) 0.f and 1.f, or Float.NaN to ignore.
+             * @return the same {@code Builder} instance.
+             * @throws IllegalArgumentException if {@code xOffset} is not between 0.f and 1.f,
+             *         or a Float.NaN.
+             */
+            public @NonNull Builder setXOffset(float xOffset) {
+                if (xOffset < -0.f) {
+                    throw new IllegalArgumentException("Negative xOffset not allowed");
+                } else if (xOffset > 1.f) {
+                    throw new IllegalArgumentException("xOffset > 1.f not allowed");
+                }
+                // Float.NaN passes through
+                mXOffset = xOffset;
+                return this;
+            }
+
+            /**
              * Sets the operation flag.  Do not call this directly but one of the
              * other builder methods.
              *
@@ -1245,7 +1331,7 @@
              * @return a new {@code VolumeShaper.Operation} object
              */
             public @NonNull Operation build() {
-                return new Operation(mFlags, mReplaceId);
+                return new Operation(mFlags, mReplaceId, mXOffset);
             }
         } // Operation.Builder
     } // Operation
@@ -1316,15 +1402,18 @@
 
         /**
          * Gets the volume of the {@link VolumeShaper.State}.
+         * @return linear volume between 0.f and 1.f.
          */
         public float getVolume() {
             return mVolume;
         }
 
         /**
-         * Gets the elapsed ms of the {@link VolumeShaper.State}
+         * Gets the {@code xOffset} position on the normalized curve
+         * of the {@link VolumeShaper.State}.
+         * @return the curve x position between 0.f and 1.f.
          */
-        public double getXOffset() {
+        public float getXOffset() {
             return mXOffset;
         }
     } // State
diff --git a/media/jni/android_media_VolumeShaper.h b/media/jni/android_media_VolumeShaper.h
index 73498a2..1a13ffa 100644
--- a/media/jni/android_media_VolumeShaper.h
+++ b/media/jni/android_media_VolumeShaper.h
@@ -40,6 +40,7 @@
         jmethodID opConstructId;
         jfieldID  opFlagsId;
         jfieldID  opReplaceIdId;
+        jfieldID  opXOffsetId;
 
         // VolumeShaper.State
         jclass    stClazz;
@@ -74,9 +75,10 @@
             if (opClazz == nullptr) {
                 return;
             }
-            opConstructId = env->GetMethodID(opClazz, "<init>", "(II)V");
+            opConstructId = env->GetMethodID(opClazz, "<init>", "(IIF)V");
             opFlagsId = env->GetFieldID(opClazz, "mFlags", "I");
             opReplaceIdId = env->GetFieldID(opClazz, "mReplaceId", "I");
+            opXOffsetId = env->GetFieldID(opClazz, "mXOffset", "F");
             env->DeleteLocalRef(lclazz);
 
             lclazz = env->FindClass("android/media/VolumeShaper$State");
@@ -179,17 +181,20 @@
         VolumeShaper::Operation::Flag flags =
             (VolumeShaper::Operation::Flag)env->GetIntField(joperation, fields.opFlagsId);
         int replaceId = env->GetIntField(joperation, fields.opReplaceIdId);
+        float xOffset = env->GetFloatField(joperation, fields.opXOffsetId);
 
-        sp<VolumeShaper::Operation> operation = new VolumeShaper::Operation(flags, replaceId);
+        sp<VolumeShaper::Operation> operation =
+                new VolumeShaper::Operation(flags, replaceId, xOffset);
         return operation;
     }
 
     static jobject convertOperationToJobject(
             JNIEnv *env, const fields_t &fields, const sp<VolumeShaper::Operation> &operation) {
         // prepare constructor args
-        jvalue args[2];
+        jvalue args[3];
         args[0].i = (jint)operation->getFlags();
         args[1].i = (jint)operation->getReplaceId();
+        args[2].f = (jfloat)operation->getXOffset();
 
         jobject joperation = env->NewObjectA(fields.opClazz, fields.opConstructId, args);
         return joperation;