Add support for searching media to Kodi - #161681
Conversation
|
Hey there @OnFreund, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
1d681d7 to
26bbb7f
Compare
26bbb7f to
db062a3
Compare
Kodi provides a mechanism to query the available movies, TV shows etc using jsonrpc. This change adds a logic to match them against the search query using rapidfuzz. This change only adds the support for searching movies and TV shows. I do not have music hosted in my kodi instance to test it. But the logic can be extended in the same way. The search behaviour is as follows: For movies, return the matching movie(s). For TV shows, return the first unwatched episode for the matching show(s). Sample usage: "Play Avengers on Kodi" "Play Brooklyn nine nine on Kodi"
db062a3 to
1028f23
Compare
If the requester (say an llm conversation agent) sends an invalid value for media class (e.g. 'tv' instead of 'tv_show') the current logic will filter it out entirely. Instead we can default to the allowed list when this happens.
joostlek
left a comment
There was a problem hiding this comment.
So while I think this is a neat change, I consider the matching of the titles a service specific, so would it make sense to move this to pykodi? The library seems to be maintained by codeowners, so we might be able to move it there and make our code less service specific
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
I would normally agree with you. However, the pykodi library does not seem to be maintained. The last checkin was almost 3 years ago. |
|
Sent the maintainer a message |
|
He just said he's open to such a PR :) |
|
Thanks! Will update. |
|
I have created a PR in PyKodi. OnFreund/PyKodi#21 |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
I have created a PR in PyKodi. OnFreund/PyKodi#21 |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
I have created a PR in PyKodi. OnFreund/PyKodi#21 |
|
@OnFreund , @joostlek, can we conclude on the approach in this PR vs the approach from the PR in the PyKodi library? If this current PR's approach is the preferred approach, I can rebase my change to the latest code base. |
|
Since the "search" functionality isn't really related to Kodi in any way, and is just a filter on the browse results, I don't think it belongs in the library, and would prefer finding a solution here. |
|
There's no other concrete media player integration which implements the search functionality in the integration, here's a summary of integrations which support search. Hence, I agree with @joostlek it makes sense to add support for searching kodi libraries to the supporting library. |
|
@emontnemery thanks, really helpful. Note that all of these use a native search mechanism. This is not the case here - there's no native mechanism - all we're doing is filtering the results, which, btw, could be useful for other media integrations. |
Yeah, I had a quick look at the kodi RPC docs and was quite surprised to see there doesn't seem to be a way to search. Not sure how active kodi development is, but it seems like a reasonable feature to add? How does the native kodi app and webui work without search, is everything focused around scrolling through lists?
Do you mean something like a fallback implementation in the base class which implements search by fetching everything and then filtering? It seems wildly inefficient to implement it like that so I'm not sure we'd accept such a proposal. |
|
Would it be reasonable to add the current filtering approach for now and switch to real search when kodi adds it? btw, kodi application does have a search option. However, it shows the search results on the screen. It does not return the search results. The VideoLibrary.Get* (e.g. VideoLibrary.GetMovies) supports a contains operator, but not a fuzzy search. The contains (substring match) search is hardly useful when combined with a voice assistant, because it would need a precise substring. |
That's a good idea, but given the Kodi release cycle, the highly optimistic case is for this to be available in over a year (probably more like early 2028).
I was thinking more opt-in, but you raise a good point: if we're reluctant to add this to other integrations, there's no reason this should be any different. I can see 3 options:
In either case, this isn't integration-specific, and definitely not part of the Kodi library. |
IMO it has to be in @OnFreund's PyKodi library unless there's a compelling reason for adding search to the media player base class. If we want to go for the latter, there needs to be an architecture proposal explaining why it's a good idea. |
The one thing I'm absolutely sure of is that in-client filtering of results does not belong in the Kodi library (see my comment above). |
It belongs in a library outside of the core integration, I don't care if it's in your library or some other library. As I wrote, the alternative would be to add search to the Home Assistant media player base class, but then you need to show why that's a good idea. |
I'm not claiming it's a good idea. My claim is that there's nothing special about this integration. This is an external filtering mechanism, and it's either something we're willing to support and make available to other integrations (i.e. a good idea), or something that we think no integrations should do (a bad idea). I don't think that "it's a bad idea in general but a good idea for Kodi" makes sense. |
|
To rephrase - I have no opinion on whether it's a good or bad idea, but I do have a strong opinion that it's not a good idea for this integration if it's a bad idea for others. |
|
Hi, Do we have any consensus on the preferred approach?
|
Kodi provides a mechanism to query the available movies, TV shows etc using jsonrpc.
This change adds a logic to match them against the search query using rapidfuzz.
This change only adds the support for searching movies and TV shows. I do not have music hosted in my kodi instance to test it. But the logic can be extended in the same way.
The search behaviour is as follows:
For movies, return the matching movie(s).
For TV shows, return the first unwatched episode for the matching show(s).
Sample usage:
"Play Avengers on Kodi"
"Play Brooklyn nine nine on Kodi"
Proposed change
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: