Skip to content
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.

Sign up for GitHub

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

feat: notification config builder #914

Open
wants to merge 3 commits into
base: minor
Choose a base branch
from
Open

feat: notification config builder #914

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
240e9ee
feat: notification config builder
Zekfad Jan 5, 2023
d353843
fix: make snapshot immutable
Zekfad Jan 13, 2023
6f645bf
fix: make PlayerStateSnapshot constructor private
Zekfad Jan 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 79 additions & 20 deletions just_audio_background/lib/just_audio_background.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export 'package:audio_service/audio_service.dart' show MediaItem;

late SwitchAudioHandler _audioHandler;
late JustAudioPlatform _platform;
late NotificationConfigBuilder _notificationConfigBuilder;

/// Provides the [init] method to initialise just_audio for background playback.
class JustAudioBackground {
Expand Down Expand Up @@ -48,6 +49,7 @@ class JustAudioBackground {
Duration rewindInterval = const Duration(seconds: 10),
bool preloadArtwork = false,
Map<String, dynamic>? androidBrowsableRootExtras,
NotificationConfigBuilder? notificationConfigBuilder,
}) async {
WidgetsFlutterBinding.ensureInitialized();
await _JustAudioBackgroundPlugin.setup(
Expand All @@ -69,6 +71,7 @@ class JustAudioBackground {
rewindInterval: rewindInterval,
preloadArtwork: preloadArtwork,
androidBrowsableRootExtras: androidBrowsableRootExtras,
notificationConfigBuilder: notificationConfigBuilder,
);
}
}
Expand All @@ -91,7 +94,10 @@ class _JustAudioBackgroundPlugin extends JustAudioPlatform {
Duration rewindInterval = const Duration(seconds: 10),
bool preloadArtwork = false,
Map<String, dynamic>? androidBrowsableRootExtras,
NotificationConfigBuilder? notificationConfigBuilder,
}) async {
_notificationConfigBuilder = notificationConfigBuilder
?? _PlayerAudioHandler._defaultNotificationConfigBuilder;
_platform = JustAudioPlatform.instance;
JustAudioPlatform.instance = _JustAudioBackgroundPlugin();
_audioHandler = await AudioService.init(
Expand Down Expand Up @@ -165,7 +171,7 @@ class _JustAudioPlayer extends AudioPlayerPlatform {
late final _PlayerAudioHandler _playerAudioHandler;

_JustAudioPlayer({required String id}) : super(id) {
_playerAudioHandler = _PlayerAudioHandler(id);
_playerAudioHandler = _PlayerAudioHandler._(id);
Zekfad marked this conversation as resolved.
Show resolved Hide resolved
_audioHandler.inner = _playerAudioHandler;
_audioHandler.playbackState.listen((playbackState) {
broadcastPlaybackEvent();
Expand Down Expand Up @@ -310,7 +316,7 @@ class _JustAudioPlayer extends AudioPlayerPlatform {
}

class _PlayerAudioHandler extends BaseAudioHandler
with QueueHandler, SeekHandler {
with QueueHandler, SeekHandler, PlayerStateSnapshot {
final _playerCompleter = Completer<AudioPlayerPlatform>();
PlaybackEventMessage _justAudioEvent = PlaybackEventMessage(
processingState: ProcessingStateMessage.idle,
Expand Down Expand Up @@ -344,10 +350,32 @@ class _PlayerAudioHandler extends BaseAudioHandler

List<MediaItem>? get currentQueue => queue.nvalue;

_PlayerAudioHandler(String playerId) {
_PlayerAudioHandler._(String playerId) {
_init(playerId);
}

/// Default notification builder
/// Showing skip, play/pause and stop buttons if applicable.
/// Stop button is hidden from android compact view.
static NotificationConfig _defaultNotificationConfigBuilder(PlayerStateSnapshot state) {
final controls = [
if (state.hasPrevious) MediaControl.skipToPrevious,
if (state.playing) MediaControl.pause else MediaControl.play,
MediaControl.stop,
if (state.hasNext) MediaControl.skipToNext,
];
return NotificationConfig(
controls: controls,
systemActions: const {
MediaAction.seek,
MediaAction.seekForward,
MediaAction.seekBackward,
},
androidCompactActionIndices: List.generate(controls.length, (i) => i)
..removeAt(controls.indexOf(MediaControl.stop))
);
}

Future<void> _init(String playerId) async {
final player = await _platform.init(InitRequest(id: playerId));
_playerCompleter.complete(player);
Expand Down Expand Up @@ -506,10 +534,13 @@ class _PlayerAudioHandler extends BaseAudioHandler
List<int> get effectiveIndices => _effectiveIndices;
List<int> get shuffleIndicesInv => _shuffleIndicesInv;
List<int> get effectiveIndicesInv => _effectiveIndicesInv;

@override
int get nextIndex => getRelativeIndex(1);
@override
int get previousIndex => getRelativeIndex(-1);
bool get hasNext => nextIndex != -1;
bool get hasPrevious => previousIndex != -1;
@override
bool get playing => _playing;

int getRelativeIndex(int offset) {
if (_repeatMode == AudioServiceRepeatMode.one) return index!;
Expand Down Expand Up @@ -676,22 +707,11 @@ class _PlayerAudioHandler extends BaseAudioHandler

/// Broadcasts the current state to all clients.
void _broadcastState() {
final controls = [
if (hasPrevious) MediaControl.skipToPrevious,
if (_playing) MediaControl.pause else MediaControl.play,
MediaControl.stop,
if (hasNext) MediaControl.skipToNext,
];
final notificationConfig = _notificationConfigBuilder(this);
playbackState.add(playbackState.nvalue!.copyWith(
controls: controls,
systemActions: {
MediaAction.seek,
MediaAction.seekForward,
MediaAction.seekBackward,
},
androidCompactActionIndices: List.generate(controls.length, (i) => i)
.where((i) => controls[i].action != MediaAction.stop)
.toList(),
controls: notificationConfig.controls,
systemActions: notificationConfig.systemActions,
androidCompactActionIndices: notificationConfig.androidCompactActionIndices,
processingState: const {
ProcessingStateMessage.idle: AudioProcessingState.idle,
ProcessingStateMessage.loading: AudioProcessingState.loading,
Expand Down Expand Up @@ -839,3 +859,42 @@ extension _ValueStreamExtension<T> on ValueStream<T> {
/// Backwards compatible version of valueOrNull.
T? get nvalue => hasValue ? value : null;
}

/// Notification config, used to set system notification properties.
@immutable
class NotificationConfig {
const NotificationConfig({
this.controls = const [],
this.systemActions = const {},
this.androidCompactActionIndices,
});

/// Not supported starting with Android 13 (API level 33)
///
/// More info: https://developer.android.com/about/versions/13/behavior-changes-13#playback-controls
final List<int>? androidCompactActionIndices;
final List<MediaControl> controls;
final Set<MediaAction> systemActions;
}


/// Snapshot of current player state.
/// Used to provide data to [NotificationConfigBuilder].
abstract class PlayerStateSnapshot {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunate to replicate existing state classes that are in just_audio, but just_audio_background should not depend on just_audio so this is understandable for now.

Eventually, I think these state classes should be defined in just_audio_platform_interface where they can be reused.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, is this in scope of this PR, if so, tell me where I should move it, please?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That comment was just to recognise the need for something now even though it should be moved elsewhere in the future.

But as it is, is this really a "snapshot"? It looks like you're using this as a mixin (although you defined it as an abstract class instead of a mixin). That means that this package is now returning a reference to an internal class which it probably shouldn't. If the so-called snapshot is actually an object where the values may change behind the scenes, then it is not really a snapshot. I think it would be far less surprising to create an immutable state object, and to build a new instance of it each time a new state needs to be broadcast.

As for naming, and considering where these state objects will likely move in the future, I would say we do not know at this stage what that future state object will look like, and anything we might come up with now would be rather narrow in its vision. So to avoid any future problems, it might be best to choose a class name for this state that is obviously specific to this particular package rather than some name that we might want to reserve for the future ideal API.

/// -1 if no next index available
int get nextIndex;
/// -1 if no previous index available
int get previousIndex;
/// Whether player is currently in playing state.
bool get playing;
/// Whether player is currently paused.
bool get paused => !playing;
Zekfad marked this conversation as resolved.
Show resolved Hide resolved
/// Whether playback queue has next entry.
bool get hasNext => nextIndex != -1;
/// Whether playback queue has previous entry.
bool get hasPrevious => previousIndex != -1;
}

/// Notification config builder.
/// Used to control available actions in system notification.
typedef NotificationConfigBuilder = NotificationConfig Function(PlayerStateSnapshot state);