-
Notifications
You must be signed in to change notification settings - Fork 405
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
MediaLibraryService.MediaLibrarySession::notifyChildrenChanged doesn't update Android Auto Browse #561
Comments
Yes, I was about to ask the same. Within MediaLibraryService extension class, what is a way to update media browser, similar way notifyChildrenChanged worked for MediaBrowserService. |
Hi @fourofspades, From the documentation of Also it would easier for us to debug if you can try and reproduce the problem on our |
Done some more work on this, and I'm not exactly sure what fixed this, however I am now getting the browser updating when the phone's library updates. I had already implemented onSubscribe, my fix was to manually call it again when the local collection changes, taking care not to create a recursive calling loop (I bundled a flag in the params to not refresh the library, as my library call into the library would also do that). |
See above. It's now working as expected. |
I am sorry for the late response. I think my issue a bit different. What I meant is if there is a class extends MediaLibraryService and internal collection of media items changed what is a way to ask framework to call MediaLibrarySession.Callback: onGetChildren, etc ... |
@ChernyshovYuriy Have you tried using the method with the same name? |
Yes, and it doesn't work to me. The use case is simple, I need to call onGetChildren or onGetLibraryRoot to update current browse tree on Automotive. Invoking mSession.notifyChildrenChanged doesn't trigger any callbacks on MediaLibrarySession.Callback. |
Just a note from debugger. The callback ended up here, in
|
This sounds like the same issue that is described in #561 (comment): The caller is not subscribed, but manually faking a subscription makes the code work again. @marcbaechinger Have you seen this issue before or are you aware of any pitfalls with Android Auto subscriptions? |
Thank you for re-considering the issue. On another hand, |
To give a more clarity on this case, here is a use case:
So how can I tell to framework to call children to update?
As I mentioned, when doing extension to MediaBrowseService, call notifyChildenChanged(browse_id) did exactly what it needed to deal with all use cases I provided. Unfortunately, can't do that with a new design... |
Hey Yuriy! Nice to meet you again. It's been a while! :)
That's the correct API to use. I tested your case with the session demo app and this seems to work I'm afraid. So not sure how helpful the following is for you: I changed var categoryChanged = false
override fun onGetChildren(
session: MediaLibrarySession,
browser: ControllerInfo,
parentId: String,
page: Int,
pageSize: Int,
params: LibraryParams?
): ListenableFuture<LibraryResult<ImmutableList<MediaItem>>> {
Log.d("subscribe", "onGetChildren called for $parentId")
val children =
MediaItemTree.getChildren(parentId)
?: return Futures.immediateFuture(
LibraryResult.ofError(LibraryResult.RESULT_ERROR_BAD_VALUE)
)
if (!categoryChanged && parentId == "[artist]The Kyoto Connection") {
categoryChanged = true
Log.d("subscribe", "post delayed")
Handler(Looper.myLooper()!!).postDelayed({
Log.d("subscribe", "trigger notifyChildrenChanged for $parentId")
session.notifyChildrenChanged(browser, parentId, children.size, params)
}, 10_000)
}
return Futures.immediateFuture(LibraryResult.ofItemList(children, params))
}
This worked for me by just deploying the session demo app from Android Studio to the Automotive emulator. Then select another app (like News), and then select the demo app again to make it load the library root. This change adds an I added a bit more logs in other classes, but essentially I see at the end that 10 seconds after the category is requested (
I'm testing with Automotive emulator API 29 that I randomly created with the device manager.
I noticed that Automotive is only subscribed to a single When testing with https://github.com/googlesamples/android-media-controller I see similar behaviour. Specifically, with both, Automotive and the controller test app I see that So this looks fine to me from the library side. The only thing that comes to mind is whether your service overrides |
Hello @marcbaechinger ! It's really me :-) I'm thrilled that you remember me, and I'm delighted to be chatting with you again. |
I created a very simple Automotive music app: Tested it on Automotive emulator 1024p landscape API32 12L ""Sv2. I tried to apply the But no I use Android Studio Hedgehog | 2023.1.1 Canary 15 |
Thanks for the test fixture. When I override the default implementation of I added the following code which makes it work, you may want to review and change it a bit to fit your requirements: override fun onSubscribe(
session: MediaLibrarySession,
browser: MediaSession.ControllerInfo,
parentId: String,
params: LibraryParams?
): ListenableFuture<LibraryResult<Void>> {
return Futures.immediateFuture(LibraryResult.ofVoid())
} The reason why this is required is that Aside: Overriding So I think for you app you can just add the snippet above to override For the Media3 library I'd actually think we should make the default implementation of Can you check whether overriding |
This comment was marked as outdated.
This comment was marked as outdated.
Hey @marcbaechinger , No need to apologize whatsoever. Being a software engineer myself, I totally get where you're coming from. This is exactly why we have this issue tracking system in place - to improve things. And regarding your suggestion - it's spot on! I've tested it with the example code and it's definitely working. Big thanks for your assistance, really appreciate it! |
@marcbaechinger Assuming this is a common requirement for Android Auto implementations, would it make sense to maintain this |
I had a problem where By the way, I think the Javadocs should put a lot more emphasis on this, because it's pretty easy to miss, or maybe make the onSubscribe return a RESULT_SUCCESS by default ? |
I agree that would be a solution that removes some friction and confusion. I have a commit that does exactly this. However, when I do return I can of course limit this, so that we for instance only allow a browser to subscribe to lets say 20 parent ids. This would prevent such an OOM attack but looks a bit arbitrary and can be confusing as well. I'm inclined to implement such an upper limit of lets say 20 parentId a browser can subscribe to. Once this limit is reached, no more subscription would be allowed. The issue/downside is that this upper limit probably also would be in place if an app actually overrides What are the sentiments of developers having such a productive app? How would you implement Thoughts? |
I now grasp the reasoning behind the decision to not have I believe that most media apps don't require verification of callers. This is primarily because media content, like music, is generally not sensitive (with an emphasis on "most" here). Additionally, it's rare, if not non-existent, to encounter malicious apps that would attempt to exploit a media app's browser service solely to access its data. Many music apps either play local files or enable remote music playback, such as the likes of Spotify. In my experience, there's often no need to validate the identity of callers. The need for caller identity verification is quite niche and, therefore, negligible. I don't see any compelling reasons, other than those you've mentioned, for Let alone the fact that this may potentially fix the (probably) broken Android Auto support, where Android Auto is not refreshing the media library (#644) since it's requiring all browsers to explicitly subscribe in order to receive callbacks. This however will require some way to add the android auto browser passively to the subscriber list. |
I consider the idea of adopting the logic contained in UAMP's PackageValidator.kt officially under media3 in the sole case where the caller is one of those listed in the It may even be enough to check the package name (one of the two: "com.google.android.projection.gearhead" or "com.android.auto"). However, it's very easy to mimic a package name, so as I mentioned, the package validator class does the job of detecting the authenticity of the caller. I am not sure how things work under the hood as i've taken very quick and non-thorough looks on the source code, but I believe it's one of the two:
However, since I am not all aware of how media3 works internally, I cannot think of a valid solution. I may be looking at the wrong direction, or maybe the Android Auto list of items not being refreshed is not related at all to this. |
The library already maintains the subscribed controllers internally. This change adds `MediaLibrarySession.getSubscribedControllers(mediaId)` to access subscribed controllers for a given media ID. To accept a subscription, `MediaLibraryService.Callback.onSubscribe` is required to return `RESULT_SUCCESS`. So far, this isn't the case for the default implementation of the library. This change implements `Callback.onSubscribe` to conditionally provide `RESULT_SUCCESS`. The default calls `Callback.onGetItem(mediaId)` to assess the availability of the media item. If the app retruns `RESULT_SUCCESS` with a browsable item, the subscription is accepted. If receiving a valid item fails, the subscription is rejected. Issue: #561 PiperOrigin-RevId: 568925079
The commit above provides a default implementation of An app can later call This behavior can be changed by overriding This will be included in 1.2.0 for which we will ship the next alpha soon. I'm closing this issue. Please open a new issue if required. |
The library already maintains the subscribed controllers internally. This change adds `MediaLibrarySession.getSubscribedControllers(mediaId)` to access subscribed controllers for a given media ID. To accept a subscription, `MediaLibraryService.Callback.onSubscribe` is required to return `RESULT_SUCCESS`. So far, this isn't the case for the default implementation of the library. This change implements `Callback.onSubscribe` to conditionally provide `RESULT_SUCCESS`. The default calls `Callback.onGetItem(mediaId)` to assess the availability of the media item. If the app retruns `RESULT_SUCCESS` with a browsable item, the subscription is accepted. If receiving a valid item fails, the subscription is rejected. Issue: androidx#561 PiperOrigin-RevId: 568925079
Issue: androidx#561 Issue: androidx#644 Issue: androidx#645 PiperOrigin-RevId: 568948230
Thanks for all the work with Media3, it is a great improvement on the disparate methods required in the previous versions of ExoPlayer, I am certainly noticing the benefits in tidying up and flexibility already. Right now I am attempting to resolve an issue, which is identical in nature to the one raised earlier regarding onGetChildren not being called for Android Automotive after notifying of a change to children via: mediaLibrarySession.notifyChildrenChanged(controller,"favourites",allFavouriteItems.count(),LibraryParams.Builder().build()) The onGetChildren for parent Id "favourites" is not being called again. Is called once when the service starts but is not called upon subsequent requests via the notifyChildrenChanged. Hence, if the list of items updates the Automotive UI is not updating children. Which leaves an invalid list presented to the user. I have attempted to update to the alpha version mentioned hoping to use the onGetItem method to confirm subscription, but before proceeding further I have immediately noticed that my list of customcommands is no longer displaying correctly. These worked perfectly in 1.1.1, but with they look like this: In 1.1.1: The icons work in both cases - they trigger their callbacks but can't proceed testing out the alpha until I understand why these are now incorrect. So essentially:
and also noticed
I am eager to resolve the issue around the subscriptions for my parentIds, but not wanting to delve further into the alpha if I am already immediately noticing an issue with customCommands. Many thanks for your advice in advance! |
Thanks for your report. Lets leave the Regarding custom commands with AA/AAOS. In the main branch, we've updated the demo app, enabled Android Auto support for the session demo and added an Automotive OS module. Both apps use the same
In short, set the available commands and custom layout of the media notification controller to set the actions and custom actions of the platform/media1 session where Android Auto and Automotive OS read from. Note: EDIT: Sorry, I forgot to ask can you give me more details on how you configure the custom commands/layout in |
Hello @marcbaechinger, Could you explain, why should we use The second question: How can I even have several versions of |
The library can't know how many controller/browser connect or subscribe and other apps may expect more than one call to subscribe. It can be that 2 apps are connected to your session each with their browser and then both subscribe for the same
I'm not sure I understand the question. If you have multiple controllers connected, then you need to notify multiple controllers. You may want to notify only certain controllers for whatever reasons, so you can select from the list of |
Hi @marcbaechinger, Thanks a lot for a quick answer! Now I use a list of ControllerInfo and notify all controllers. It solved my issue, thanks. I tried to use notifyChildrenChanged without the controller info argument, but it didn't work for a some reason. But now I use list of controllers and generally it's okay for me I have also the following question: Thanks in advance! |
Version
Media3 1.1.0
More version details
I have an issue when a MediaItem is added locally on the device, I call MediaLibraryService.MediaLibrarySession::notifyChildrenChanged with the updated library, however the browser in Android Auto isn't updated (the Queue is however updated).
Wondering if there are issues in this area, or if this is not the correct API to call?
Devices that reproduce the issue
Pixel 7 with Sony Android Auto head unit
Emulated Pixel 6 and Android Auto head unit emulator
I believe all other devices.
Devices that do not reproduce the issue
None
Reproducible in the demo app?
Not tested
Reproduction steps
Connect phone to Android Auto head unit
Add an item to the phone's media database
Call the MediaLibraryService.MediaLibrarySession::notifyChildrenChanged API
open the Browser on Android Auto
Notice new stream is not displayed.
Expected result
New stream should be displayed.
Actual result
New stream is not displayed.
Media
N/A
Bug Report
adb bugreport
to dev.exoplayer@gmail.com after filing this issue.The text was updated successfully, but these errors were encountered: