Skip to content

Fixed resetting finished audiobooks #355

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

Open
MitjaHenner wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixed resetting finished audiobooks #355

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea5986a
Fixed resetting finished audiobooks
MitjaHenner Feb 28, 2026
eb9a6ff
Moved check inside try/catch and added constants for first chapter an…
MitjaHenner Mar 1, 2026
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
7 changes: 7 additions & 0 deletions app/src/main/kotlin/org/grakovne/lissen/playback/service/PlaybackSynchronizationService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ class PlaybackSynchronizationService
try {
val currentIndex = calculateChapterIndex(currentItem, overallProgress.currentTotalTime)

if (currentIndex > FIRST_CHAPTER && overallProgress.currentTotalTime == NO_PROGRESS) {
Timber.d("Don't sync currentTotalTime of 0.0 if chapter is not first")
return@launch
}

if (playbackSession == null ||
playbackSession?.itemId != currentItem.id ||
currentIndex != currentChapterIndex ||
Expand Down Expand Up @@ -183,6 +188,8 @@ class PlaybackSynchronizationService
private const val SHORT_SYNC_WINDOW = SYNC_INTERVAL_LONG * 2 - 1

private const val SYNC_INTERVAL_SHORT = 5_000L
private const val FIRST_CHAPTER = 0
Copy link
Copy Markdown
Owner

@GrakovNe GrakovNe Mar 1, 2026

Choose a reason for hiding this comment

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

I’m still a bit concerned that this looks like “magic logic”.

Why is this the correct rule:

don’t sync currentTotalTime == 0.0 unless it’s the first chapter

gated by currentIndex > FIRST_CHAPTER

What is the empirical basis for this behavior? Was it derived from a specific reproducible bug, or from observed incorrect sync scenarios?

At the moment, it feels like a narrowly scoped workaround that solves a concrete issue, but also hard-codes an implicit assumption into the sync layer: that 0.0 is invalid progress except for chapter 0. That assumption is not self-evident and may be fragile long term (especially considering chapter/file mapping, explicit progress reset, cross-device sync races, etc.).

I would be more comfortable if we could clearly articulate the invariant we’re trying to enforce here, rather than encoding it indirectly through positional checks.

Copy link
Copy Markdown
Author

@MitjaHenner MitjaHenner Mar 2, 2026

Choose a reason for hiding this comment

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

Yea it would be cleaner to not trigger this sync at all.

Right now the problem is:
Last chapter of an audiobook is getting played and the audiobook goes to finished once there are only a few seconds left in the audiobook.

Once the last chapter ended a few sync calls get triggered where the chapterIndex is still the last chapter but currentTotalTime is 0.0 this sync resets the progress on the server.

Changing it to isLastChapter && 0.0 -> return would do the same but still cause problems with single chapter audiobooks I guess.

private const val NO_PROGRESS = 0.0

private val syncEvents =
listOf(
Expand Down