Skip to content

Adapt artist / audiobook controller for authors and narrators#3570

Open
fmunkes wants to merge 31 commits into
devfrom
audiobooks_extra_endpoints
Open

Adapt artist / audiobook controller for authors and narrators#3570
fmunkes wants to merge 31 commits into
devfrom
audiobooks_extra_endpoints

Conversation

@fmunkes
Copy link
Copy Markdown
Contributor

@fmunkes fmunkes commented Apr 4, 2026

Adds endpoints for authors/ narrators. Is that sql stuff actually safe?

Same question in reverse to #3569 can be asked here - would it make sense to make authors/ narrators media items, so things like pictures, or more metadata can be added. Also, the provider could be asked directly for their items, instead of searching the db.

I'm willing to do that, but what do you think?

update after discussion below

depends on music-assistant/models#209

@marcelveldt
Copy link
Copy Markdown
Member

Not sure how common it is that the authors/narrators have actual metadata ?
We could also say that these are mapped to the artist mediaitem (and we add a artist_type?)

I think it may make sense to quickly browse by narrator or author but not sure if we need to create a whole media type for them, I'm also wondering if we even have metadata. When I checked spotify etc. they did not provide any metadata for authors or narrators except the name

@fmunkes
Copy link
Copy Markdown
Contributor Author

fmunkes commented Apr 4, 2026

Not sure how common it is that the authors/narrators have actual metadata ? We could also say that these are mapped to > I think it may make sense to quickly browse by narrator or author but not sure if we need to create a whole media type for them, I'm also wondering if we even have metadata. When I checked spotify etc. they did not provide any metadata for authors or narrators except the name

I like the idea of an artist_type, and then enhancing the artist controller a bit. You are right, the data is usually name and maybe a picture - but since we then have a provider id, we can also ask the provider for the audiobooks belonging to the author/ narrator. Shall I go with the artist_type then?

I could also enhance the albums controller for series and introduce another album_type?

@marcelveldt
Copy link
Copy Markdown
Member

I'm not sure how it would work in practice to count authors and narrators as artists but I guess we can say the same for composers (classical music) so it's worth the investigation. would be nice if we can fix it.

As for series; can you enlighten me what those are ?
I'm not into podcasts that much so I need a bit of help here and there understanding how its structured and used in practice ?

@fmunkes
Copy link
Copy Markdown
Contributor Author

fmunkes commented Apr 4, 2026

Series would be for audiobooks, not podcast (but why not include it, although I don't know if that exists).

A book series would be something like Lord of the rings with multiple books. Attributes would be the series name, picture, and the overall progress. So it is similar to an album (or playlist?) but if I would hit play on a series, my expectation would be that it starts where you left off, (e.g. book no 2) and not with the first one.

I guess essentially you can map all of these types to existing MA types with a little bit of extra information. What I can't judge is if it is more future proof to just keep this extra branches on an artist or album in mind, or just separate them completely with their own controller. Or put a controller between base controller and implementation, where both types can inherit from. Maintenance of course being important too.

I don't know how well Spotify's integration of audiobooks is, but series is definitely something you find e.g. in audible.

Of course, we can also just add them via attributes, and search the db for them. I really don't know what's best - maybe I'm to influenced by abs...
I would add least add support for this to abs and the local filesystem.

@marcelveldt
Copy link
Copy Markdown
Member

For putting narrators, authors and composers into artists with a dedicated type I'd say yes.
But with a sidenote that we cant make this exactly foolproof as often all we got is a single string with the name, no id or anything.

For series I'm not sure. Definitely not Album. To me it's more a metadata field which we can use for extra filtering and such. I guess you can compare this to collections within movies

@fmunkes
Copy link
Copy Markdown
Contributor Author

fmunkes commented Apr 7, 2026

Ok, sounds good. I'll adapt both PRs, and mark them as draft for the time being. For series, what I really would like to have, is, that you can collapse them into a single visual item, instead of multiple Audiobooks. But that's a frontend thing I think, so I'll think about something there.

@fmunkes fmunkes marked this pull request as draft April 7, 2026 14:18
@fmunkes
Copy link
Copy Markdown
Contributor Author

fmunkes commented Apr 16, 2026

Ok, I think this is ready for a first review.

@fmunkes fmunkes changed the title Add endpoints for authors / narrators to audiobooks controller Adapt artist / audiobook controller for authors and narrators Apr 16, 2026
Comment thread music_assistant/models/music_provider.py Outdated
Comment thread music_assistant/models/music_provider.py Outdated
@MarvinSchenkel
Copy link
Copy Markdown
Contributor

Before doing an in-depth review, let's chat about the controller first. It feels a bit strange to have all this mixed logic in the ArtistController (eg Type = ARTIST or Type IN (Narrator , Author) ).

@marcelveldt would it be an idea to implement this as a child controller or generic?

@marcelveldt
Copy link
Copy Markdown
Member

A composer, author or writer is a form of Artist. Currently we only support singers but maybe we should broaden it. Can still belong in the Artist controller, just like we already have multiple Album types. It's just a type...

@fmunkes
Copy link
Copy Markdown
Contributor Author

fmunkes commented Apr 26, 2026

I renamed the default artist type to "singer", that should make it clearer then, that we are talking about music artists and so on.

@fmunkes fmunkes requested a review from MarvinSchenkel April 26, 2026 10:33
async def _remove_author_narrator_from_library(self, db_id: int, recursive: bool) -> None:
# recursively also remove author/ narrator audiobooks
for db_row in await self.mass.music.database.get_rows_from_query(
f"SELECT album_id FROM {DB_TABLE_AUDIOBOOK_ARTISTS} WHERE artist_id = :artist_id",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think album_id does not exist in the table?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah. Rename issue, I should have checked that more closely...

return []
prov = cast("MusicProvider", prov)
if ProviderFeature.NARRATOR_AUDIOBOOKS in prov.supported_features:
return await prov.get_author_audiobooks(item_id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be prov.get_narrator_audiobooks(item_id) ?

for provider_mapping in library_artist.provider_mappings:
if provider_mapping.provider_instance not in unique_providers:
continue
provider_audiobooks = await self.get_provider_author_audiobooks(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't this call depend on the artist_type? Ie get_provider_author_audiobooks or get_provider_narrator_audiobooks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the artist/audiobook controllers to treat audiobook authors and narrators as artist-like entities, adding new provider hooks, database schema support, and controller APIs so audiobook relationships can be queried similarly to music artists.

Changes:

  • Added provider-facing author/narrator audiobook APIs and stricter audiobook sync/type handling.
  • Added artist_type support plus a new audiobook_artists join table to model audiobook-to-author/narrator relationships.
  • Extended artist and audiobook controllers with author/narrator-aware library queries, removal logic, and audiobook lookup endpoints.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
music_assistant/models/music_provider.py Adds provider hooks for author/narrator audiobook retrieval and sync-time validation for audiobook contributor types.
music_assistant/helpers/compare.py Prevents artist matching across different artist_type values.
music_assistant/controllers/music.py Bumps DB schema, adds artist_type to artists, and creates the new audiobook_artists table.
music_assistant/controllers/media/audiobooks.py Stores string contributors separately from linked artist records and adds audiobook↔artist mapping writes.
music_assistant/controllers/media/artists.py Adds author/narrator-aware library filtering, removal paths, and audiobook lookup APIs.
music_assistant/constants.py Defines the DB_TABLE_AUDIOBOOK_ARTISTS table name constant.
Comments suppressed due to low confidence (2)

music_assistant/controllers/media/audiobooks.py:325

  • [CRITICAL] _update_library_item now strips Artist authors/narrators out of the JSON fields, but it never updates audiobook_artists, so overwriting an existing audiobook can drop all author/narrator information from the library item while leaving no replacement links. Update the join table here just like albums update album_artists.
        # only serialize str narrators/ authors to db
        _update_authors = [author for author in update.authors if isinstance(author, str)]
        _update_narrators = [narrator for narrator in update.narrators if isinstance(narrator, str)]
        await self.mass.music.database.update(
            self.db_table,
            {"item_id": db_id},
            {
                "name": name,
                "sort_name": sort_name,
                "version": update.version if overwrite else cur_item.version or update.version,
                "metadata": serialize_to_json(metadata),
                "external_ids": serialize_to_json(
                    update.external_ids if overwrite else cur_item.external_ids
                ),
                "publisher": cur_item.publisher or update.publisher,
                "authors": serialize_to_json(
                    _update_authors if overwrite else cur_item.authors or _update_authors
                ),
                "narrators": serialize_to_json(
                    _update_narrators if overwrite else cur_item.narrators or _update_narrators
                ),
                "duration": update.duration if overwrite else cur_item.duration or update.duration,
                "search_name": create_safe_string(name, True, True),
                "search_sort_name": create_safe_string(sort_name or "", True, True),
                "timestamp_added": int(update.date_added.timestamp())
                if update.date_added
                else UNSET,
            },
        )
        # update/set provider_mappings table
        provider_mappings = (
            update.provider_mappings
            if overwrite
            else {*update.provider_mappings, *cur_item.provider_mappings}
        )
        await self.set_provider_mappings(db_id, provider_mappings, overwrite)
        self.logger.debug("updated %s in database: (id %s)", update.name, db_id)
        await self._set_playlog(db_id, update)

music_assistant/controllers/media/artists.py:74

  • [PROBLEM] library_items() only applies album_artists_only for singers, but library_count() still filters every artist type through album_artists, so author/narrator result counts can disagree with the actual listing and break pagination. Guard this clause the same way as library_items().
        query_parts = [f"artist_type = '{artist_type}'"]
        if favorite_only:
            query_parts.append("favorite = 1")
        if album_artists_only:
            query_parts.append(
                f"item_id in (select {DB_TABLE_ALBUM_ARTISTS}.artist_id "
                f"FROM {DB_TABLE_ALBUM_ARTISTS})"
            )

Comment thread music_assistant/controllers/media/artists.py Outdated
Comment thread music_assistant/controllers/media/artists.py Outdated
Comment thread music_assistant/controllers/media/artists.py Outdated
Comment on lines +1058 to +1063
if (isinstance(lib_author, Artist) and not isinstance(prov_author, Artist)) or (
isinstance(lib_narrator, Artist) and not isinstance(prov_narrator, Artist)
):
library_item = await self.mass.music.audiobooks.update_item_in_library(
library_item.item_id, prov_item
)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@OzGav
Copy link
Copy Markdown
Contributor

OzGav commented May 17, 2026

@fmunkes can you have a look at what I am working on here as there is some overlap in ideas I think. music-assistant/models@main...add-classical

@fmunkes
Copy link
Copy Markdown
Contributor Author

fmunkes commented May 18, 2026

@fmunkes can you have a look at what I am working on here as there is some overlap in ideas I think. music-assistant/models@main...add-classical

Yeah, very similar, and much more extensive on your end :). I think your ArtistRole and the ArtistType do in some way describe the same, though I get that an artist can have multiple roles at the same (conductor, composer, etc). Then my question would be, which way we should go - have the same artist potentially multiple times in the database due to different types (e.g. one row for composer, one row for singer), or remove the ArtistType from the Artist completely and add the role of an artist via your credit approach.

I personally like the first approach a bit more, not because it's mine, but because I think the downside of having some duplicates, is compensated by the easier use of db methods via sqlite, i.e. by just filtering for the ID of the artist (get me all tracks composed by xy would be a db query, and not in python, which should be faster.

What do you think?

@OzGav
Copy link
Copy Markdown
Contributor

OzGav commented May 18, 2026

Thanks for taking a look. Looking at it more deeply I think these are actually compatible and we don’t need to pick one as they solve different problems.

Your Artist.type is the artist’s primary identity (what they mostly are — narrator, author, singer, composer). That’s what your audiobook PR needs, and it’s a useful field to keep.

Our Credit with ArtistRole is the per-relationship role — what this artist does on this specific track. Classical music (and could be extended to all music types) needs this because the same person can play multiple roles on the same recording (Karajan is both conductor and main artist on a Beethoven track) and different roles across recordings (some performers also compose). We can’t collapse that to a single per-Artist type without losing information.

On the performance point — both approaches are DB-side queries, not Python filtering. Ours filters on track_artists.role, yours filters on artist_id. Both are indexed; neither is slower.

Concretely I’d suggest: keep your Artist.type for primary type / audiobook browse, and if mine gets up then I’ll keep Credit for per-track-relationship role. They sit at different layers and serve different queries.

@fmunkes
Copy link
Copy Markdown
Contributor Author

fmunkes commented May 19, 2026

Alright, sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants