feat(audible): bidirectional Whispersync progress sync#3893
Draft
scootaash wants to merge 18 commits into
Draft
Conversation
Adds sync_progress_from_audible() to AudibleHelper which batch-reads last-heard positions from GET /1.0/annotations/lastpositions and pushes them into MA's internal progress database via mark_item_played(). This closes the read direction gap confirmed in live testing: MA now resumes at the correct position even when the previous session happened in the Audible phone app or another device. Called from handle_async_init() as a non-blocking background task so provider initialisation is not delayed. Positions are processed in chunks of 50 ASINs to keep query strings within safe limits.
…SspT4 feat(audible): sync Audible positions into MA on startup (read direction fix)
Audible enforces a 500-char limit on the comma-joined asins query parameter value. The previous chunk_size=50 produced ~550 chars per chunk (50 ASINs × ~11 chars each), causing a 400 Bad Request on every request in a 272-book library. Replace count-based chunking with length-based chunking that accumulates ASINs until the next one would exceed 480 chars (20 char headroom), then flushes the batch. Also downgrades the per-chunk log from ERROR to WARNING since a chunk failure is recoverable — other chunks still run.
…ble lastpositions batch calls; fix literal backslash-quotes syntax error in get_audiobooks_by_narrator
…kslash-quote syntax error in get_audiobooks_by_narrator
…und progress sync via TaskSchedule
… class tuple[bool, int, datetime | None]
…t_updated timestamp in get_resume_position; raise NotImplementedError on transport failure; drop dead try/except in unload
…rd; pass user_initiated=False; drop unused UTC import
- guard sync_progress_from_audible against re-entry (cold-start vs scheduled first-run race when persisted last_run is in the past) - pass user_initiated=False to mark_item_played so background sync doesn't pose as a user action in the playlog - narrow _sync_progress_chunk's broad Exception to the same set used by set_last_position so real bugs surface - rename get_resume_position's parameter to item_id to match the base MusicProvider signature and every other provider override - drop unused UTC import from __init__.py - extract _parse_audible_timestamp to handle ISO-8601, numeric-string, and epoch (seconds and ms) timestamp variants observed across marketplaces - add :param: docstrings to get_resume_position and get_audible_resume_position
Combines the in-flight feasibility-branch fix (00b8de4: schedule interval hourly(every=1), concurrent-run guard, user_initiated=False, drop unused UTC import) with the audit corrections branch (3f711f4), which additionally narrows _sync_progress_chunk's exception handler, renames get_resume_position's parameter to item_id to match the base MusicProvress signature and every other provider override, extracts _parse_audible_timestamp to handle ISO-8601, numeric-string, and epoch (seconds and milliseconds) timestamp variants, and adds :param: docstrings. Re-entrancy guard wording resolved to the more explanatory variant from the audit branch.
…vention, multi-user scoping - Replace _syncing_from_audible boolean in on_played() with per-ASIN _sync_suppressed_asins set. on_played() discards the ASIN on first hit and returns; finally block in sync_progress_from_audible() clears any remainder. Eliminates the asyncio.sleep(0) race and makes suppression per-item rather than global. - In _sync_progress_chunk(), parse Audible's last_updated timestamp and compare it against the existing MA playlog row before calling mark_item_played(). Skips the write when MA already has an equal or newer timestamp, preventing stale Audible positions from overwriting more recent in-session progress. - Resolve the MA user that owns this provider instance via _get_user_for_provider() and thread userid through to mark_item_played() and the DB lookup, so background syncs write only to the correct user's playlog instead of fanning out to all users on a multi-user server. - Add DB_TABLE_PLAYLOG import from music_assistant.constants (needed for direct playlog timestamp comparison).
…SspT4 Audible bidirectional progress sync (Whispersync read + write)
- Add missing await to _get_user_for_provider call and extract .user_id from returned User object (was returning a coroutine, silently crashing every sync run and writing nothing) - Raise NotImplementedError in get_audible_resume_position when position_ms == 0 so MA uses its own playlog rather than clobbering valid in-session position with a zero from Audible - Remove bulk _sync_suppressed_asins.clear() from finally block; individual discard in on_played() handles per-ASIN cleanup when each create_task-scheduled callback fires These fixes were pushed to claude/audible-sync-feasibility-SspT4 after PR #3 was already merged, so did not land in dev.
…zone handling - Stale-overwrite check was dead code: get_row used provider_instance as the provider key but mark_item_played writes rows with media_item.provider which resolves to 'library' for library items. Changed to mass_audiobook.provider to match the actual row key. Without this fix the timestamp comparison was always skipped and every hourly sync overwrote MA's playlog unconditionally. - Coerce timezone-naive Audible timestamps to UTC before calling .timestamp() in both _sync_progress_chunk and get_audible_resume_position. Audible uses 'Z' today but this prevents incorrect comparisons on non-UTC servers if that changes. - Add comment on _sync_suppressed_asins.add explaining the narrow user-playback race and why it is acceptable. - Add comment on get_audible_resume_position explaining why fully_played is always False (lastpositions payload does not carry finished-state).
…rovider call - Move _syncing_from_audible = True inside the try block so any exception during sync setup cannot permanently disable future runs by leaving the flag stuck True. - Remove _get_user_for_provider call entirely. mark_item_played already resolves the user via its own internal fallback when userid=None is passed, so the audible provider calling the private API added nothing. Also avoids the accidental string-iteration behaviour identified in audit (passing a bare instance_id str iterates its characters).
…ently played When the MA web UI is opened via Home Assistant ingress, the WebSocket session authenticates as the dedicated `homeassistant_system` service user. Playback initiated through HA-owned players therefore scrobbles under that shared system identity, not the viewing human user. The playlog unique key `(item_id, provider, media_type, userid)` pins the row to `homeassistant_system`, so `in_progress_items()` and `recently_played()` — which scoped strictly to the current user — never surface those rows for the human viewer. Treat `homeassistant_system` as a shared household identity: OR it into the userid filter alongside the viewing user. No schema changes, no attribution refactor; the per-user scoping for any non-system user remains intact.
…elf-8R6P4 fix(shelves): include HA system-user rows in Continue Listening / Recently played
Contributor
Contributor
|
Why are there changes to the music controller in this PR? https://github.com/music-assistant/.github/blob/main/AI_POLICY.md |
OzGav
requested changes
May 17, 2026
|
|
||
| try: | ||
| response = await self._call_api("annotations/lastpositions", asins=asin) | ||
| except Exception as exc: |
Contributor
There was a problem hiding this comment.
Broad exception handling should be avoided and instead more narrow exceptions should be used for the errors you'd like to catch. This occurs in various places in this PR
There was a problem hiding this comment.
@OzGav I think this could be automated with ruff/lint checks? :)
Either one of these? 🤔
https://docs.astral.sh/ruff/rules/bare-except/
https://docs.astral.sh/ruff/rules/blind-except/
Contributor
There was a problem hiding this comment.
Thanks I'll pass the suggestion onto the lint gurus!
| # correctly. The field is "last_updated" in the Audible annotation payload; | ||
| # additional names are tried as a fallback against future API variation. | ||
| timestamp: datetime | None = None | ||
| for field in ("last_updated", "reported_time", "last_updated_time", "timestamp"): |
Contributor
There was a problem hiding this comment.
This is duplicated in sync_progress_chunk. Please extract to a helper
| # mark_item_played writes with media_item.provider which resolves to | ||
| # "library" for library items — use mass_audiobook.provider to match. | ||
| if audible_ts is not None: | ||
| existing = await self.mass.music.database.get_row( |
Contributor
There was a problem hiding this comment.
A provider must not access the database directly mass.music.database is the music controller's internal concern. The controller already handles playlog timestamp comparison in get_resume_position
| in progress) are skipped silently. | ||
| """ | ||
| # Prevent concurrent runs (cold-start task + first scheduled fire can overlap). | ||
| if self._syncing_from_audible: |
Contributor
There was a problem hiding this comment.
Consider asyncio.Lock instead
| # calling a private API and is equivalent for all supported setups. | ||
| userid: str | None = None | ||
| asins: list[str] = [] | ||
| async for item in self._fetch_library_items( |
Contributor
There was a problem hiding this comment.
This looks like you are fetching every book just to get a list of ASINs? You can get this from mass.music.audiobooks.library_items() filtered by this provider instance
| except Exception as exc: | ||
| self.logger.error(f"Unexpected error reporting position for ASIN {asin}: {exc}") | ||
|
|
||
| async def get_audible_resume_position( |
Contributor
There was a problem hiding this comment.
Why was the music controller's get_last_postion not extended or replaced instead of adding a new method?
| media_item is the full media item details of the played/playing track. | ||
| """ | ||
| if prov_item_id in self.helper._sync_suppressed_asins: | ||
| self.helper._sync_suppressed_asins.discard(prov_item_id) |
Contributor
There was a problem hiding this comment.
You are accessing a private attribute here?
| async def get_resume_position( | ||
| self, prov_item_id: str, media_type: MediaType | ||
| ) -> tuple[bool, int, datetime | None]: | ||
| """Return resume position from Audible for the given item. |
Contributor
There was a problem hiding this comment.
Suggested change
| """Return resume position from Audible for the given item. | |
| """ | |
| Return resume position from Audible for the given item. |
For standardisation please ensure all multi line docstrings start on their own line
Comment on lines
+603
to
+607
| Called by MA just before queuing an audiobook for playback. | ||
| MA compares the returned timestamp against its internal playlog and | ||
| uses whichever source is more recent. Raises NotImplementedError on | ||
| transport failure so MA falls back to its own playlog rather than | ||
| presenting a stale or missing position as authoritative. |
Contributor
There was a problem hiding this comment.
Dosctrings may not contain implementation detail
Contributor
|
Also lint is failing. Marking as draft so we can keep track of what needs attention. Please mark as ready for review again when you want another look. |
Author
|
Many thanks for the detailed comments. Will take a look. Cheers |
Contributor
|
Marking this PR as draft so we can keep track of which PRs needs our attention. Please mark as 'Ready for review' when you want us to have another look 🙏 . |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements full bidirectional progress sync between Music Assistant and the
Audible app via Whispersync. My first PR so feedback appreciated
Before: MA always resumed from 0:00 after listening on the Audible phone app
or any other Whispersync-connected device. Write-back (MA → Audible) existed but
was unreliable due to ACR cache lifecycle and a feedback loop that fired redundant
PUT calls every 30 s.
After: Position is shared correctly in both directions.
Read direction (Audible app → MA) — was completely missing
sync_progress_from_audible()batch-fetches last-heard positions viaGET /1.0/annotations/lastpositionsfor all library audiobooks and writesthem into MA's internal playlog via
mark_item_played().TaskSchedule.hourly(every=1).handle_async_init/unload.get_resume_position()does a live single-ASIN fetch immediately beforeplayback so MA always starts from the freshest position; raises
NotImplementedErroron failure so MA falls back to its own playlog.Write direction (MA → Audible) — reliability fixes
_sync_progress_chunk: parses Audible'slast_updatedand compares against MA's playlog row before writing —skips the write when MA is already as current or newer.
_sync_suppressed_asins): prevents themark_item_played→on_played→set_last_positionfeedback loopwithout a global flag or
asyncio.sleep(0)race.(
_MAX_ASINS_PARAM_LEN = 480) and the 25-ASIN per-request limit(
_MAX_ASINS_PER_REQUEST = 25).get_audible_resume_position()parses Audible's actuallast_updatedtimestamp so MA's playlog tiebreaker picks the fresher source correctly.
Raises
NotImplementedErroron transport failure for graceful fallback.Notes
_update_playlog_book/on_playedapproach.fully_playedis alwaysFalsein sync — thelastpositionsendpointdoes not expose finished-state.
_get_user_for_provideris not called;userid=Noneletsmark_item_playedresolve the user via its own existing fallback.
Test plan [all completed by scootaash]