simojenki / bonob

sonos SMAPI implementation allowing integrating different music sources with sonos.
GNU General Public License v3.0
208 stars 15 forks source link

Issue loading large number of playlists #158

Closed Vorror closed 1 year ago

Vorror commented 1 year ago

Bonob seems to have issues loading the "playlists" section of the Sonos controller app. If I have a small number of playlists (< 20), then bonob/Sonos controller will correctly and quickly load all of the playlists.

image

Unfortunately, I have 200 (smart) playlists. Sometimes, only the first 40 or so will load. The majority of the time, either Sonos or Bonob will time-out/give up retrieving the account's playlist and either show a blank page or only shows a small portion of the available playlists.

The issue may be because bonob is trying to retrieve the playlist album art for every playlist.

Can I disable album arts for the playlist section/folder?

simojenki commented 1 year ago

Yes, it could be beside it's too busy generating album art.

Given that ND now supports playlist artwork perhaps this feature should be disabled for ND

Vorror commented 1 year ago

Yes, it could be beside it's too busy generating album art.

Given that ND now supports playlist artwork perhaps this feature should be disabled for ND

Thanks for the quick reply. Yes, that's my feeling based on watching the logs of bonob and noticing how slow the album art-related messages seem to pass.

simojenki commented 1 year ago

I've added a config param BNB_DISABLE_PLAYLIST_ART and released a new version, try setting this to "true" and it should disable album art and just show an icon. When I have some time I'll come up with a better solution that uses ND for the playlist art now that it supports it.

Vorror commented 1 year ago

Thanks! I just tried the setting, and playlists load much faster!

dhalem commented 1 year ago

I tried it as well and they are still loading slowly.

simojenki commented 1 year ago

Still loading with images disabled? How many playlists?
Which backend type? How does the API of the backend itself perform?

With images disabled bonob really doesn't do much other than transform the subsonic api into something that Sonos understands so it's quite likely that the performance issue is elsewhere.

dhalem commented 1 year ago

The backed in ND, and I have ~800 playlists. It looks like the lag is on the ND side, but it also appears that bonob is pulling the full set of 800 on each request. I'm still poking around trying to see how pagination works.

dhalem commented 1 year ago

I see ND trying to load album art in its logs right now.

dhalem commented 1 year ago

I also see lots of individual calls to getPlaylist in the ND logs.

simojenki commented 1 year ago

Yes, still loads each playlist for the tracks, which on 800 is going to be very slow... I'll look at fixing that.

simojenki commented 1 year ago

Out of interest, how are you managing 800 playlists? That's a lot of playlists to be individually adding tracks to.

dhalem commented 1 year ago

That’s a longer story. I have a home built system that syncs them from various sources (Spotify, Tidal, Plex, etc) then matches up tracks across those and pushes them back to the same services as well as exporting them as file system artifacts that Sonos and Navidrome can read.

dhalem commented 1 year ago

Does it need to get all the tracks during the listing phase? Seems like it can operate on a shallower set of data.

simojenki commented 1 year ago

No, not any more. It did that because ND did not support album art for playlists, so bonob generated it from the playlist tracks. However now that ND supports the same functionality natively it can be removed.

I hadn't imagined that people would have 800 odd playlists

dhalem commented 1 year ago

Thanks. I think I understand the code enough to fix it.

On Tue, Apr 25, 2023 at 1:51 PM Simon J @.***> wrote:

No, not any more. It did that because ND did not support album art for playlists, so bonob generated it from the playlist tracks. However now that ND supports the same functionality natively it can be removed.

I hadn't imagined that people would have 800 odd playlists

— Reply to this email directly, view it on GitHub https://github.com/simojenki/bonob/issues/158#issuecomment-1522399100, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACESIEUABWOR2TUSMT5DYNTXDA2NNANCNFSM6AAAAAAXCBYV5I . You are receiving this because you commented.Message ID: @.***>

simojenki commented 1 year ago

Go for it, I'm extremely busy right now with other things, so if you can send me a PR that would be great.

You could re-use the same config param i added in the most recent commit to master to disable loading the tracks for the playlists?

dhalem commented 1 year ago

I’m the opposite. Retired

On Tue, Apr 25, 2023 at 2:25 PM Simon J @.***> wrote:

Go for it, I'm extremely busy right now with other things, so if you can send me a PR that would be great.

You could re-use the same config param i added in the most recent commit to master to disable loading the tracks for the playlists?

— Reply to this email directly, view it on GitHub https://github.com/simojenki/bonob/issues/158#issuecomment-1522438006, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACESIES4COD26R4WU4F7QSTXDA6OBANCNFSM6AAAAAAXCBYV5I . You are receiving this because you commented.Message ID: @.***>

dhalem commented 1 year ago

PR sent

simojenki commented 1 year ago

PR merged. That 'master' tagged docker image should contain this change. Let me know how it goes, if its good, ill tag and release.

Vorror commented 1 year ago

@dhalem Did you validate your changes?

dhalem commented 1 year ago

Ah yes. I did. Apologies.

On Sat, May 13, 2023 at 10:16 AM Paul J. Miller @.***> wrote:

@dhalem https://github.com/dhalem Did you validate your changes?

— Reply to this email directly, view it on GitHub https://github.com/simojenki/bonob/issues/158#issuecomment-1546713530, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACESIEV47O2YSVUEVGBO5N3XF66VTANCNFSM6AAAAAAXCBYV5I . You are receiving this because you were mentioned.Message ID: @.***>

Vorror commented 1 year ago

@simojenki I guess you can tag/release? I'll test the version that's uploaded. If everything things ends up okay Ill close the ticket.

Vorror commented 1 year ago

After running with @dhalem changes getting playlists is really fast now. Good work!

Closing.

simojenki commented 1 year ago

This is released, sorry was moving house, so hadn't had time.