androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
https://developer.android.com/media/media3
Apache License 2.0
1.71k stars 409 forks source link

Searching in Media3 #1525

Open WSteverink opened 4 months ago

WSteverink commented 4 months ago

First of all, thanks for Media3 its pretty awesome 👍 .

I do have a few questions related to searching. We are implementing and using Media3, and we are also adding search support (mainly for Android Auto). I noticed the following:

When using the MediaController app, a search triggers onAddMediaItems. I would like to achieve something like the snippet below.

override fun onAddMediaItems(
    mediaSession: MediaSession,
    controller: MediaSession.ControllerInfo,
    mediaItems: MutableList<MediaItem>
): ListenableFuture<MutableList<MediaItem>> {

    val query = mediaItems.find { it.requestMetadata.searchQuery != null }?.requestMetadata?.searchQuery

    if (query != null){
        datasource.search(query) // Returns item(s) based on the first query found
    } else{
        val ids = mediaItems.map { it.mediaId }
        dataSource.resolveItems(ids) // Returns list of media items
    }

Question 1: Will onAddMediaItems, when containing a MediaItem with a query, ever contain multiple items? As far as I can tell, it is always a list of one.


Another thing I noticed is that when using the DHU to test Android Auto, there is also a search bar. This search request seems to be using a Legacy Session, as it triggers MediaLibraryService.onSearch and MediaLibraryService.onGetSearchResult.

Does onSearch have to match the results I want to return in onGetSearchResult? Since I don't like to trigger an async search in both functions, I am considering always returning success in onSearch and performing the actual async search in onGetSearchResult.

It would look something like this:

        override fun onSearch(
            session: MediaLibrarySession,
            browser: MediaSession.ControllerInfo,
            query: String,
            params: LibraryParams?
        ): ListenableFuture<LibraryResult<Void>> {
            session.notifySearchResultChanged(browser, query, 1, params)   // <------- fake success of 1
            return Futures.immediateFuture(LibraryResult.ofVoid())
        }

        override fun onGetSearchResult(
            session: MediaLibrarySession,
            browser: MediaSession.ControllerInfo,
            query: String,
            page: Int,
            pageSize: Int,
            params: LibraryParams?
        ): ListenableFuture<LibraryResult<ImmutableList<MediaItem>>> {

            return scope.future {
                val results =  mediaDataSource.searchForMediaItems(query) // <------- actual search results
                LibraryResult.ofItemList(results,params)
            }
        }

Question 2: will the above be safe to do?


Question 3: Would it be possible to funnel all the 'search types' through the same function and drop one of the two? For example, onAddMediaItems or a dedicated search function.

marcbaechinger commented 4 months ago

Thanks for your question!

Question 1: Will onAddMediaItems, when containing a MediaItem with a query, ever contain multiple items? As far as I can tell, it is always a list of one.

Media3 sends only a single media item to the MediaSession.Callback when the legacy a onPlayFromSearch requests arrives from a platform or legacy controller. This is to provide backwards compatibilty with media1 mediaControllerCompat or platform MediaController.

As far your app or another app that you allow to connect with a Media3 controller doesn't set multiple such media items with a search term, there is only a single item to expect. Doing that wouldn't really be sensible IMO. In such a case, you should advice such a client to do differently I think.

Question 2: will the above be safe to do?

This depends on what the app that receives the initial search result notification. But as long as you notify that there is at least a single item, I'd expect the controller app is calling onGetSearchResult with the same query. Do you know what app is sending the onPlayFromSearch ? Is this coming from Android Auto/AAOS too?

Question 3: Would it be possible to funnel all the 'search types' through the same function and drop one of the two? For example, onAddMediaItems or a dedicated search function.

From an API perspective there are two different APIs: MediaBrowser.search and MediaBrowser.getSearchResult that are targeted against the MediaLibrarySession.Callback. The other API is from MediaControler.setMediaItem(MediaItem) that can have a RequestMetadata with a search query, which is what you receive in the MediaSession.Callback.

While the latter is available for MediaBrowser and MediaController, the former is only available to a browser. While it would be probably possible to unify this to a single search callback of the MediaSession.Callback I think we need to provide the option for apps that they can respond differently in these two cases if an app wants to. We would at least have to tell that callback method where the search is coming from. This would require the MediaSession.Callback.unifiedSearchForLibraryOrMediaSession(query), as the lowest common denominator, to provide a hint whether the search comes from the browser or the controller. For a user that only has a MediaSessionService this looks a bit confusing to me to be required to take into account a browser that isn't relevant.

I wonder if there is a reason that makes it difficult for an app to funnel this into a single method with app code? Like what you ask for the library to do, your app could just delegate in the three callbacks to the same search method? This seems to be fairly easy from my, probably naive, understanding. Is there something from the library side that makes this difficult?

WSteverink commented 4 months ago

Do you know what app is sending the onPlayFromSearch ? Is this coming from Android Auto/AAOS too?

Yes, i am testing with the Android Auto DHU which seems to use onSearchand onGetSearchResult . I was expecting that the PageSize and Page params might be the reason to return a number of results in onSearch first.

Android Auto seems to use Int.MAX_VALUE as a pageSize though. I will try what a watch will do.

Is there something from the library side that makes this difficult? Nope, it was just a suggestion to improve the API a bit :) .

Now that i think about it more, could we say the following:

marcbaechinger commented 4 months ago

Yeah I was thinking about the page size as well, but it would be a bit weird to set it to 1, even when not more items are expected to be contained in the search result. You are right it's at the app discretion to set these params. I think if Auto is using MAX_VALUE like you say, you can expect this to be like this consistently.

Searching via setMedia should probably return 1 item that can be prepped/played right away

I think for instance Assistance would expect an app to search and then play an arbitrary number of items. For Auto the same appies IMO. I'm not a hundert percent sure about what Assistant is doing, but I would expect when I say "Hey Google, play Pink" I'd expect assistant to query a search with Pink and I as a user would expect to get a playlist rather than a single song. Further, the more blurry the search term is the more likey it is you don't hit what a user wants and returning a list sound more useful for a user?

that can be prepped/played right away

That's similar for hwen we put the list of items in the playlist and then start playing the first item I'd say.

marcbaechinger commented 4 months ago

session.notifySearchResultChanged(browser, query, 1, params)

Just one note I wanted to add. If you send this, you should be sure to have at least 1 search hit when the app collects the search result. If you don't find an item for a query, the app is likely to expect this to know at the first request, so that the user can be informed quickly.

I also think that auto does a search-as-you-type which expects the search to quickly return an empty result as quickly as possible in the first call.

WSteverink commented 4 months ago

That's similar for hwen we put the list of items in the playlist and then start playing the first item I'd say. Yes, that makes sense.

I will most probably delegate all search related function through a single function that can return a list of MediaItems. I will use some sort of caching so that onSearch and onGetSearchResults won't trigger 2 async calls on the client side.

I also think that auto does a search-as-you-type which expects the search to quickly return an empty result as quickly as possible in the first call. The DHU only triggers onSearch after submitting explicitly, but i am not sure if this is the same for all versions.

Anyway, i think i can work with this! Thanks for you responses 👍