Move EventListener registration down from Player

The deprecated `Player.addListener(EventListener)`
is moved out of Player into its subclasses
(CastPlayer and ExoPlayer).

This is unlikely to break users because:
 - the method has been deprecated in the last major version
 - the method is still present in the major implementations

If an users is affected, they can either:
 - use ExoPlayer instead of Player
 - (recommended) switch to Player.Listener.

Additionally update the threading guarantees that did not
reflect the current implementation.

PiperOrigin-RevId: 397272144
This commit is contained in:
krocard 2021-09-17 10:11:04 +01:00 committed by Oliver Woodman
parent a75f902c81
commit 74c6ef9ba0
8 changed files with 64 additions and 109 deletions

View File

@ -24,6 +24,8 @@
the start or after the end of the media are now handled as seeks to the the start or after the end of the media are now handled as seeks to the
start and end respectively start and end respectively
([#8906](https://github.com/google/ExoPlayer/issues/8906)). ([#8906](https://github.com/google/ExoPlayer/issues/8906)).
* Move `Player.addListener(EventListener)` and
`Player.removeListener(EventListener)` out of `Player` into subclasses.
* Extractors: * Extractors:
* Support TS packets without PTS flag * Support TS packets without PTS flag
([#9294](https://github.com/google/ExoPlayer/issues/9294)). ([#9294](https://github.com/google/ExoPlayer/issues/9294)).
@ -92,9 +94,8 @@
deprecated since 2.11.0. deprecated since 2.11.0.
* Remove `DownloadService.onDownloadChanged` and * Remove `DownloadService.onDownloadChanged` and
`DownloadService.onDownloadRemoved`. Instead, use `DownloadService.onDownloadRemoved`. Instead, use
`DownloadManager.addListener` to register a listener directly to `DownloadManager.addListener` to register a listener directly to the
the `DownloadManager` returned through `DownloadManager` returned through `DownloadService.getDownloadManager`.
`DownloadService.getDownloadManager`.
* Cast extension: * Cast extension:
* Implement `CastPlayer.setPlaybackParameters(PlaybackParameters)` to * Implement `CastPlayer.setPlaybackParameters(PlaybackParameters)` to
support setting the playback speed support setting the playback speed

View File

@ -276,7 +276,17 @@ public final class CastPlayer extends BasePlayer {
addListener(eventListener); addListener(eventListener);
} }
@Override /**
* Registers a listener to receive events from the player.
*
* <p>The listener's methods will be called on the thread associated with {@link
* #getApplicationLooper()}.
*
* @param listener The listener to register.
* @deprecated Use {@link #addListener(Listener)} and {@link #removeListener(Listener)} instead.
*/
@Deprecated
@SuppressWarnings("deprecation")
public void addListener(EventListener listener) { public void addListener(EventListener listener) {
listeners.add(listener); listeners.add(listener);
} }
@ -287,7 +297,15 @@ public final class CastPlayer extends BasePlayer {
removeListener(eventListener); removeListener(eventListener);
} }
@Override /**
* Unregister a listener registered through {@link #addListener(EventListener)}. The listener will
* no longer receive events from the player.
*
* @param listener The listener to unregister.
* @deprecated Use {@link #addListener(Listener)} and {@link #removeListener(Listener)} instead.
*/
@Deprecated
@SuppressWarnings("deprecation")
public void removeListener(EventListener listener) { public void removeListener(EventListener listener) {
listeners.remove(listener); listeners.remove(listener);
} }

View File

@ -43,28 +43,16 @@ public class ForwardingPlayer implements Player {
this.player = player; this.player = player;
} }
@Override @Deprecated
public Looper getApplicationLooper() { public Looper getApplicationLooper() {
return player.getApplicationLooper(); return player.getApplicationLooper();
} }
@Deprecated @Deprecated
@Override
public void addListener(EventListener listener) {
player.addListener(new ForwardingEventListener(this, listener));
}
@Override
public void addListener(Listener listener) { public void addListener(Listener listener) {
player.addListener(new ForwardingListener(this, listener)); player.addListener(new ForwardingListener(this, listener));
} }
@Deprecated
@Override
public void removeListener(EventListener listener) {
player.removeListener(new ForwardingEventListener(this, listener));
}
@Override @Override
public void removeListener(Listener listener) { public void removeListener(Listener listener) {
player.removeListener(new ForwardingListener(this, listener)); player.removeListener(new ForwardingListener(this, listener));

View File

@ -1434,40 +1434,16 @@ public interface Player {
*/ */
Looper getApplicationLooper(); Looper getApplicationLooper();
/**
* Registers a listener to receive events from the player.
*
* <p>The listener's methods will be called on the thread that was used to construct the player.
* However, if the thread used to construct the player does not have a {@link Looper}, then the
* listener will be called on the main thread.
*
* @param listener The listener to register.
* @deprecated Use {@link #addListener(Listener)} and {@link #removeListener(Listener)} instead.
*/
@Deprecated
void addListener(EventListener listener);
/** /**
* Registers a listener to receive all events from the player. * Registers a listener to receive all events from the player.
* *
* <p>The listener's methods will be called on the thread that was used to construct the player. * <p>The listener's methods will be called on the thread associated with {@link
* However, if the thread used to construct the player does not have a {@link Looper}, then the * #getApplicationLooper()}.
* listener will be called on the main thread.
* *
* @param listener The listener to register. * @param listener The listener to register.
*/ */
void addListener(Listener listener); void addListener(Listener listener);
/**
* Unregister a listener registered through {@link #addListener(EventListener)}. The listener will
* no longer receive events from the player.
*
* @param listener The listener to unregister.
* @deprecated Use {@link #addListener(Listener)} and {@link #removeListener(Listener)} instead.
*/
@Deprecated
void removeListener(EventListener listener);
/** /**
* Unregister a listener registered through {@link #addListener(Listener)}. The listener will no * Unregister a listener registered through {@link #addListener(Listener)}. The listener will no
* longer receive events. * longer receive events.

View File

@ -61,22 +61,6 @@ public class ForwardingPlayerTest {
assertThat(player.listeners).hasSize(2); assertThat(player.listeners).hasSize(2);
} }
@Test
@SuppressWarnings("deprecation") // Testing backwards compatibility with deprecated method.
public void addEventListener_addsForwardingListener() {
FakePlayer player = new FakePlayer(ApplicationProvider.getApplicationContext());
Player.EventListener listener1 = mock(Player.EventListener.class);
Player.EventListener listener2 = mock(Player.EventListener.class);
ForwardingPlayer forwardingPlayer = new ForwardingPlayer(player);
forwardingPlayer.addListener(listener1);
// Add listener1 again.
forwardingPlayer.addListener(listener1);
forwardingPlayer.addListener(listener2);
assertThat(player.eventListeners).hasSize(2);
}
@Test @Test
public void removeListener_removesForwardingListener() { public void removeListener_removesForwardingListener() {
FakePlayer player = new FakePlayer(ApplicationProvider.getApplicationContext()); FakePlayer player = new FakePlayer(ApplicationProvider.getApplicationContext());
@ -95,25 +79,6 @@ public class ForwardingPlayerTest {
assertThat(player.listeners).isEmpty(); assertThat(player.listeners).isEmpty();
} }
@Test
@SuppressWarnings("deprecation") // Testing backwards compatibility with deprecated method.
public void removeEventListener_removesForwardingListener() {
FakePlayer player = new FakePlayer(ApplicationProvider.getApplicationContext());
Player.EventListener listener1 = mock(Player.EventListener.class);
Player.EventListener listener2 = mock(Player.EventListener.class);
ForwardingPlayer forwardingPlayer = new ForwardingPlayer(player);
forwardingPlayer.addListener(listener1);
forwardingPlayer.addListener(listener2);
forwardingPlayer.removeListener(listener1);
assertThat(player.eventListeners).hasSize(1);
// Remove same listener again.
forwardingPlayer.removeListener(listener1);
assertThat(player.eventListeners).hasSize(1);
forwardingPlayer.removeListener(listener2);
assertThat(player.eventListeners).isEmpty();
}
@Test @Test
public void onEvents_passesForwardingPlayerAsArgument() { public void onEvents_passesForwardingPlayerAsArgument() {
FakePlayer player = new FakePlayer(ApplicationProvider.getApplicationContext()); FakePlayer player = new FakePlayer(ApplicationProvider.getApplicationContext());
@ -217,32 +182,17 @@ public class ForwardingPlayerTest {
private static class FakePlayer extends StubExoPlayer { private static class FakePlayer extends StubExoPlayer {
@SuppressWarnings("deprecation") // Use of deprecated type for backwards compatibility.
private final Set<EventListener> eventListeners = new HashSet<>();
private final Set<Listener> listeners = new HashSet<>(); private final Set<Listener> listeners = new HashSet<>();
public FakePlayer(Context context) { public FakePlayer(Context context) {
super(context); super(context);
} }
@Override
@SuppressWarnings("deprecation") // Implementing deprecated method.
public void addListener(EventListener listener) {
eventListeners.add(listener);
}
@Override @Override
public void addListener(Listener listener) { public void addListener(Listener listener) {
listeners.add(listener); listeners.add(listener);
} }
@Override
@SuppressWarnings("deprecation") // Implementing deprecated method.
public void removeListener(EventListener listener) {
eventListeners.remove(listener);
}
@Override @Override
public void removeListener(Listener listener) { public void removeListener(Listener listener) {
listeners.remove(listener); listeners.remove(listener);

View File

@ -873,6 +873,30 @@ public interface ExoPlayer extends Player {
@Deprecated @Deprecated
DeviceComponent getDeviceComponent(); DeviceComponent getDeviceComponent();
/**
* Registers a listener to receive events from the player.
*
* <p>The listener's methods will be called on the thread associated with {@link
* #getApplicationLooper()}.
*
* @param listener The listener to register.
* @deprecated Use {@link #addListener(Listener)} and {@link #removeListener(Listener)} instead.
*/
@Deprecated
@SuppressWarnings("deprecation")
void addListener(EventListener listener);
/**
* Unregister a listener registered through {@link #addListener(EventListener)}. The listener will
* no longer receive events from the player.
*
* @param listener The listener to unregister.
* @deprecated Use {@link #addListener(Listener)} and {@link #removeListener(Listener)} instead.
*/
@Deprecated
@SuppressWarnings("deprecation")
void removeListener(EventListener listener);
/** /**
* Adds a listener to receive audio offload events. * Adds a listener to receive audio offload events.
* *

View File

@ -289,24 +289,22 @@ import java.util.concurrent.CopyOnWriteArraySet;
@Override @Override
public void addListener(Listener listener) { public void addListener(Listener listener) {
EventListener eventListener = listener; addEventListener(listener);
addListener(eventListener);
}
@Override
public void addListener(Player.EventListener listener) {
listeners.add(listener);
} }
@Override @Override
public void removeListener(Listener listener) { public void removeListener(Listener listener) {
EventListener eventListener = listener; removeEventListener(listener);
removeListener(eventListener);
} }
@Override @SuppressWarnings("deprecation") // Register deprecated EventListener.
public void removeListener(Player.EventListener listener) { public void addEventListener(Player.EventListener eventListener) {
listeners.remove(listener); listeners.add(eventListener);
}
@SuppressWarnings("deprecation") // Deregister deprecated EventListener.
public void removeEventListener(Player.EventListener eventListener) {
listeners.remove(eventListener);
} }
public void addAudioOffloadListener(AudioOffloadListener listener) { public void addAudioOffloadListener(AudioOffloadListener listener) {

View File

@ -558,7 +558,7 @@ public class SimpleExoPlayer extends BasePlayer
builder.looper, builder.looper,
/* wrappingPlayer= */ this, /* wrappingPlayer= */ this,
additionalPermanentAvailableCommands); additionalPermanentAvailableCommands);
player.addListener(componentListener); player.addEventListener(componentListener);
player.addAudioOffloadListener(componentListener); player.addAudioOffloadListener(componentListener);
if (builder.foregroundModeTimeoutMs > 0) { if (builder.foregroundModeTimeoutMs > 0) {
player.experimentalSetForegroundModeTimeoutMs(builder.foregroundModeTimeoutMs); player.experimentalSetForegroundModeTimeoutMs(builder.foregroundModeTimeoutMs);
@ -1082,7 +1082,7 @@ public class SimpleExoPlayer extends BasePlayer
public void addListener(Player.EventListener listener) { public void addListener(Player.EventListener listener) {
// Don't verify application thread. We allow calls to this method from any thread. // Don't verify application thread. We allow calls to this method from any thread.
Assertions.checkNotNull(listener); Assertions.checkNotNull(listener);
player.addListener(listener); player.addEventListener(listener);
} }
@Override @Override
@ -1097,7 +1097,7 @@ public class SimpleExoPlayer extends BasePlayer
@Override @Override
public void removeListener(Player.EventListener listener) { public void removeListener(Player.EventListener listener) {
// Don't verify application thread. We allow calls to this method from any thread. // Don't verify application thread. We allow calls to this method from any thread.
player.removeListener(listener); player.removeEventListener(listener);
} }
@Override @Override