-
-
Notifications
You must be signed in to change notification settings - Fork 483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve Android Service lifecycle/notification management. #849
base: major
Are you sure you want to change the base?
Changes from all commits
e6ce459
6e49376
3c82655
c845c48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -221,6 +221,7 @@ private static int calculateInSampleSize(BitmapFactory.Options options, int reqW | |
private boolean notificationCreated; | ||
private final Handler handler = new Handler(Looper.getMainLooper()); | ||
private VolumeProviderCompat volumeProvider; | ||
private boolean isInForeground; | ||
|
||
public AudioProcessingState getProcessingState() { | ||
return processingState; | ||
|
@@ -404,9 +405,9 @@ else if (errorMessage != null) | |
mediaSession.setShuffleMode(shuffleMode); | ||
mediaSession.setCaptioningEnabled(captioningEnabled); | ||
|
||
if (!wasPlaying && playing) { | ||
if (!isInForeground && isActuallyPlaying()) { | ||
enterPlayingState(); | ||
} else if (wasPlaying && !playing) { | ||
} else if (isInForeground && !playing) { | ||
exitPlayingState(); | ||
} | ||
|
||
|
@@ -489,11 +490,12 @@ private Notification buildNotification() { | |
final MediaStyle style = new MediaStyle() | ||
.setMediaSession(mediaSession.getSessionToken()) | ||
.setShowActionsInCompactView(compactActionIndices); | ||
if (config.androidNotificationOngoing) { | ||
style.setShowCancelButton(true); | ||
style.setCancelButtonIntent(buildMediaButtonPendingIntent(PlaybackStateCompat.ACTION_STOP)); | ||
builder.setOngoing(true); | ||
} | ||
boolean ongoing = (config.androidNotificationOngoing != null) | ||
? config.androidNotificationOngoing | ||
: isActuallyPlaying(); | ||
style.setShowCancelButton(!ongoing); | ||
style.setCancelButtonIntent(buildMediaButtonPendingIntent(PlaybackStateCompat.ACTION_STOP)); | ||
builder.setOngoing(ongoing); | ||
builder.setStyle(style); | ||
return builder.build(); | ||
} | ||
|
@@ -548,14 +550,22 @@ private void updateNotification() { | |
} | ||
} | ||
|
||
private boolean isActuallyPlaying() { | ||
return playing && (processingState == AudioProcessingState.loading | ||
|| processingState == AudioProcessingState.buffering | ||
|| processingState == AudioProcessingState.ready); | ||
} | ||
|
||
private void enterPlayingState() { | ||
ContextCompat.startForegroundService(this, new Intent(AudioService.this, AudioService.class)); | ||
if (!mediaSession.isActive()) | ||
mediaSession.setActive(true); | ||
|
||
acquireWakeLock(); | ||
mediaSession.setSessionActivity(contentIntent); | ||
internalStartForeground(); | ||
startForeground(NOTIFICATION_ID, buildNotification()); | ||
notificationCreated = true; | ||
isInForeground = true; | ||
} | ||
|
||
private void exitPlayingState() { | ||
|
@@ -567,11 +577,7 @@ private void exitPlayingState() { | |
private void exitForegroundState() { | ||
stopForeground(false); | ||
releaseWakeLock(); | ||
} | ||
|
||
private void internalStartForeground() { | ||
startForeground(NOTIFICATION_ID, buildNotification()); | ||
notificationCreated = true; | ||
isInForeground = false; | ||
} | ||
|
||
private void acquireWakeLock() { | ||
|
@@ -595,6 +601,7 @@ private void deactivateMediaSession() { | |
} | ||
// Force cancellation of the notification | ||
getNotificationManager().cancel(NOTIFICATION_ID); | ||
notificationCreated = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of this change notification won't update when go There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes I was wondering whether this would break anything, although the logic seemed right. That said, when I planned out this change, I originally wanted to get rid of |
||
} | ||
|
||
private void releaseMediaSession() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes
playing=true
andidle
thing. It's also looks ok to exit foreground state when it'scompleted
orerror
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But other problem is if you try to stop the
idle
service - nothing happens, I'll try to think about this...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment in #847
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above is only used when entering foreground. When exiting foreground, it uses a broader condition (simply
!playing
).