Fix some playback parameter signalling problems.

Playback parameter signalling can be quite complex because
 (a) the renderer clock often has a delay before it realizes
     that it doesn't support a previously set speed and
 (b) the speed set on media clock sometimes intentionally
     differs from the one surfaced to the user, e.g. during
     live speed adjustment or when overriding ad playback
     speed to 1.0f.

This change fixes two problems related to this signalling:
 1. When resetting the media clock speed at a period transition,
    we don't currently tell the renderers that this happened.
 2. When a delayed speed change update from the media clock is
    pending and the renderer for this media clock is disabled
    before the change can be handled, the pending update becomes
    stale but it still applied later and overrides any other valid
    speed set in the meantime.

Both edge cases are also covered by extended or new player tests.

Issue: google/ExoPlayer#10882

#minor-release

PiperOrigin-RevId: 512658918
This commit is contained in:
tonihei 2023-02-27 18:06:36 +00:00
parent 36aa29809d
commit e79b47ccff
3 changed files with 161 additions and 72 deletions

View File

@ -26,6 +26,9 @@
* Encapsulate Opus frames in Ogg packets in direct playbacks (offload). * Encapsulate Opus frames in Ogg packets in direct playbacks (offload).
* Fix broken gapless MP3 playback on Samsung devices * Fix broken gapless MP3 playback on Samsung devices
([#8594](https://github.com/google/ExoPlayer/issues/8594)). ([#8594](https://github.com/google/ExoPlayer/issues/8594)).
* Fix bug where playback speeds set immediately after disabling audio may
be overridden by a previous speed change
([#10882](https://github.com/google/ExoPlayer/issues/10882)).
* Video: * Video:
* Map HEVC HDR10 format to `HEVCProfileMain10HDR10` instead of * Map HEVC HDR10 format to `HEVCProfileMain10HDR10` instead of
`HEVCProfileMain10`. `HEVCProfileMain10`.

View File

@ -963,7 +963,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
livePlaybackSpeedControl.getAdjustedPlaybackSpeed( livePlaybackSpeedControl.getAdjustedPlaybackSpeed(
getCurrentLiveOffsetUs(), getTotalBufferedDurationUs()); getCurrentLiveOffsetUs(), getTotalBufferedDurationUs());
if (mediaClock.getPlaybackParameters().speed != adjustedSpeed) { if (mediaClock.getPlaybackParameters().speed != adjustedSpeed) {
mediaClock.setPlaybackParameters(playbackInfo.playbackParameters.withSpeed(adjustedSpeed)); setMediaClockPlaybackParameters(playbackInfo.playbackParameters.withSpeed(adjustedSpeed));
handlePlaybackParameters( handlePlaybackParameters(
playbackInfo.playbackParameters, playbackInfo.playbackParameters,
/* currentPlaybackSpeed= */ mediaClock.getPlaybackParameters().speed, /* currentPlaybackSpeed= */ mediaClock.getPlaybackParameters().speed,
@ -973,6 +973,12 @@ import java.util.concurrent.atomic.AtomicBoolean;
} }
} }
private void setMediaClockPlaybackParameters(PlaybackParameters playbackParameters) {
// Previously sent speed updates from the media clock now become stale.
handler.removeMessages(MSG_PLAYBACK_PARAMETERS_CHANGED_INTERNAL);
mediaClock.setPlaybackParameters(playbackParameters);
}
private void notifyTrackSelectionRebuffer() { private void notifyTrackSelectionRebuffer() {
MediaPeriodHolder periodHolder = queue.getPlayingPeriod(); MediaPeriodHolder periodHolder = queue.getPlayingPeriod();
while (periodHolder != null) { while (periodHolder != null) {
@ -1359,7 +1365,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
private void setPlaybackParametersInternal(PlaybackParameters playbackParameters) private void setPlaybackParametersInternal(PlaybackParameters playbackParameters)
throws ExoPlaybackException { throws ExoPlaybackException {
mediaClock.setPlaybackParameters(playbackParameters); setMediaClockPlaybackParameters(playbackParameters);
handlePlaybackParameters(mediaClock.getPlaybackParameters(), /* acknowledgeCommand= */ true); handlePlaybackParameters(mediaClock.getPlaybackParameters(), /* acknowledgeCommand= */ true);
} }
@ -1674,7 +1680,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
nextPendingMessageIndexHint = nextPendingMessageIndex; nextPendingMessageIndexHint = nextPendingMessageIndex;
} }
private void ensureStopped(Renderer renderer) throws ExoPlaybackException { private void ensureStopped(Renderer renderer) {
if (renderer.getState() == Renderer.STATE_STARTED) { if (renderer.getState() == Renderer.STATE_STARTED) {
renderer.stop(); renderer.stop();
} }
@ -1931,14 +1937,20 @@ import java.util.concurrent.atomic.AtomicBoolean;
MediaPeriodId newPeriodId, MediaPeriodId newPeriodId,
Timeline oldTimeline, Timeline oldTimeline,
MediaPeriodId oldPeriodId, MediaPeriodId oldPeriodId,
long positionForTargetOffsetOverrideUs) { long positionForTargetOffsetOverrideUs)
throws ExoPlaybackException {
if (!shouldUseLivePlaybackSpeedControl(newTimeline, newPeriodId)) { if (!shouldUseLivePlaybackSpeedControl(newTimeline, newPeriodId)) {
// Live playback speed control is unused for the current period, reset speed to user-defined // Live playback speed control is unused for the current period, reset speed to user-defined
// playback parameters or 1.0 for ad playback. // playback parameters or 1.0 for ad playback.
PlaybackParameters targetPlaybackParameters = PlaybackParameters targetPlaybackParameters =
newPeriodId.isAd() ? PlaybackParameters.DEFAULT : playbackInfo.playbackParameters; newPeriodId.isAd() ? PlaybackParameters.DEFAULT : playbackInfo.playbackParameters;
if (!mediaClock.getPlaybackParameters().equals(targetPlaybackParameters)) { if (!mediaClock.getPlaybackParameters().equals(targetPlaybackParameters)) {
mediaClock.setPlaybackParameters(targetPlaybackParameters); setMediaClockPlaybackParameters(targetPlaybackParameters);
handlePlaybackParameters(
playbackInfo.playbackParameters,
targetPlaybackParameters.speed,
/* updatePlaybackInfo= */ false,
/* acknowledgeCommand= */ false);
} }
return; return;
} }
@ -1987,7 +1999,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
return maxReadPositionUs; return maxReadPositionUs;
} }
private void updatePeriods() throws ExoPlaybackException, IOException { private void updatePeriods() throws ExoPlaybackException {
if (playbackInfo.timeline.isEmpty() || !mediaSourceList.isPrepared()) { if (playbackInfo.timeline.isEmpty() || !mediaSourceList.isPrepared()) {
// No periods available. // No periods available.
return; return;
@ -2029,7 +2041,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
} }
} }
private void maybeUpdateReadingPeriod() { private void maybeUpdateReadingPeriod() throws ExoPlaybackException {
@Nullable MediaPeriodHolder readingPeriodHolder = queue.getReadingPeriod(); @Nullable MediaPeriodHolder readingPeriodHolder = queue.getReadingPeriod();
if (readingPeriodHolder == null) { if (readingPeriodHolder == null) {
return; return;

View File

@ -91,6 +91,7 @@ import android.media.AudioManager;
import android.net.Uri; import android.net.Uri;
import android.os.Handler; import android.os.Handler;
import android.os.Looper; import android.os.Looper;
import android.util.Pair;
import android.view.Surface; import android.view.Surface;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.media3.common.AdPlaybackState; import androidx.media3.common.AdPlaybackState;
@ -135,14 +136,12 @@ import androidx.media3.exoplayer.source.TrackGroupArray;
import androidx.media3.exoplayer.source.WrappingMediaSource; import androidx.media3.exoplayer.source.WrappingMediaSource;
import androidx.media3.exoplayer.source.ads.ServerSideAdInsertionMediaSource; import androidx.media3.exoplayer.source.ads.ServerSideAdInsertionMediaSource;
import androidx.media3.exoplayer.text.TextOutput; import androidx.media3.exoplayer.text.TextOutput;
import androidx.media3.exoplayer.trackselection.DefaultTrackSelector;
import androidx.media3.exoplayer.upstream.Allocation; import androidx.media3.exoplayer.upstream.Allocation;
import androidx.media3.exoplayer.upstream.Allocator; import androidx.media3.exoplayer.upstream.Allocator;
import androidx.media3.exoplayer.upstream.Loader; import androidx.media3.exoplayer.upstream.Loader;
import androidx.media3.exoplayer.video.VideoRendererEventListener; import androidx.media3.exoplayer.video.VideoRendererEventListener;
import androidx.media3.extractor.metadata.id3.BinaryFrame; import androidx.media3.extractor.metadata.id3.BinaryFrame;
import androidx.media3.extractor.metadata.id3.TextInformationFrame; import androidx.media3.extractor.metadata.id3.TextInformationFrame;
import androidx.media3.test.utils.Action;
import androidx.media3.test.utils.ActionSchedule; import androidx.media3.test.utils.ActionSchedule;
import androidx.media3.test.utils.ActionSchedule.PlayerRunnable; import androidx.media3.test.utils.ActionSchedule.PlayerRunnable;
import androidx.media3.test.utils.ActionSchedule.PlayerTarget; import androidx.media3.test.utils.ActionSchedule.PlayerTarget;
@ -3851,41 +3850,29 @@ public final class ExoPlayerTest {
@Test @Test
public void setPlaybackSpeedConsecutivelyNotifiesListenerForEveryChangeOnceAndIsMasked() public void setPlaybackSpeedConsecutivelyNotifiesListenerForEveryChangeOnceAndIsMasked()
throws Exception { throws Exception {
ExoPlayer player = new TestExoPlayerBuilder(context).build();
List<Float> maskedPlaybackSpeeds = new ArrayList<>(); List<Float> maskedPlaybackSpeeds = new ArrayList<>();
Action getPlaybackSpeedAction =
new Action("getPlaybackSpeed", /* description= */ null) {
@Override
protected void doActionImpl(
ExoPlayer player, DefaultTrackSelector trackSelector, @Nullable Surface surface) {
maskedPlaybackSpeeds.add(player.getPlaybackParameters().speed);
}
};
ActionSchedule actionSchedule =
new ActionSchedule.Builder(TAG)
.pause()
.waitForPlaybackState(Player.STATE_READY)
.setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.1f))
.apply(getPlaybackSpeedAction)
.setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.2f))
.apply(getPlaybackSpeedAction)
.setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.3f))
.apply(getPlaybackSpeedAction)
.play()
.build();
List<Float> reportedPlaybackSpeeds = new ArrayList<>(); List<Float> reportedPlaybackSpeeds = new ArrayList<>();
Player.Listener listener = player.addListener(
new Player.Listener() { new Player.Listener() {
@Override @Override
public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) { public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) {
reportedPlaybackSpeeds.add(playbackParameters.speed); reportedPlaybackSpeeds.add(playbackParameters.speed);
} }
}; });
new ExoPlayerTestRunner.Builder(context) player.setMediaSource(
.setActionSchedule(actionSchedule) new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT));
.setPlayerListener(listener) player.prepare();
.build() runUntilPlaybackState(player, Player.STATE_READY);
.start()
.blockUntilEnded(TIMEOUT_MS); player.setPlaybackSpeed(1.1f);
maskedPlaybackSpeeds.add(player.getPlaybackParameters().speed);
player.setPlaybackSpeed(1.2f);
maskedPlaybackSpeeds.add(player.getPlaybackParameters().speed);
player.setPlaybackSpeed(1.3f);
maskedPlaybackSpeeds.add(player.getPlaybackParameters().speed);
runUntilPendingCommandsAreFullyHandled(player);
player.release();
assertThat(reportedPlaybackSpeeds).containsExactly(1.1f, 1.2f, 1.3f).inOrder(); assertThat(reportedPlaybackSpeeds).containsExactly(1.1f, 1.2f, 1.3f).inOrder();
assertThat(maskedPlaybackSpeeds).isEqualTo(reportedPlaybackSpeeds); assertThat(maskedPlaybackSpeeds).isEqualTo(reportedPlaybackSpeeds);
@ -3895,46 +3882,28 @@ public final class ExoPlayerTest {
public void public void
setUnsupportedPlaybackSpeedConsecutivelyNotifiesListenerForEveryChangeOnceAndResetsOnceHandled() setUnsupportedPlaybackSpeedConsecutivelyNotifiesListenerForEveryChangeOnceAndResetsOnceHandled()
throws Exception { throws Exception {
Renderer renderer = ExoPlayer player =
new FakeMediaClockRenderer(C.TRACK_TYPE_AUDIO) { new TestExoPlayerBuilder(context)
@Override .setRenderers(new AudioClockRendererWithoutSpeedChangeSupport())
public long getPositionUs() {
return 0;
}
@Override
public void setPlaybackParameters(PlaybackParameters playbackParameters) {}
@Override
public PlaybackParameters getPlaybackParameters() {
return PlaybackParameters.DEFAULT;
}
};
ActionSchedule actionSchedule =
new ActionSchedule.Builder(TAG)
.pause()
.waitForPlaybackState(Player.STATE_READY)
.setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.1f))
.setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.2f))
.setPlaybackParameters(new PlaybackParameters(/* speed= */ 1.3f))
.play()
.build(); .build();
List<PlaybackParameters> reportedPlaybackParameters = new ArrayList<>(); List<PlaybackParameters> reportedPlaybackParameters = new ArrayList<>();
Player.Listener listener = player.addListener(
new Player.Listener() { new Player.Listener() {
@Override @Override
public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) { public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) {
reportedPlaybackParameters.add(playbackParameters); reportedPlaybackParameters.add(playbackParameters);
} }
}; });
new ExoPlayerTestRunner.Builder(context) player.setMediaSource(
.setSupportedFormats(ExoPlayerTestRunner.AUDIO_FORMAT) new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT));
.setRenderers(renderer) player.prepare();
.setActionSchedule(actionSchedule) runUntilPlaybackState(player, Player.STATE_READY);
.setPlayerListener(listener)
.build() player.setPlaybackSpeed(1.1f);
.start() player.setPlaybackSpeed(1.2f);
.blockUntilEnded(TIMEOUT_MS); player.setPlaybackSpeed(1.3f);
runUntilPendingCommandsAreFullyHandled(player);
player.release();
assertThat(reportedPlaybackParameters) assertThat(reportedPlaybackParameters)
.containsExactly( .containsExactly(
@ -3945,6 +3914,51 @@ public final class ExoPlayerTest {
.inOrder(); .inOrder();
} }
@Test
public void
setUnsupportedPlaybackSpeedDirectlyFollowedByDisablingTheRendererAndSupportedPlaybackSpeed_keepsCorrectFinalSpeedAndInformsListenersCorrectly()
throws Exception {
ExoPlayer player =
new TestExoPlayerBuilder(context)
.setRenderers(new AudioClockRendererWithoutSpeedChangeSupport())
.build();
List<PlaybackParameters> reportedPlaybackParameters = new ArrayList<>();
player.addListener(
new Player.Listener() {
@Override
public void onPlaybackParametersChanged(PlaybackParameters playbackParameters) {
reportedPlaybackParameters.add(playbackParameters);
}
});
player.setMediaSource(
new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.AUDIO_FORMAT));
player.prepare();
runUntilPlaybackState(player, Player.STATE_READY);
player.setPlaybackSpeed(2f);
// We need to do something that reliably triggers a position sync with the renderer, but no
// further playback progress as we want to test what happens if the parameter reset is still
// pending when we disable the audio renderer below. Calling play and pause will achieve this.
player.play();
player.pause();
// Disabling the audio renderer and setting a new speed should work, and should not be affected
// by the still pending parameter reset from above.
player.setTrackSelectionParameters(
player
.getTrackSelectionParameters()
.buildUpon()
.setTrackTypeDisabled(C.TRACK_TYPE_AUDIO, /* disabled= */ true)
.build());
player.setPlaybackSpeed(5f);
runUntilPendingCommandsAreFullyHandled(player);
player.release();
assertThat(reportedPlaybackParameters)
.containsExactly(
new PlaybackParameters(/* speed= */ 2f), new PlaybackParameters(/* speed= */ 5f))
.inOrder();
}
@Test @Test
public void simplePlaybackHasNoPlaybackSuppression() throws Exception { public void simplePlaybackHasNoPlaybackSuppression() throws Exception {
ActionSchedule actionSchedule = ActionSchedule actionSchedule =
@ -10089,8 +10103,10 @@ public final class ExoPlayerTest {
@Test @Test
public void setPlaybackSpeed_withAdPlayback_onlyAppliesToContent() throws Exception { public void setPlaybackSpeed_withAdPlayback_onlyAppliesToContent() throws Exception {
// Create renderer with media clock to listen to playback parameter changes. // Create renderer with media clock to listen to playback parameter changes and reported speed
// changes.
ArrayList<PlaybackParameters> playbackParameters = new ArrayList<>(); ArrayList<PlaybackParameters> playbackParameters = new ArrayList<>();
ArrayList<Pair<Float, Float>> speedChanges = new ArrayList<>();
FakeMediaClockRenderer audioRenderer = FakeMediaClockRenderer audioRenderer =
new FakeMediaClockRenderer(C.TRACK_TYPE_AUDIO) { new FakeMediaClockRenderer(C.TRACK_TYPE_AUDIO) {
private long positionUs; private long positionUs;
@ -10118,6 +10134,12 @@ public final class ExoPlayerTest {
? PlaybackParameters.DEFAULT ? PlaybackParameters.DEFAULT
: Iterables.getLast(playbackParameters); : Iterables.getLast(playbackParameters);
} }
@Override
public void setPlaybackSpeed(float currentPlaybackSpeed, float targetPlaybackSpeed)
throws ExoPlaybackException {
speedChanges.add(Pair.create(currentPlaybackSpeed, targetPlaybackSpeed));
}
}; };
ExoPlayer player = new TestExoPlayerBuilder(context).setRenderers(audioRenderer).build(); ExoPlayer player = new TestExoPlayerBuilder(context).setRenderers(audioRenderer).build();
AdPlaybackState adPlaybackState = AdPlaybackState adPlaybackState =
@ -10150,7 +10172,7 @@ public final class ExoPlayerTest {
runUntilPlaybackState(player, Player.STATE_ENDED); runUntilPlaybackState(player, Player.STATE_ENDED);
player.release(); player.release();
// Assert that the renderer received the playback speed updates at each ad/content boundary. // Assert that the media clock received the playback parameters at each ad/content boundary.
assertThat(playbackParameters) assertThat(playbackParameters)
.containsExactly( .containsExactly(
/* preroll ad */ new PlaybackParameters(1f), /* preroll ad */ new PlaybackParameters(1f),
@ -10161,6 +10183,18 @@ public final class ExoPlayerTest {
/* content after postroll */ new PlaybackParameters(5f)) /* content after postroll */ new PlaybackParameters(5f))
.inOrder(); .inOrder();
// Assert that the renderer received the speed changes at each ad/content boundary.
assertThat(speedChanges)
.containsExactly(
/* initial setup */ Pair.create(5f, 5f),
/* preroll ad */ Pair.create(1f, 5f),
/* content after preroll */ Pair.create(5f, 5f),
/* midroll ad */ Pair.create(1f, 5f),
/* content after midroll */ Pair.create(5f, 5f),
/* postroll ad */ Pair.create(1f, 5f),
/* content after postroll */ Pair.create(5f, 5f))
.inOrder();
// Assert that user-set speed was reported, but none of the ad overrides. // Assert that user-set speed was reported, but none of the ad overrides.
verify(mockListener).onPlaybackParametersChanged(any()); verify(mockListener).onPlaybackParametersChanged(any());
verify(mockListener).onPlaybackParametersChanged(new PlaybackParameters(5.0f)); verify(mockListener).onPlaybackParametersChanged(new PlaybackParameters(5.0f));
@ -12532,6 +12566,46 @@ public final class ExoPlayerTest {
} }
} }
private static final class AudioClockRendererWithoutSpeedChangeSupport
extends FakeMediaClockRenderer {
private PlaybackParameters playbackParameters;
private boolean delayingPlaybackParameterReset;
private long positionUs;
public AudioClockRendererWithoutSpeedChangeSupport() {
super(C.TRACK_TYPE_AUDIO);
playbackParameters = PlaybackParameters.DEFAULT;
}
@Override
protected void onPositionReset(long positionUs, boolean joining) throws ExoPlaybackException {
super.onPositionReset(positionUs, joining);
this.positionUs = positionUs;
}
@Override
public long getPositionUs() {
return positionUs;
}
@Override
public void setPlaybackParameters(PlaybackParameters playbackParameters) {
this.playbackParameters = playbackParameters;
// Similar to a real renderer, the missing speed support is only detected with a delay.
delayingPlaybackParameterReset = true;
}
@Override
public PlaybackParameters getPlaybackParameters() {
if (delayingPlaybackParameterReset) {
delayingPlaybackParameterReset = false;
return playbackParameters;
}
return PlaybackParameters.DEFAULT;
}
}
/** /**
* Returns an argument matcher for {@link Timeline} instances that ignores period and window uids. * Returns an argument matcher for {@link Timeline} instances that ignores period and window uids.
*/ */