search

LMS-Community / slimserver

Server for Squeezebox and compatible players. This server is also called Lyrion Music Server.
https://lyrion.org
Other
1.19k stars 296 forks source link

Add all songs from search not working #1138

Closed Duberry closed 1 month ago

Duberry commented 3 months ago

When selecting on the 'add' or 'play' links at the top of a search result, only the first track returned is added to the currently selected devices playlist.

image

image

michaelherger commented 3 months ago

Hmm... I'm seeing a different behaviour: LMS would add all my library... How exactly did you get to this screen? What LMS version is this?

darrell-k commented 2 months ago

I did some work in this area, at least for LMS 9.0. So depending of the version @Duberry is running, I've either fixed it or broken it...

Duberry commented 2 months ago

Updated to 9.0.0 development build.

The behaviour persists. When I search within the 'Songs' search option, selecting the 'play' icon on the 'All Songs' top line results in only the first search return being added to the active players playlist.

I apologise for being unable to assist with much in the way of debugging. I'm an experienced LAMP / JavaScript developer and used to write a lot of Perl code decades ago, but the Lyrion codebase and trying to figure out how it all fits together makes my brain hurt.

Update from 8.5.3 to 9.0.0 was glitch free on Linux migration from Ubuntu repository logitechmediaserver package to installation of the .deb using dpkg. Removed logitechmediaserver first then installed lyrion 9.0.0. Database and plug-ins all persisted.

michaelherger commented 2 months ago

I can only confirm so far that it's broken for me too. But I'd actually get the first two items - in reversed order...

darrell-k commented 2 months ago

Whereas I get nothing at all added to the playlist. I'm debugging it...

Duberry commented 1 month ago

I'm keeping an eye on the nightlies and I can see changes being made to the search source. Updated and still the same behaviour, let me know if you need any testing.

darrell-k commented 1 month ago

Got diverted to the search performance issue, looking at this again now.

darrell-k commented 1 month ago

OK, I know what's going on here. A search for songs will use the fulltextsearch, so for example a search for "Elvis Costello" returns me a list of all songs featuring Elvis Costello.

However, clicking "All Songs" play or add simply does a search for track names containing "Elvis Costello", so in this case comes up with nothing and so adds nothing to the play queue.

This explains the inconsistent behaviour we're seeing: depending on the original search terms, anything between zero and all the songs will be added. So it will work as expected when searching for a song name, but not when searching eg songs for an artist.

I think the culprit is the code at the top of _getTagDataForTracks: because it receives the $search parameter as something like sql=(tracks.titlesearch LIKE 'ELVIS%' OR tracks.titlesearch LIKE '% ELVIS%') it just feeds that into the query and skips the fulltext search.

We could fix it there, or go back to BrowseLibrary::_tracks and make sure it's passing only the search terms and not an SQL snippet.

What do you think would be the best approach, @michaelherger ?

michaelherger commented 1 month ago

Could the search expression passed to the page handler be tweaked to transfer some cache key, in which the result set could be cached (or rather only the IDs of the resultset)?

darrell-k commented 1 month ago

Or just pass an array of trackIds from BrowseLibrary::_tracks?

darrell-k commented 1 month ago

This seems to work: See #1138

darrell-k commented 1 month ago

I mean PR #1167 !!!

michaelherger commented 1 month ago

Fixed by #1167 - thanks a ton, @darrell-k!

Duberry commented 1 month ago

Thank you. I'll download the Nightly tomorrow and give it a go.

Duberry commented 1 month ago

Can report it works. Good job.