music-assistant / hass-music-assistant

Turn your Home Assistant instance into a jukebox, hassle free streaming of your favorite media to Home Assistant media players.
Apache License 2.0
1.21k stars 44 forks source link

When you change the album in all tracks from the album, a new album is created but the old one isn't removed #2362

Closed nldroid closed 3 weeks ago

nldroid commented 1 month ago

What version of Music Assistant has the issue?

2.0.4

What version of the Home Assistant Integration have you got installed?

None

Have you tried everything in the Troubleshooting FAQ and reviewed the Open and Closed Issues and Discussions to resolve this yourself?

The problem

When you change the album in all tracks from the album, a new album is created but the old one isn't removed.

I'm currently organizing my local files so i'm changing a lot of Album metadata in tracks. This creates 'new' albums for the tracks and i found out that the old albums were still present in Music Assistant.

I found the problem running this query:

select * from albums where item_id not in (select album_id from album_tracks)

image

Here you can see that i changed the Album artist:

image

So Music Assistant sees that the Album artist from the tracks is different. All tracks are 'moved' to the new album and the old empty one isn't removed.

image

How to reproduce

Have an Album without musicbrainz tags on the tracks Make sure the album is in the Music Assistant library Tag the tracks with Musicbrainz Picard Synchronize the local file provider See that the old Album still available in Music Assistant but without any tracks

Music Providers

Filesystem (local tracks)

Player Providers

Chromecast

Full log output

No response

Additional information

No response

What version of Home Assistant Core are your running

None

What type of installation are you running?

Home Assistant OS

On what type of hardware are you running?

Linux

nldroid commented 1 month ago

There probably needs a check in _set_track_album: https://github.com/music-assistant/server/blob/bf9b16dc61c558bf784f6e13a55cf6e5eecff7c4/music_assistant/server/controllers/media/tracks.py#L396 that checks if there are still tracks for the old album and if not, remove the album.

Same probably for _set_track_artists and _set_track_artist

nldroid commented 1 month ago

Actually it goes wrong in sync library

At the end there's a await self._process_deletions(deleted_files)

But the problem is that the existing tracks are updated and not deleted. Once the program comes here

            if library_item := await controller.get_library_item_by_prov_id(
                file_path, self.instance_id
            ):
                if library_item.media_type == MediaType.TRACK:
                    if library_item.album:
                        album_ids.add(library_item.album.item_id)

it can't find the library_item anymore because the record is re-used for the new track.

This is the situation in provider mappings with the old album tags:

image

And after the sync with the new album, right before the _process_deletions is called:

image

In _process_deletions the file path Vibe/01 Ohh Baby Baby.flac is used as parameter for controller.get_library_item_by_prov_id and thats not in the database anymore..

This is the contents of deleted_files:

image

OzGav commented 1 month ago

PRs are always welcome or you can wait until time allows this to be looked at.

OzGav commented 1 month ago

Actually I think this might be the same as or related to this https://github.com/music-assistant/hass-music-assistant/issues/2167

nldroid commented 1 month ago

PRs are always welcome or you can wait until time allows this to be looked at.

I probably can help with some coding but i guess we need to think/discuss on an approach. My first thought is to pass a list of 'items' to the _process_deletions instead of just the names/id's. And populate the list before the tracks are updated. You guys may have another idea/preference.

marcelveldt commented 1 month ago

This is an issue that can only exist on local files so we need to fix it in the sync method. After a sync we just need to do a check for all orphaned items on this provider instance and remove those. I think the code for that is already halfway there, I should be able to fix it later this week if still needed.

OzGav commented 3 weeks ago

@nldroid can you confirm your fix has resolved this?

nldroid commented 3 weeks ago

Yes. I've created the fix ;)

OzGav commented 3 weeks ago

I saw that just checking!