Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Search for song #26

Open
xxxserxxx opened this issue Aug 14, 2024 · 13 comments
Open

[FEATURE] Search for song #26

xxxserxxx opened this issue Aug 14, 2024 · 13 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@xxxserxxx
Copy link
Collaborator

xxxserxxx commented Aug 14, 2024

The search function is quite limited, at the moment. It would be nice to expand search to a full global search, including song titles, genres, artists, and all the usual jazz supported by music player clients.

The feasibility of this greatly depends on the Subsonic API, because we would not want to do this on the client, as it would require loading the entire server music library metadata which -- in many cases -- would be impractical. I haven't looked into this at all, but Subsonic probably provides an API for this, so the effort would be mainly building a view around it.

On the one hand, extending the current simple '/' function would be nice; on the other, we would probably want a new panel with vertical splits for each category of search result: album, artist, genre, title. This would then need to have similar functionality to the Browser panel for navigation and adding results to the Queue.

This sounds like a non-trivial chunk of work, and what it would look like should be discussed before the hard, UI, part is started. A POC that simply accepts input, submits to the server, and caches the results might be a good start.

Update

Discussion #28 was created for ... discussing.

@xxxserxxx xxxserxxx mentioned this issue Sep 18, 2024
@xxxserxxx
Copy link
Collaborator Author

The PR does not implement search-by-genre. I'm going to break that out into a separate ticket.

@xxxserxxx
Copy link
Collaborator Author

Closed by #40

@spezifisch
Copy link
Owner

stmpscol

Current state of main behaves unexpectedly in the song list.

To reproduce:

  • Use Navidrome, probably
  • Start stmps, press '4'
  • enter an existing artist like 'pendulum'
  • (artist Pendulum now highlighted in artist column)
  • the songs list doesn't change, no matter what album is selected or if Return is pressed

PS: I added you as a collaborator. If I set it up correctly, you should be able to create branches directly in this repository now.
main branch is protected from pushes - the intended way is to 1. to create a new feature branch here, and then to 2. PR or I can directly merge if it's just cleanup.
Please LMK if that works out for you.

@xxxserxxx
Copy link
Collaborator Author

Thanks; I'll track this down.

And thanks for adding me as a collaborator! Now I may have to start getting serious about building out the unit tests.

@spezifisch spezifisch added the enhancement New feature or request label Oct 13, 2024
@xxxserxxx
Copy link
Collaborator Author

xxxserxxx commented Oct 15, 2024

* the songs list doesn't change, no matter what album is selected or if Return is pressed

Ok, this is a documentation issue -- this is the intended behavior. The artist/album/song results are the results returned in each category by search3. Nothing will change unless you change the search. If you search for "pendulum", you're going to see artist results for "pendulum", album results for "pendulum", and song results for "pendulum". This is how we get "search entire DB for songs matching X". We already have a "search for artists, and then navigate the result" in the browser tab.

The Subsonic API search3 returns a searchResults structure containing a branch with matched artists, a branch with matched albums, and a branch with matched songs.

Navidrome is apparently returning albums in the album section that have "pendulum" somewhere in their idv3 metadata. Could you dump the tags for album titled "LP009" and see which field contains "pendulum"?

Gonic behaves differently -- it only returns artists with "pendulum" in the artist name in the artists results; albums with "pendulum" in the album name in the albums results, and songs with "pendulum" in the title in the songs results.

As I'm sure you've discovered, the Subsonic API behavior is very poorly defined; the API doesn't say what servers should search for with search3 or how to organize the results, only that search3 searches idv3 data. Which is correct depends on your perspective; IMHO gonic is doing this more correctly than Navidrome, but -- again -- "correct" is poorly defined.

Now, I could change the behavior to do client-side filtering so that only titles match; this would, essentially, re-implement search on the client. The more grave concern is that, now, search3 in stmps will no longer match search3 in the Navidrome web interface. I think, whichever Subsonic server a user is using, the clients should all act the same.

As for the search panel, it is not a browser like BrowserPage. It merely displays the search results, filtered by result type into the columns. search3 returns three categories ("artist", "album", and "song"), and we just put each result from each section into its own column. a (and Enter) should be adding whatever is selected to the queue: if you are in the artist column and select an artist and press a, all of the artist's results should be added to the queue; same for a selected album, or a selected song... if that's not working for you, that's a bug. It is not expected that selecting a different album should change the songs -- the songs column doesn't show all of the songs for the select artist or album, but all of the songs that the server returned as search result matches.

This behavior is different than BrowserPage because BrowserPage is a navigator, and SearchPage is not. SearchPage displays search results, which you can coincidentally add to the queue from.

I'm interested in your perspective on this, but I think it's how it should behave. I do think it'd be worth describing how the search page works in the README, and maybe in the HelpPage. I don't have a better idea for how to address Navidrome's results; again, they're not necessarily wrong because the API docs don't describe what's right, but I do think gonic has a more correct behavior:

2024-10-15-075111_1660x695_scrot

I could add something to explain Navidrome's decisions: print in the log for each item the user selects which idv3 field matched the query (probably only in debug mode, because otherwise the log would get messy); or add another status line with ": ", and this would change as the user navigates around in the UI between elements. That might help people understand e.g. why Navidrome returned album "LP009" as an album that matched the search3 query for "Pendulum".

@xxxserxxx
Copy link
Collaborator Author

Yeah, I've just confirmed this: Navidrome's search3 function includes in each section anything of that type that has the search query in any metadata fields. This means that if you search for "black", you'll get "The Black Keys" in the artists section, but also their album "El Camino" in the albums section (because the album artist "The Black Keys" matched). Basically, Navidrome provides redundant results: "The Black Keys" shows up in the artist results; all of "The Black Keys'" albums show up in the album results -- whether or not "Black" is in the album name; and all of "The Black Keys" songs show up in the song results, again, whether or not "Black" is in the song name.

I don't know if you noticed, but results are paged. You're only getting the first 20 results in each column. Press n to load another 20 in each column. With Navidrome, you need paging because it's going to duplicate results, and the "songs" column is going to get very full since it's matching the query in all of the songs' metadata fields.

So, I'm going to update my position on this: Navdrome's search function is confusing and almost useless, since it over-matches. I think it'd be useful to simulate gonic's behavior by client-side filtering, so that only album and songs matching the query are actually populated into the columns, regardless of what the server returns. This may mean performing multiple server calls for each search, because the results are in no order and because of how Navidrome implements search3, it's possible (and even probable) that the first several pages of song results to not contain any with titles matching the query. It would be confusing for the user to press n repeatedly with no change in the columns, just to eventually get to songs with the query in their title.

tl;dr: Navidrome's search3 heuristics are not useful, and I'd like to replicate gonic's heuristics on the client such that a search returns artists with the query in their name in the artists column, albums with the query in their title (and only albums with the query in their title) in the albums column, and the same for songs in the songs column. Further, I'll need to make sure that at least some results are put into each column on ever n press. For gonic, this is already the case, but Navidrome may return several pages of results without the query contained in the relevant tag, and so the code may need to make several server search calls to get useful results.

@spezifisch
Copy link
Owner

I'm interested in your perspective on this, but I think it's how it should behave.

Now that you explained it, it seems obvious to me. Maybe we can avoid confusion also by using the Box title "artist results" instead of "artists" etc.

@spezifisch
Copy link
Owner

That might help people understand e.g. why Navidrome returned album "LP009" as an album that matched the search3 query for "Pendulum".

This is actually the behavior I expected first because it kinda looked like three-pane directory browser mode in macOS Finder, which is of course hierarchical.

@xxxserxxx
Copy link
Collaborator Author

I agree about the rename -- I'll sneak that into some other PR.

Yeah, I like the idea of adding a "why." Even if it's only Navidrome that behaves this way, it doesn't really hurt to show the matching IDv3 tag. New ticket incoming.

@xxxserxxx
Copy link
Collaborator Author

xxxserxxx commented Oct 15, 2024

@spezifisch actually, would you rather have the current behavior with an explanation, or what I suggested: filter on the client side (emulating gonic's behavior) so that no explanation is needed?

Edit

The more I think about this, the stronger I feel about emulating the gonic approach. I think it's better, and users don't know what's being used under the hood so there's no reason to keep the Navidrome behavior. Having columns contain things which obviously match, and why they match, is a better solution IMO.

@xxxserxxx
Copy link
Collaborator Author

Pulling into 1.0.0 because search has already been merged (which, by itself, would implement this), and the additional PR is fixes for Navidrome (and possibly other servers).

@spezifisch
Copy link
Owner

Having columns contain things which obviously match, and why they match, is a better solution IMO.

That's convincing. I was leaning the other way, but let's go with that instead.

@xxxserxxx
Copy link
Collaborator Author

That's awesome, because it's already committed and PR'd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

No branches or pull requests

2 participants