Jump to next / previous chapter

[lks-nbg]

I worked on the second part of issue #6 and tried to implement jumping to next and previous chapters from the player and the notification. Jumping to the next or previous chapter basically works but I'm struggling on how to update currentlyPlaying information. Therefore the actions currently only work once, since the progress is not updated :-/

Furthermore I had to adjust the player ui to accommodate two more buttons. I added another row which has a lot of free space at the moment, but it can be used for new features like bookmarks (#11 ).

I would really appreciate your review and hope to get some tips to finish this pr.

Resolves #6

[mattttvaughn]

I dig the updated design!

I don't have a fully-formed idea for how to do all of this, but take this with a grain of salt, but here are some ideas which might help:

For your chapterIndex issue where Plex is reporting 1-based indexing, you can use currentBook.chapters.indexOf(currentChapter) to get a consistent 0-based index.

As far as updating CurrentlyPlaying goes, I'm thinking you should be able to do this via a manual call to ProgressUpdater.updateProgress wherever needed since that will update currentlyPlaying, but I haven't thought through the full details. You could even use a convenience like below for updateProgress() so you don't have to provide all of those parameters everywhere you call it

    override fun updateProgress() {
        val controller = mediaController ?: return
        val playbackState = when (controller.playbackState.state) {
            PlaybackStateCompat.STATE_PLAYING -> MediaPlayerService.PLEX_STATE_PLAYING
            PlaybackStateCompat.STATE_PAUSED -> MediaPlayerService.PLEX_STATE_PAUSED
            PlaybackStateCompat.STATE_STOPPED -> MediaPlayerService.PLEX_STATE_PAUSED
            else -> ""
        }
        val currentTrack = controller.metadata.id?.toInt() ?: return
        val currentTrackProgress = controller.playbackState.currentPlayBackPosition
        updateProgress(
           currentTrack,
            playbackState,
            currentTrackProgress,
            false
        )
    }

[lks-nbg]

I fixed the index and moved most of the skipToNext & skipToPrevious code to PlayerExt.kt. Your updateProgress idea is working and you can now skip between chapters as expected.

There is one issue left with the notification. After skipping to the another chapter (in player view or notification) the notification get's updated but shows the previous chapter title until it's rebuild again.

Resolves #6

[mattttvaughn]

and @lks-nbg on the notification issue. I'd think calling NotificationBuilder would be the best way to go about this

Providing the following to the skipTo functions should do it:

    notificationBuilder: NotificationBuilder,
    controller: MediaControllerCompat,
    serviceScope: CoroutineScope
) {

and then at the end of skipToPrev/Next()

        serviceScope.launch {
            notificationBuilder.buildNotification(controller.sessionToken)
        }

And then just providing via @Injects and parent functions where needed.

It's gonna be ugly, but we can eventually just build a Player wrapper at some point to replace PlayerExt

[lks-nbg]

Thanks for your comments. I tried to implement them:

• skipToNext + 300ms (like in AudiobookMediaSessionCallback)
• multi-track → I added your code but since I only have single-track audiobooks I cannot test it
• NotificationBuilder → not sure if I missed something, but my notifications are still one chapter behind

[mattttvaughn]

@lks-nbg Added a commit to fix that notification not building issue, that was my fault, the val notification = notificationBuilder.buildNotification(...) doesn't actually set the notification, it just creates it.

Give it a whirl and lmk if you run into any issues. Looking pretty good from my end

[lks-nbg]

I think I found a more elegant solution, where we can get rid of the ugly code in PlayerExt. The callback onChapterChange is already implemented but was not used so far.

What do you think about this?

[mattttvaughn]

Good idea, looks good to me.

At this point I'm happy to merge whenever you're happy with it, just lmk. Can put out an unofficial build and get some people to try it out against edge cases.

Also, maybe worth noting that it looks like there is an unrelated bug in OnMediaChangedCallback which is causing multi-track books not to update chapter names in the notification on chapter change, but it works on my end after fixing that. I'll push a separate commit to fix that after merging this.

[lks-nbg]

For now I am happy with the code and you can merge it. Getting some feedback is a good idea 👍