diff options
6 files changed, 123 insertions, 76 deletions
diff --git a/app/src/main/java/de/danoeh/antennapod/adapter/DefaultActionButtonCallback.java b/app/src/main/java/de/danoeh/antennapod/adapter/DefaultActionButtonCallback.java index 1286d9dc7..fda6d48a5 100644 --- a/app/src/main/java/de/danoeh/antennapod/adapter/DefaultActionButtonCallback.java +++ b/app/src/main/java/de/danoeh/antennapod/adapter/DefaultActionButtonCallback.java @@ -1,8 +1,6 @@ package de.danoeh.antennapod.adapter; import android.content.Context; -import android.support.annotation.NonNull; -import android.content.Intent; import android.widget.Toast; import com.afollestad.materialdialogs.MaterialDialog; @@ -23,7 +21,6 @@ import de.danoeh.antennapod.core.storage.DownloadRequester; import de.danoeh.antennapod.core.util.IntentUtils; import de.danoeh.antennapod.core.util.LongList; import de.danoeh.antennapod.core.util.NetworkUtils; -import de.danoeh.antennapod.core.util.playback.PlaybackServiceStarter; /** * Default implementation of an ActionButtonCallback @@ -84,16 +81,16 @@ public class DefaultActionButtonCallback implements ActionButtonCallback { } } else { // media is downloaded if (media.isCurrentlyPlaying()) { - new PlaybackServiceStarter(context, media) - .startWhenPrepared(true) - .shouldStream(false) - .start(); +// new PlaybackServiceStarter(context, media) // TODO: [2716] probably not needed but not 100% sure +// .startWhenPrepared(true) +// .shouldStream(false) +// .start(); IntentUtils.sendLocalBroadcast(context, PlaybackService.ACTION_PAUSE_PLAY_CURRENT_EPISODE); } else if (media.isCurrentlyPaused()) { - new PlaybackServiceStarter(context, media) - .startWhenPrepared(true) - .shouldStream(false) - .start(); +// new PlaybackServiceStarter(context, media) // TODO: [2716] probably not needed but not 100% sure +// .startWhenPrepared(true) +// .shouldStream(false) +// .start(); IntentUtils.sendLocalBroadcast(context, PlaybackService.ACTION_RESUME_PLAY_CURRENT_EPISODE); } else { DBTasks.playMedia(context, media, false, true, false); diff --git a/core/src/main/java/de/danoeh/antennapod/core/receiver/MediaButtonReceiver.java b/core/src/main/java/de/danoeh/antennapod/core/receiver/MediaButtonReceiver.java index b191dbf8b..3b52ac212 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/receiver/MediaButtonReceiver.java +++ b/core/src/main/java/de/danoeh/antennapod/core/receiver/MediaButtonReceiver.java @@ -3,7 +3,6 @@ package de.danoeh.antennapod.core.receiver; import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; -import android.support.v4.content.ContextCompat; import android.util.Log; import android.view.KeyEvent; @@ -30,7 +29,7 @@ public class MediaButtonReceiver extends BroadcastReceiver { Intent serviceIntent = new Intent(context, PlaybackService.class); serviceIntent.putExtra(EXTRA_KEYCODE, event.getKeyCode()); serviceIntent.putExtra(EXTRA_SOURCE, event.getSource()); - ContextCompat.startForegroundService(context, serviceIntent); + context.startService(serviceIntent); // the service itself will determine if it needs to become a foreground service. } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java index 66cf7d244..632d0d41b 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java @@ -25,6 +25,7 @@ import android.preference.PreferenceManager; import android.support.annotation.NonNull; import android.support.annotation.StringRes; import android.support.v4.app.NotificationCompat; +import android.support.v4.content.ContextCompat; import android.support.v4.media.MediaBrowserCompat; import android.support.v4.media.MediaBrowserServiceCompat; import android.support.v4.media.MediaDescriptionCompat; @@ -75,6 +76,7 @@ import de.greenrobot.event.EventBus; /** * Controls the MediaPlayer that plays a FeedMedia-file + * TODO [2716] add some guidelines on how to / not to start / stop the service from callers. */ public class PlaybackService extends MediaBrowserServiceCompat { /** @@ -264,9 +266,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { Log.d(TAG, "Service created."); isRunning = true; - NotificationCompat.Builder notificationBuilder = createBasicNotification(); - startForeground(NOTIFICATION_ID, notificationBuilder.build()); - registerReceiver(autoStateUpdated, new IntentFilter("com.google.android.gms.car.media.STATUS")); registerReceiver(headsetDisconnected, new IntentFilter(Intent.ACTION_HEADSET_PLUG)); registerReceiver(shutdownReceiver, new IntentFilter(ACTION_SHUTDOWN_PLAYBACK_SERVICE)); @@ -342,7 +341,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { public void onDestroy() { super.onDestroy(); Log.d(TAG, "Service is about to be destroyed"); - stopForeground(true); + stopForeground(true); // TODO: [2716] unclear if stopForeground is still needed isRunning = false; started = false; currentMediaType = MediaType.UNKNOWN; @@ -367,7 +366,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { } private void stopService() { - stopForeground(true); + stopForeground(true); // TODO: [2716] unclear if stopForeground is still needed stopSelf(); } @@ -465,13 +464,16 @@ public class PlaybackService extends MediaBrowserServiceCompat { Playable playable = intent.getParcelableExtra(EXTRA_PLAYABLE); if (keycode == -1 && playable == null && !castDisconnect) { Log.e(TAG, "PlaybackService was started with no arguments"); - stopService(); + // some code call startService with no arg, causing playback to stop prematurely in typical case + // temporarily comment it out for now. + // TODO: Next step - find the source of the no-arg service start +// stopService(); // TODO: find the source of starting service with no argument that causes me to comment out this line temporarily return Service.START_NOT_STICKY; } if ((flags & Service.START_FLAG_REDELIVERY) != 0) { Log.d(TAG, "onStartCommand is a redelivered intent, calling stopForeground now."); - stopForeground(true); + stopForeground(true); // TODO: unclear if stopForeground is still needed } else { if (keycode != -1) { Log.d(TAG, "Received media button event"); @@ -494,7 +496,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { } mediaPlayer.playMediaObject(playable, stream, startWhenPrepared, prepareImmediately); } - setupNotification(playable); } return Service.START_NOT_STICKY; @@ -573,7 +574,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { started = false; } - stopForeground(true); // gets rid of persistent notification + stopForeground(true); // gets rid of persistent notification // TODO: [2716] unclear if stopForeground is still needed return true; default: Log.d(TAG, "Unhandled key code: " + keycode); @@ -606,8 +607,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { public void notifyVideoSurfaceAbandoned() { mediaPlayer.pause(true, false); mediaPlayer.resetVideoSurface(); - setupNotification(getPlayable()); - stopForeground(!UserPreferences.isPersistNotify()); } private final PlaybackServiceTaskManager.PSTMCallback taskManagerCallback = new PlaybackServiceTaskManager.PSTMCallback() { @@ -670,15 +669,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { break; case PAUSED: - if ((UserPreferences.isPersistNotify() || isCasting) && - android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) { - // do not remove notification on pause based on user pref and whether android version supports expanded notifications - // Change [Play] button to [Pause] - setupNotification(newInfo); - } else if (!UserPreferences.isPersistNotify() && !isCasting) { - // remove notification on pause - stopForeground(true); - } writePlayerStatusPlaybackPreferences(); break; @@ -689,7 +679,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { case PLAYING: writePlayerStatusPlaybackPreferences(); - setupNotification(newInfo); started = true; // set sleep timer if auto-enabled if (newInfo.oldPlayerStatus != null && newInfo.oldPlayerStatus != PlayerStatus.SEEKING && @@ -701,7 +690,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { case ERROR: writePlaybackPreferencesNoMediaPlaying(); - stopService(); break; } @@ -714,7 +702,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { @Override public void shouldStop() { - stopService(); + stopService(); // TODO: [2716] unclear if stopService() call is still needed } @Override @@ -763,7 +751,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { } sendNotificationBroadcast(NOTIFICATION_TYPE_ERROR, what); writePlaybackPreferencesNoMediaPlaying(); - stopService(); return true; } @@ -848,7 +835,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { taskManager.cancelPositionSaver(); writePlaybackPreferencesNoMediaPlaying(); if (!isCasting) { - stopForeground(true); + stopForeground(true); // TODO: [2716] unclear if stopForeground is still needed } } if (mediaType == null) { @@ -1063,6 +1050,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { private void updateMediaSession(final PlayerStatus playerStatus) { PlaybackStateCompat.Builder sessionState = new PlaybackStateCompat.Builder(); + @PlaybackStateCompat.State int state; if (playerStatus != null) { switch (playerStatus) { @@ -1126,7 +1114,9 @@ public class PlaybackService extends MediaBrowserServiceCompat { flavorHelper.mediaSessionSetExtraForWear(mediaSession); - mediaSession.setPlaybackState(sessionState.build()); + final PlaybackStateCompat sessionStateBuilt = sessionState.build(); + mediaSession.setPlaybackState(sessionStateBuilt); + serviceManager.onPlaybackStateChange(sessionStateBuilt); } private static boolean useSkipToPreviousForRewindInLockscreen() { @@ -1203,13 +1193,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { */ private Thread notificationSetupThread; - /** - * Prepares notification and starts the service in the foreground. - */ - private void setupNotification(final PlaybackServiceMediaPlayer.PSMPInfo info) { - setupNotification(info.playable); - } - private synchronized void setupNotification(final Playable playable) { if (notificationSetupThread != null) { notificationSetupThread.interrupt(); @@ -1226,7 +1209,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { @Override public void run() { - Log.d(TAG, "Starting background work"); + Log.d(TAG, "notificationSetupTask: Starting background work"); if (mediaPlayer == null) { Log.d(TAG, "notificationSetupTask: mediaPlayer is null"); @@ -1256,6 +1239,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { } PlayerStatus playerStatus = mediaPlayer.getPlayerStatus(); + Log.v(TAG, "notificationSetupTask: playerStatus=" + playerStatus); if (!Thread.currentThread().isInterrupted() && started) { String contentText = playable.getEpisodeTitle(); @@ -1349,7 +1333,22 @@ public class PlaybackService extends MediaBrowserServiceCompat { playerStatus == PlayerStatus.PREPARING || playerStatus == PlayerStatus.SEEKING || isCasting) { + Log.v(TAG, "DBG - notificationSetupTask: make service foreground"); startForeground(NOTIFICATION_ID, notification); + } else if (playerStatus == PlayerStatus.PAUSED) { + // TODO: logic originally in mediaPlayerCallback case PAUSED. + if ((UserPreferences.isPersistNotify() || isCasting) && + android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) { + // do not remove notification on pause based on user pref and whether android version supports expanded notifications + // Change [Play] button to [Pause] + // TODO LATER: logic same as outer else clause: setupNotification(info) + stopForeground(false); + NotificationManager mNotificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); + mNotificationManager.notify(NOTIFICATION_ID, notification); + } else if (!UserPreferences.isPersistNotify() && !isCasting) { + // remove notification on pause + stopForeground(true); + } } else { stopForeground(false); NotificationManager mNotificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); @@ -1846,12 +1845,10 @@ public class PlaybackService extends MediaBrowserServiceCompat { void setIsCasting(boolean isCasting); - void sendNotificationBroadcast(int type, int code); + void sendNotificationBroadcast(int type, int code); // TODO: unclear if the broadcast is still needed with ServiceManager void saveCurrentPosition(boolean fromMediaPlayer, Playable playable, int position); - void setupNotification(boolean connected, PlaybackServiceMediaPlayer.PSMPInfo info); - MediaSessionCompat getMediaSession(); Intent registerReceiver(BroadcastReceiver receiver, IntentFilter filter); @@ -1891,24 +1888,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { } @Override - public void setupNotification(boolean connected, PlaybackServiceMediaPlayer.PSMPInfo info) { - if (connected) { - PlaybackService.this.setupNotification(info); - } else { - PlayerStatus status = info.playerStatus; - if ((status == PlayerStatus.PLAYING || - status == PlayerStatus.SEEKING || - status == PlayerStatus.PREPARING || - UserPreferences.isPersistNotify()) && - android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) { - PlaybackService.this.setupNotification(info); - } else if (!UserPreferences.isPersistNotify()) { - PlaybackService.this.stopForeground(true); - } - } - } - - @Override public MediaSessionCompat getMediaSession() { return PlaybackService.this.mediaSession; } @@ -1923,4 +1902,80 @@ public class PlaybackService extends MediaBrowserServiceCompat { PlaybackService.this.unregisterReceiver(receiver); } }; + + /** + * The helper that manages PlaybackService's foreground service life cycle and the associated + * notification control. + * + * The logic is adapted from a sample app from The Android Open Source Project. + * See https://github.com/googlesamples/android-MediaBrowserService/blob/6cf01be9ef82ca2dd653f03e2a4af0b075cc0021/Application/src/main/java/com/example/android/mediasession/service/MusicService.java#L211 + * + */ + private class ServiceManager { + private boolean mServiceInStartedState; // TODO: rationalize it with parent's static started + // TODO LATER: rename the variable, do not use m prefix + + /** + * + * Entry point method for callers. Upon PlaybackState changes, + * the manager start/stop the PlaybackService as well as relevant notification + */ + void onPlaybackStateChange(PlaybackStateCompat state) { + // Report the state to the MediaSession. + + Log.v(TAG, "DBG - onPlaybackStateChange(" + state + ")"); + // Manage the started state of this service. + switch (state.getState()) { + case PlaybackStateCompat.STATE_PLAYING: + moveServiceToStartedState(state); + break; + case PlaybackStateCompat.STATE_PAUSED: + updateNotificationForPause(state); + break; + case PlaybackStateCompat.STATE_STOPPED: + moveServiceOutOfStartedState(state); + break; + case PlaybackStateCompat.STATE_ERROR: + moveServiceOutOfStartedState(state); + break; + } + } + + private void moveServiceToStartedState(PlaybackStateCompat state) { + Log.v(TAG, "DBG - ServiceManager.moveServiceToStartedState() - mServiceInStartedState:" + mServiceInStartedState); + + if (!mServiceInStartedState) { + ContextCompat.startForegroundService( + PlaybackService.this, + new Intent(PlaybackService.this, PlaybackService.class)); + mServiceInStartedState = true; + } + + doSetupNotification(); + } + + private void updateNotificationForPause(PlaybackStateCompat state) { + doSetupNotification(); + } + + private void moveServiceOutOfStartedState(PlaybackStateCompat state) { + stopForeground(true); + stopSelf(); + mServiceInStartedState = false; + } + + private void doSetupNotification() { + if (mediaPlayer != null && mediaPlayer.getPlayable() != null) { + // it updates notification and set foreground status + // based on player status (similar to PlaybackState) + setupNotification(mediaPlayer.getPlayable()); + } else { + // should not happen unless there are bugs. + Log.e(TAG, "doSetupNotification() - unexpectedly there is no playable. No notification setup done. mediaPlayer." + mediaPlayer); + } + } + } + + private final ServiceManager serviceManager = new ServiceManager(); + } diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackController.java b/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackController.java index 01d6812fd..81110da7b 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackController.java +++ b/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackController.java @@ -13,7 +13,6 @@ import android.os.Build; import android.os.IBinder; import android.support.annotation.NonNull; import android.support.annotation.Nullable; -import android.support.v4.content.ContextCompat; import android.text.TextUtils; import android.util.Log; import android.util.Pair; @@ -105,6 +104,7 @@ public abstract class PlaybackController { } private synchronized void initServiceRunning() { + Log.v(TAG, "initServiceRunning()"); if (initialized) { return; } @@ -192,7 +192,7 @@ public abstract class PlaybackController { if (!PlaybackService.started) { if (intent != null) { Log.d(TAG, "Calling start service"); - ContextCompat.startForegroundService(activity, intent); + // ContextCompat.startForegroundService(activity, intent); // TODO: [2716] likely not needed but is not 100% sure bound = activity.bindService(intent, mConnection, 0); } else { status = PlayerStatus.STOPPED; @@ -784,6 +784,7 @@ public abstract class PlaybackController { } private void initServiceNotRunning() { + Log.v(TAG, "DBG - initServiceNotRunning()"); // TODO: review it it's still needed mediaLoader = Maybe.create((MaybeOnSubscribe<Playable>) emitter -> { Playable media = getMedia(); if (media != null) { diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackServiceStarter.java b/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackServiceStarter.java index f7d2ee409..c42b6dd49 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackServiceStarter.java +++ b/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackServiceStarter.java @@ -2,9 +2,6 @@ package de.danoeh.antennapod.core.util.playback; import android.content.Context; import android.content.Intent; -import android.media.MediaPlayer; -import android.support.annotation.NonNull; -import android.support.v4.content.ContextCompat; import de.danoeh.antennapod.core.preferences.PlaybackPreferences; import de.danoeh.antennapod.core.service.playback.PlaybackService; @@ -73,6 +70,6 @@ public class PlaybackServiceStarter { if (PlaybackService.isRunning && !callEvenIfRunning) { return; } - ContextCompat.startForegroundService(context, getIntent()); + context.startService(getIntent()); // the service itself will decide if it needs to become foreground } } diff --git a/core/src/play/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceFlavorHelper.java b/core/src/play/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceFlavorHelper.java index 0cca2ffa4..a6b732a4f 100644 --- a/core/src/play/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceFlavorHelper.java +++ b/core/src/play/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceFlavorHelper.java @@ -151,7 +151,6 @@ public class PlaybackServiceFlavorHelper { // hardware volume buttons control the local device volume mediaRouter.setMediaSessionCompat(null); unregisterWifiBroadcastReceiver(); - callback.setupNotification(false, info); } }; } @@ -181,7 +180,6 @@ public class PlaybackServiceFlavorHelper { // hardware volume buttons control the remote device volume mediaRouter.setMediaSessionCompat(callback.getMediaSession()); registerWifiBroadcastReceiver(); - callback.setupNotification(true, info); } private void switchMediaPlayer(@NonNull PlaybackServiceMediaPlayer newPlayer, |