Correctly mask playback info changes in ExoPlayerImpl.

PlaybackInfo changes are one of the last ones not masked and reported in the same
way as all other changes.

The main change to support this is to also mask the parameters set in
DefaultAudioSink.

PiperOrigin-RevId: 258787744

library/core/src/main/java/com/google/android/exoplayer2/DefaultMediaClock.java

@@ -32,12 +32,12 @@ import com.google.android.exoplayer2.util.StandaloneMediaClock;
   public interface PlaybackParameterListener {
 
     /**
-     * Called when the active playback parameters changed.
+     * Called when the active playback parameters changed. Will not be called for {@link
+     * #setPlaybackParameters(PlaybackParameters)}.
      *
      * @param newPlaybackParameters The newly active {@link PlaybackParameters}.
      */
     void onPlaybackParametersChanged(PlaybackParameters newPlaybackParameters);
-
   }
 
   private final StandaloneMediaClock standaloneMediaClock;
@@ -141,13 +141,12 @@ import com.google.android.exoplayer2.util.StandaloneMediaClock;
   }
 
   @Override
-  public PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters) {
+  public void setPlaybackParameters(PlaybackParameters playbackParameters) {
     if (rendererClock != null) {
-      playbackParameters = rendererClock.setPlaybackParameters(playbackParameters);
+      rendererClock.setPlaybackParameters(playbackParameters);
+      playbackParameters = rendererClock.getPlaybackParameters();
     }
     standaloneMediaClock.setPlaybackParameters(playbackParameters);
-    listener.onPlaybackParametersChanged(playbackParameters);
-    return playbackParameters;
   }
 
   @Override

library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java

@@ -69,6 +69,7 @@ import java.util.concurrent.CopyOnWriteArrayList;
   private boolean hasPendingPrepare;
   private boolean hasPendingSeek;
   private boolean foregroundMode;
+  private int pendingSetPlaybackParametersAcks;
   private PlaybackParameters playbackParameters;
   private SeekParameters seekParameters;
   @Nullable private ExoPlaybackException playbackError;
@@ -336,7 +337,14 @@ import java.util.concurrent.CopyOnWriteArrayList;
     if (playbackParameters == null) {
       playbackParameters = PlaybackParameters.DEFAULT;
     }
+    if (this.playbackParameters.equals(playbackParameters)) {
+      return;
+    }
+    pendingSetPlaybackParametersAcks++;
+    this.playbackParameters = playbackParameters;
     internalPlayer.setPlaybackParameters(playbackParameters);
+    PlaybackParameters playbackParametersToNotify = playbackParameters;
+    notifyListeners(listener -> listener.onPlaybackParametersChanged(playbackParametersToNotify));
   }
 
   @Override
@@ -560,11 +568,7 @@ import java.util.concurrent.CopyOnWriteArrayList;
             /* positionDiscontinuityReason= */ msg.arg2);
         break;
       case ExoPlayerImplInternal.MSG_PLAYBACK_PARAMETERS_CHANGED:
-        PlaybackParameters playbackParameters = (PlaybackParameters) msg.obj;
+        handlePlaybackParameters((PlaybackParameters) msg.obj, /* operationAck= */ msg.arg1 != 0);
-        if (!this.playbackParameters.equals(playbackParameters)) {
-          this.playbackParameters = playbackParameters;
-          notifyListeners(listener -> listener.onPlaybackParametersChanged(playbackParameters));
-        }
         break;
       case ExoPlayerImplInternal.MSG_ERROR:
         ExoPlaybackException playbackError = (ExoPlaybackException) msg.obj;
@@ -576,6 +580,19 @@ import java.util.concurrent.CopyOnWriteArrayList;
     }
   }
 
+  private void handlePlaybackParameters(
+      PlaybackParameters playbackParameters, boolean operationAck) {
+    if (operationAck) {
+      pendingSetPlaybackParametersAcks--;
+    }
+    if (pendingSetPlaybackParametersAcks == 0) {
+      if (!this.playbackParameters.equals(playbackParameters)) {
+        this.playbackParameters = playbackParameters;
+        notifyListeners(listener -> listener.onPlaybackParametersChanged(playbackParameters));
+      }
+    }
+  }
+
   private void handlePlaybackInfo(
       PlaybackInfo playbackInfo,
       int operationAcks,

library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java

@@ -297,9 +297,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 
   @Override
   public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) {
-    handler
+    sendPlaybackParametersChangedInternal(playbackParameters, /* acknowledgeCommand= */ false);
-        .obtainMessage(MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL, playbackParameters)
-        .sendToTarget();
   }
 
   // Handler.Callback implementation.
@@ -358,7 +356,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
           reselectTracksInternal();
           break;
         case MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL:
-          handlePlaybackParameters((PlaybackParameters) msg.obj);
+          handlePlaybackParameters(
+              (PlaybackParameters) msg.obj, /* acknowledgeCommand= */ msg.arg1 != 0);
           break;
         case MSG_SEND_MESSAGE:
           sendMessageInternal((PlayerMessage) msg.obj);
@@ -783,6 +782,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
 
   private void setPlaybackParametersInternal(PlaybackParameters playbackParameters) {
     mediaClock.setPlaybackParameters(playbackParameters);
+    sendPlaybackParametersChangedInternal(
+        mediaClock.getPlaybackParameters(), /* acknowledgeCommand= */ true);
   }
 
   private void setSeekParametersInternal(SeekParameters seekParameters) {
@@ -1663,9 +1664,13 @@ import java.util.concurrent.atomic.AtomicBoolean;
     maybeContinueLoading();
   }
 
-  private void handlePlaybackParameters(PlaybackParameters playbackParameters)
+  private void handlePlaybackParameters(
+      PlaybackParameters playbackParameters, boolean acknowledgeCommand)
       throws ExoPlaybackException {
-    eventHandler.obtainMessage(MSG_PLAYBACK_PARAMETERS_CHANGED, playbackParameters).sendToTarget();
+    eventHandler
+        .obtainMessage(
+            MSG_PLAYBACK_PARAMETERS_CHANGED, acknowledgeCommand ? 1 : 0, 0, playbackParameters)
+        .sendToTarget();
     updateTrackSelectionPlaybackSpeed(playbackParameters.speed);
     for (Renderer renderer : renderers) {
       if (renderer != null) {
@@ -1820,6 +1825,17 @@ import java.util.concurrent.atomic.AtomicBoolean;
     loadControl.onTracksSelected(renderers, trackGroups, trackSelectorResult.selections);
   }
 
+  private void sendPlaybackParametersChangedInternal(
+      PlaybackParameters playbackParameters, boolean acknowledgeCommand) {
+    handler
+        .obtainMessage(
+            MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL,
+            acknowledgeCommand ? 1 : 0,
+            0,
+            playbackParameters)
+        .sendToTarget();
+  }
+
   private static Format[] getFormats(TrackSelection newSelection) {
     // Build an array of formats contained by the selection.
     int length = newSelection != null ? newSelection.length() : 0;

library/core/src/main/java/com/google/android/exoplayer2/Player.java

@@ -761,13 +761,10 @@ public interface Player {
   /**
    * Attempts to set the playback parameters. Passing {@code null} sets the parameters to the
    * default, {@link PlaybackParameters#DEFAULT}, which means there is no speed or pitch adjustment.
-   * <p>
+   *
-   * Playback parameters changes may cause the player to buffer.
+   * <p>Playback parameters changes may cause the player to buffer. {@link
-   * {@link EventListener#onPlaybackParametersChanged(PlaybackParameters)} will be called whenever
+   * EventListener#onPlaybackParametersChanged(PlaybackParameters)} will be called whenever the
-   * the currently active playback parameters change. When that listener is called, the parameters
+   * currently active playback parameters change.
-   * passed to it may not match {@code playbackParameters}. For example, the chosen speed or pitch
-   * may be out of range, in which case they are constrained to a set of permitted values. If it is
-   * not possible to change the playback parameters, the listener will not be invoked.
    *
    * @param playbackParameters The playback parameters, or {@code null} to use the defaults.
    */

library/core/src/main/java/com/google/android/exoplayer2/audio/AudioSink.java

@@ -259,13 +259,12 @@ public interface AudioSink {
   boolean hasPendingData();
 
   /**
-   * Attempts to set the playback parameters and returns the active playback parameters, which may
+   * Attempts to set the playback parameters. The audio sink may override these parameters if they
-   * differ from those passed in.
+   * are not supported.
    *
    * @param playbackParameters The new playback parameters to attempt to set.
-   * @return The active playback parameters.
    */
-  PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters);
+  void setPlaybackParameters(PlaybackParameters playbackParameters);
 
   /**
    * Gets the active {@link PlaybackParameters}.

library/core/src/main/java/com/google/android/exoplayer2/audio/DefaultAudioSink.java

@@ -825,17 +825,12 @@ public final class DefaultAudioSink implements AudioSink {
   }
 
   @Override
-  public PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters) {
+  public void setPlaybackParameters(PlaybackParameters playbackParameters) {
     if (configuration != null && !configuration.canApplyPlaybackParameters) {
       this.playbackParameters = PlaybackParameters.DEFAULT;
-      return this.playbackParameters;
+      return;
     }
-    PlaybackParameters lastSetPlaybackParameters =
+    PlaybackParameters lastSetPlaybackParameters = getPlaybackParameters();
-        afterDrainPlaybackParameters != null
-            ? afterDrainPlaybackParameters
-            : !playbackParametersCheckpoints.isEmpty()
-                ? playbackParametersCheckpoints.getLast().playbackParameters
-                : this.playbackParameters;
     if (!playbackParameters.equals(lastSetPlaybackParameters)) {
       if (isInitialized()) {
         // Drain the audio processors so we can determine the frame position at which the new
@@ -847,12 +842,16 @@ public final class DefaultAudioSink implements AudioSink {
         this.playbackParameters = playbackParameters;
       }
     }
-    return this.playbackParameters;
   }
 
   @Override
   public PlaybackParameters getPlaybackParameters() {
-    return playbackParameters;
+    // Mask the already set parameters.
+    return afterDrainPlaybackParameters != null
+        ? afterDrainPlaybackParameters
+        : !playbackParametersCheckpoints.isEmpty()
+            ? playbackParametersCheckpoints.getLast().playbackParameters
+            : playbackParameters;
   }
 
   @Override

library/core/src/main/java/com/google/android/exoplayer2/audio/MediaCodecAudioRenderer.java

@@ -648,8 +648,8 @@ public class MediaCodecAudioRenderer extends MediaCodecRenderer implements Media
   }
 
   @Override
-  public PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters) {
+  public void setPlaybackParameters(PlaybackParameters playbackParameters) {
-    return audioSink.setPlaybackParameters(playbackParameters);
+    audioSink.setPlaybackParameters(playbackParameters);
   }
 
   @Override

library/core/src/main/java/com/google/android/exoplayer2/audio/SimpleDecoderAudioRenderer.java

@@ -517,8 +517,8 @@ public abstract class SimpleDecoderAudioRenderer extends BaseRenderer implements
   }
 
   @Override
-  public PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters) {
+  public void setPlaybackParameters(PlaybackParameters playbackParameters) {
-    return audioSink.setPlaybackParameters(playbackParameters);
+    audioSink.setPlaybackParameters(playbackParameters);
   }
 
   @Override

library/core/src/main/java/com/google/android/exoplayer2/util/MediaClock.java

@@ -28,13 +28,12 @@ public interface MediaClock {
   long getPositionUs();
 
   /**
-   * Attempts to set the playback parameters and returns the active playback parameters, which may
+   * Attempts to set the playback parameters. The media clock may override these parameters if they
-   * differ from those passed in.
+   * are not supported.
    *
-   * @param playbackParameters The playback parameters.
+   * @param playbackParameters The playback parameters to attempt to set.
-   * @return The active playback parameters.
    */
-  PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters);
+  void setPlaybackParameters(PlaybackParameters playbackParameters);
 
   /**
    * Returns the active playback parameters.

library/core/src/main/java/com/google/android/exoplayer2/util/StandaloneMediaClock.java

@@ -88,13 +88,12 @@ public final class StandaloneMediaClock implements MediaClock {
   }
 
   @Override
-  public PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters) {
+  public void setPlaybackParameters(PlaybackParameters playbackParameters) {
     // Store the current position as the new base, in case the playback speed has changed.
     if (started) {
       resetPosition(getPositionUs());
     }
     this.playbackParameters = playbackParameters;
-    return playbackParameters;
   }
 
   @Override

library/core/src/test/java/com/google/android/exoplayer2/DefaultMediaClockTest.java

@@ -17,7 +17,6 @@ package com.google.android.exoplayer2;
 
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.MockitoAnnotations.initMocks;
@@ -116,15 +115,14 @@ public class DefaultMediaClockTest {
 
   @Test
   public void standaloneSetPlaybackParameters_getPlaybackParametersShouldReturnSameValue() {
-    PlaybackParameters parameters = mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
+    mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    assertThat(parameters).isEqualTo(TEST_PLAYBACK_PARAMETERS);
     assertThat(mediaClock.getPlaybackParameters()).isEqualTo(TEST_PLAYBACK_PARAMETERS);
   }
 
   @Test
-  public void standaloneSetPlaybackParameters_shouldTriggerCallback() {
+  public void standaloneSetPlaybackParameters_shouldNotTriggerCallback() {
     mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    verify(listener).onPlaybackParametersChanged(TEST_PLAYBACK_PARAMETERS);
+    verifyNoMoreInteractions(listener);
   }
 
   @Test
@@ -137,24 +135,9 @@ public class DefaultMediaClockTest {
 
   @Test
   public void standaloneSetOtherPlaybackParameters_getPlaybackParametersShouldReturnSameValue() {
-    mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    PlaybackParameters parameters = mediaClock.setPlaybackParameters(PlaybackParameters.DEFAULT);
-    assertThat(parameters).isEqualTo(PlaybackParameters.DEFAULT);
-    assertThat(mediaClock.getPlaybackParameters()).isEqualTo(PlaybackParameters.DEFAULT);
-  }
-
-  @Test
-  public void standaloneSetOtherPlaybackParameters_shouldTriggerCallbackAgain() {
     mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
     mediaClock.setPlaybackParameters(PlaybackParameters.DEFAULT);
-    verify(listener).onPlaybackParametersChanged(PlaybackParameters.DEFAULT);
+    assertThat(mediaClock.getPlaybackParameters()).isEqualTo(PlaybackParameters.DEFAULT);
-  }
-
-  @Test
-  public void standaloneSetSamePlaybackParametersAgain_shouldTriggerCallbackAgain() {
-    mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    verify(listener, times(2)).onPlaybackParametersChanged(TEST_PLAYBACK_PARAMETERS);
   }
 
   @Test
@@ -210,19 +193,18 @@ public class DefaultMediaClockTest {
     FakeMediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
         /* playbackParametersAreMutable= */ true);
     mediaClock.onRendererEnabled(mediaClockRenderer);
-    PlaybackParameters parameters = mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
+    mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    assertThat(parameters).isEqualTo(TEST_PLAYBACK_PARAMETERS);
     assertThat(mediaClock.getPlaybackParameters()).isEqualTo(TEST_PLAYBACK_PARAMETERS);
   }
 
   @Test
-  public void rendererClockSetPlaybackParameters_shouldTriggerCallback()
+  public void rendererClockSetPlaybackParameters_shouldNotTriggerCallback()
       throws ExoPlaybackException {
     FakeMediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
         /* playbackParametersAreMutable= */ true);
     mediaClock.onRendererEnabled(mediaClockRenderer);
     mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    verify(listener).onPlaybackParametersChanged(TEST_PLAYBACK_PARAMETERS);
+    verifyNoMoreInteractions(listener);
   }
 
   @Test
@@ -231,19 +213,8 @@ public class DefaultMediaClockTest {
     FakeMediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
         /* playbackParametersAreMutable= */ false);
     mediaClock.onRendererEnabled(mediaClockRenderer);
-    PlaybackParameters parameters = mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    assertThat(parameters).isEqualTo(PlaybackParameters.DEFAULT);
-    assertThat(mediaClock.getPlaybackParameters()).isEqualTo(PlaybackParameters.DEFAULT);
-  }
-
-  @Test
-  public void rendererClockSetPlaybackParametersOverwrite_shouldTriggerCallback()
-      throws ExoPlaybackException {
-    FakeMediaClockRenderer mediaClockRenderer = new MediaClockRenderer(PlaybackParameters.DEFAULT,
-        /* playbackParametersAreMutable= */ false);
-    mediaClock.onRendererEnabled(mediaClockRenderer);
     mediaClock.setPlaybackParameters(TEST_PLAYBACK_PARAMETERS);
-    verify(listener).onPlaybackParametersChanged(PlaybackParameters.DEFAULT);
+    assertThat(mediaClock.getPlaybackParameters()).isEqualTo(PlaybackParameters.DEFAULT);
   }
 
   @Test
@@ -418,11 +389,10 @@ public class DefaultMediaClockTest {
     }
 
     @Override
-    public PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters) {
+    public void setPlaybackParameters(PlaybackParameters playbackParameters) {
       if (playbackParametersAreMutable) {
         this.playbackParameters = playbackParameters;
       }
-      return this.playbackParameters;
     }
 
     @Override

library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java

@@ -223,9 +223,7 @@ public final class ExoPlayerTest {
           }
 
           @Override
-          public PlaybackParameters setPlaybackParameters(PlaybackParameters playbackParameters) {
+          public void setPlaybackParameters(PlaybackParameters playbackParameters) {}
-            return PlaybackParameters.DEFAULT;
-          }
 
           @Override
           public PlaybackParameters getPlaybackParameters() {
@@ -2641,6 +2639,95 @@ public final class ExoPlayerTest {
     assertThat(contentStartPositionMs.get()).isAtLeast(5_000L);
   }
 
+  @Test
+  public void setPlaybackParametersConsecutivelyNotifiesListenerForEveryChangeOnce()
+      throws Exception {
+    ActionSchedule actionSchedule =
+        new ActionSchedule.Builder("setPlaybackParametersNotifiesListenerForEveryChangeOnce")
+            .pause()
+            .waitForPlaybackState(Player.STATE_READY)
+            .setPlaybackParameters(new PlaybackParameters(1.1f))
+            .setPlaybackParameters(new PlaybackParameters(1.2f))
+            .setPlaybackParameters(new PlaybackParameters(1.3f))
+            .play()
+            .build();
+    List<PlaybackParameters> reportedPlaybackParameters = new ArrayList<>();
+    EventListener listener =
+        new EventListener() {
+          @Override
+          public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) {
+            reportedPlaybackParameters.add(playbackParameters);
+          }
+        };
+    new ExoPlayerTestRunner.Builder()
+        .setActionSchedule(actionSchedule)
+        .setEventListener(listener)
+        .build(context)
+        .start()
+        .blockUntilEnded(TIMEOUT_MS);
+
+    assertThat(reportedPlaybackParameters)
+        .containsExactly(
+            new PlaybackParameters(1.1f),
+            new PlaybackParameters(1.2f),
+            new PlaybackParameters(1.3f))
+        .inOrder();
+  }
+
+  @Test
+  public void
+      setUnsupportedPlaybackParametersConsecutivelyNotifiesListenerForEveryChangeOnceAndResetsOnceHandled()
+          throws Exception {
+    Renderer renderer =
+        new FakeMediaClockRenderer(Builder.AUDIO_FORMAT) {
+          @Override
+          public long getPositionUs() {
+            return 0;
+          }
+
+          @Override
+          public void setPlaybackParameters(PlaybackParameters playbackParameters) {}
+
+          @Override
+          public PlaybackParameters getPlaybackParameters() {
+            return PlaybackParameters.DEFAULT;
+          }
+        };
+    ActionSchedule actionSchedule =
+        new ActionSchedule.Builder("setUnsupportedPlaybackParametersNotifiesListenersCorrectly")
+            .pause()
+            .waitForPlaybackState(Player.STATE_READY)
+            .setPlaybackParameters(new PlaybackParameters(1.1f))
+            .setPlaybackParameters(new PlaybackParameters(1.2f))
+            .setPlaybackParameters(new PlaybackParameters(1.3f))
+            .play()
+            .build();
+    List<PlaybackParameters> reportedPlaybackParameters = new ArrayList<>();
+    EventListener listener =
+        new EventListener() {
+          @Override
+          public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) {
+            reportedPlaybackParameters.add(playbackParameters);
+          }
+        };
+    new ExoPlayerTestRunner.Builder()
+        .setSupportedFormats(Builder.AUDIO_FORMAT)
+        .setRenderers(renderer)
+        .setActionSchedule(actionSchedule)
+        .setEventListener(listener)
+        .build(context)
+        .start()
+        .blockUntilEnded(TIMEOUT_MS);
+
+    assertThat(reportedPlaybackParameters)
+        .containsExactly(
+            new PlaybackParameters(1.1f),
+            new PlaybackParameters(1.2f),
+            new PlaybackParameters(1.3f),
+            PlaybackParameters.DEFAULT)
+        .inOrder();
+  }
+
   // Internal methods.
 
   private static ActionSchedule.Builder addSurfaceSwitch(ActionSchedule.Builder builder) {