MediaSession2: Clean up TODOs under frameworks/base/media
This CL includes following changes
- Remove outdated TODOs
- Added buganizer issue if the remaining work take more than 5m
Test: Run MediaComponents tests
Change-Id: I8968e12aabcbc67f69dbf14485b3716d0e95779b
diff --git a/media/java/android/media/MediaController2.java b/media/java/android/media/MediaController2.java
index 2ef2bb3..da2b8e0 100644
--- a/media/java/android/media/MediaController2.java
+++ b/media/java/android/media/MediaController2.java
@@ -139,13 +139,15 @@
/**
* Called when the playlist is changed.
* <p>
- * When it's called, you should invalidate previous playback information such as position,
- * player state, current item, etc.
+ * If the previously playing media item is gone, you should invalidate previous playback
+ * information and wait for later callbacks.
*
* @param controller the controller for this event
* @param playlist A new playlist set by the session.
+ * @see #onPositionChanged(long, long)
+ * @see #onBufferedPositionChanged(long)
+ * @see #onCurrentPlaylistItemChanged(MediaItem2)
*/
- // TODO(jaewan): Enhance doc
public void onPlaylistChanged(@NonNull MediaController2 controller,
@NonNull List<MediaItem2> playlist) { }
@@ -156,7 +158,7 @@
* @param state latest playback state
* @hide
*/
- // TODo(jaewan): Remove
+ // TODO(jaewan): Remove (b/73971431)
public void onPlaybackStateChanged(@NonNull MediaController2 controller,
@NonNull PlaybackState2 state) { }
@@ -208,9 +210,15 @@
/**
* Called when the player's current playing item is changed
+ * <p>
+ * When it's called, you should invalidate previous playback information and wait for later
+ * callbacks.
*
* @param controller the controller for this event
* @param item new item
+ * @see #onPositionChanged(long, long)
+ * @see #onBufferedPositionChanged(long)
+ * @see #onCurrentPlaylistItemChanged(MediaItem2)
*/
public void onCurrentPlaylistItemChanged(@NonNull MediaController2 controller,
@NonNull MediaItem2 item) { }
@@ -325,7 +333,6 @@
* @param executor executor to run callbacks on.
* @param callback controller callback to receive changes in
*/
- // TODO(jaewan): Put @CallbackExecutor to the constructor.
public MediaController2(@NonNull Context context, @NonNull SessionToken2 token,
@NonNull @CallbackExecutor Executor executor, @NonNull ControllerCallback callback) {
super();
@@ -656,7 +663,7 @@
* Set the playback speed.
*/
public void setPlaybackSpeed(float speed) {
- // TODO: implement this
+ // TODO(jaewan): implement this (b/74093080)
}
/**
diff --git a/media/java/android/media/MediaItem2.java b/media/java/android/media/MediaItem2.java
index f9eceab..f6edd0c 100644
--- a/media/java/android/media/MediaItem2.java
+++ b/media/java/android/media/MediaItem2.java
@@ -70,7 +70,6 @@
* @return a new bundle instance
*/
public Bundle toBundle() {
- // TODO(jaewan): Fill here when we rebase.
return mProvider.toBundle_impl();
}
diff --git a/media/java/android/media/MediaMetadata2.java b/media/java/android/media/MediaMetadata2.java
index 07367bb..502f929 100644
--- a/media/java/android/media/MediaMetadata2.java
+++ b/media/java/android/media/MediaMetadata2.java
@@ -37,7 +37,6 @@
// - Don't implement Parcelable for updatable support.
// - Also support MediaDescription features. MediaDescription is deprecated instead because
// it was insufficient for controller to display media contents.
-// TODO(jaewan): Add @see for APIs from MediaDescription
public final class MediaMetadata2 {
/**
* The metadata key for a {@link CharSequence} or {@link String} typed value to retrieve the
diff --git a/media/java/android/media/MediaSession2.java b/media/java/android/media/MediaSession2.java
index bbc4299..d94be66 100644
--- a/media/java/android/media/MediaSession2.java
+++ b/media/java/android/media/MediaSession2.java
@@ -81,10 +81,6 @@
public class MediaSession2 implements AutoCloseable, MediaPlaylistController {
private final MediaSession2Provider mProvider;
- // TODO(jaewan): Should we define IntDef? Currently we don't have to allow subclass to add more.
- // TODO(jaewan): Shouldn't we pull out?
- // TODO(jaewan): Should we also protect getters not related with metadata?
- // Getters are getPlaybackState(), getSessionActivity(), getPlaylistParams()
// Next ID: 23
/**
* Command code for the custom command which can be defined by string action in the
@@ -338,7 +334,6 @@
* If {@link #getCommandCode()} is {@link #COMMAND_CODE_CUSTOM}), it's custom command and
* {@link #getCustomCommand()} shouldn't be {@code null}.
*/
- // TODO(jaewan): Move this into the updatable.
public static final class Command {
private final CommandProvider mProvider;
@@ -434,8 +429,7 @@
}
public List<Command> getCommands() {
- // TODO: implement this
- return null;
+ return mProvider.getCommands_impl();
}
/**
@@ -469,7 +463,7 @@
* If it's not set, the session will accept all controllers and all incoming commands by
* default.
*/
- // TODO(jaewan): Can we move this inside of the updatable for default implementation.
+ // TODO(jaewan): Move this to updatable for default implementation (b/74091963)
public static abstract class SessionCallback {
private final Context mContext;
@@ -489,7 +483,6 @@
* @param controller controller information.
* @return allowed commands. Can be {@code null} to reject coonnection.
*/
- // TODO(jaewan): Change return type. Once we do, null is for reject.
public @Nullable CommandGroup onConnect(@NonNull MediaSession2 session,
@NonNull ControllerInfo controller) {
CommandGroup commands = new CommandGroup(mContext);
@@ -716,7 +709,7 @@
* @param player a {@link MediaPlayerBase} that handles actual media playback in your app.
*/
U setPlayer(@NonNull MediaPlayerBase player) {
- // TODO: Change the provider properly
+ // TODO(jaewan): Change the provider properly (b/74093082)
mProvider.setPlayer_impl(player, null, null);
return (U) this;
}
@@ -729,7 +722,7 @@
* {@code player.}
*/
U setPlaylistController(@NonNull MediaPlaylistController mplc) {
- // TODO: implement this
+ // TODO(jaewan): implement this (b/74093082)
return (U) this;
}
@@ -740,7 +733,7 @@
* @param volumeProvider The provider that will receive volume button events.
*/
U setVolumeProvider(@NonNull VolumeProvider2 volumeProvider) {
- // TODO: implement this
+ // TODO(jaewan): implement this (b/74093082)
return (U) this;
}
@@ -865,7 +858,6 @@
/**
* @hide
*/
- // TODO(jaewan): Also accept componentName to check notificaiton listener.
public ControllerInfo(Context context, int uid, int pid, String packageName,
IInterface callback) {
mProvider = ApiLoader.getProvider(context)
@@ -912,18 +904,12 @@
@Override
public boolean equals(Object obj) {
- if (!(obj instanceof ControllerInfo)) {
- return false;
- }
- ControllerInfo other = (ControllerInfo) obj;
- return mProvider.equals_impl(other.mProvider);
+ return mProvider.equals_impl(obj);
}
@Override
public String toString() {
- // TODO(jaewan): Move this to updatable.
- return "ControllerInfo {pkg=" + getPackageName() + ", uid=" + getUid() + ", trusted="
- + isTrusted() + "}";
+ return mProvider.toString_impl();
}
}
@@ -1193,14 +1179,13 @@
*
* @param player a {@link MediaPlayerBase} that handles actual media playback in your app.
* @param mplc a {@link MediaPlaylistController} that manages playlist of the
- * {@code player.}
+ * {@code player}
* @param volumeProvider The provider that will receive volume button events. If
* {@code null}, system will adjust the appropriate stream volume for this session's player.
*/
public void updatePlayer(@NonNull MediaPlayerBase player,
- @Nullable MediaPlaylistController mplc, @NonNull VolumeProvider2 volumeProvider) {
- // TODO: rename setPlayer_impl to updatePlayer_impl
- mProvider.setPlayer_impl(player, mplc, volumeProvider);
+ @Nullable MediaPlaylistController mplc, @Nullable VolumeProvider2 volumeProvider) {
+ mProvider.updatePlayer_impl(player, mplc, volumeProvider);
}
@Override
@@ -1219,19 +1204,16 @@
/**
* @return playlist controller
*/
- public @Nullable
- MediaPlaylistController getMediaPlaylistController() {
- // TODO: implement this
+ public @Nullable MediaPlaylistController getMediaPlaylistController() {
+ // TODO(jaewan): implement this (b/74090741)
return null;
}
/**
* @return volume provider
*/
- public @Nullable
- VolumeProvider2 getVolumeProvider() {
- // TODO: implement this
- return null;
+ public @Nullable VolumeProvider2 getVolumeProvider() {
+ return mProvider.getVolumeProvider_impl();
}
/**
@@ -1252,7 +1234,7 @@
* @param afr the full request parameters
*/
public void setAudioFocusRequest(AudioFocusRequest afr) {
- // TODO: implement this
+ // TODO(jaewan): implement this (b/72529899)
// mProvider.setAudioFocusRequest_impl(focusGain);
}
@@ -1426,8 +1408,6 @@
*
* @throws IllegalArgumentException if the play list is null
*/
- // TODO(jaewan): Remove with index was previously rejected by council (b/36524925)
- // TODO(jaewan): Should we also add movePlaylistItem from index to index?
public void removePlaylistItem(MediaItem2 item) {
mProvider.removePlaylistItem_impl(item);
}
@@ -1518,7 +1498,7 @@
* @throws IllegalArgumentException if executor or callback is {@code null}.
* @hide
*/
- // TODO(jaewan): Unhide or remove
+ // TODO(jaewan): Remove (b/74157064)
public void registerPlayerEventCallback(@NonNull @CallbackExecutor Executor executor,
@NonNull PlayerEventCallback callback) {
mProvider.registerPlayerEventCallback_impl(executor, callback);
@@ -1531,7 +1511,7 @@
* @throws IllegalArgumentException if the callback is {@code null}.
* @hide
*/
- // TODO(jaewan): Unhide or remove
+ // TODO(jaewan): Remove (b/74157064)
public void unregisterPlayerEventCallback(@NonNull PlayerEventCallback callback) {
mProvider.unregisterPlayerEventCallback_impl(callback);
}
@@ -1552,7 +1532,7 @@
* @return speed
*/
public float getPlaybackSpeed() {
- // TODO: implement this
+ // TODO(jaewan): implement this (b/74093080)
return -1;
}
@@ -1560,6 +1540,6 @@
* Set the playback speed.
*/
public void setPlaybackSpeed(float speed) {
- // TODO: implement this
+ // TODO(jaewan): implement this (b/74093080)
}
}
diff --git a/media/java/android/media/PlaybackState2.java b/media/java/android/media/PlaybackState2.java
index 7afb579..afc2bfa 100644
--- a/media/java/android/media/PlaybackState2.java
+++ b/media/java/android/media/PlaybackState2.java
@@ -33,7 +33,7 @@
* the current playback position and extra.
* @hide
*/
-// TODO(jaewan): Remove this.
+// TODO(jaewan): Remove this (b/73971431)
public final class PlaybackState2 {
// Similar to the PlaybackState with following changes
// - Not implement Parcelable and added from/toBundle()
@@ -76,7 +76,6 @@
// - Removed actions and custom actions.
// - Removed error string
// - Repeat mode / shuffle mode is now in the PlaylistParams
- // TODO(jaewan): Replace states from MediaPlayer2
/**
* @hide
*/
diff --git a/media/java/android/media/update/MediaSession2Provider.java b/media/java/android/media/update/MediaSession2Provider.java
index d0ec104..f97a6f0 100644
--- a/media/java/android/media/update/MediaSession2Provider.java
+++ b/media/java/android/media/update/MediaSession2Provider.java
@@ -44,9 +44,10 @@
*/
public interface MediaSession2Provider extends TransportControlProvider {
void close_impl();
- void setPlayer_impl(MediaPlayerBase player, MediaPlaylistController mplc,
+ void updatePlayer_impl(MediaPlayerBase player, MediaPlaylistController mplc,
VolumeProvider2 volumeProvider);
MediaPlayerBase getPlayer_impl();
+ VolumeProvider2 getVolumeProvider_impl();
SessionToken2 getToken_impl();
List<ControllerInfo> getConnectedControllers_impl();
void setCustomLayout_impl(ControllerInfo controller, List<CommandButton> layout);
@@ -84,6 +85,7 @@
void removeCommand_impl(Command command);
boolean hasCommand_impl(Command command);
boolean hasCommand_impl(int code);
+ List<Command> getCommands_impl();
Bundle toBundle_impl();
}
@@ -109,7 +111,8 @@
int getUid_impl();
boolean isTrusted_impl();
int hashCode_impl();
- boolean equals_impl(ControllerInfoProvider obj);
+ boolean equals_impl(Object obj);
+ String toString_impl();
}
interface PlaylistParamsProvider {