-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add support for Android Auto #9592
base: dev
Are you sure you want to change the base?
Conversation
a4760b6
to
4a28129
Compare
Kudos, SonarCloud Quality Gate passed! |
Interesting! Is it possible that using |
I'm not familiar with these issues, but I can try testing them if they are reproduced with this patch. Can you point me in their direction?
I wanted to start with media controls, because it seems useful in its own. I can set a play queue in advance and control it from the car. Later I would also like to add see browsing. I think that for Auto it should be limited - maybe things like saved playlists and search history? There's also an API for search queries I am hoping would allow integration with voice search.
Yes, I think the documentation had something to say about it - that the service shouldn't start playing when it starts. It should only start playing when you get a callback from the MediaBrowserServiceCompat. But the service can tell how if it gets started by another UI or by a media browser, can't it? I had a related question: at first I thought it would be better to create a separate service. I then saw that it is easier to use the existing player service, but do you think it is the right approach? Another small problem I noticed with the pull request in its current state, is that the Android Auto's rendering of the media session notification shows the progress circle around the pause button in gray, rather than showing the current position. This is weird, because the notification on my phone does show a seekable progress bar. |
You actually answered my question: the issue with the notification I was referring to is the one related to no seek bar showing up. And apparently no,
Yeah that's fine, you can take care of the rest later if you want. Integration with voice search would also be really nice.
Yes it can, but atm I think that part is broken, because there are user reports of the player popping up from nowhere or crashing at random when connecting or disconnecting bluetooth.
Here are a few considerations:
So I think all in all it's best to keep it in |
I'm trying to implement some simple browsing, but I got into lifecycle issues. The How can I handle this? Perhaps closing the notification shouldn't close the entire player, and instead, stop and close the notification UI? |
Closing the notification should definitely release the player. Afterwards any incoming intent to start the player should create a new player, while other intents should be treated separately (and NOT start the player). But it's a big mess. If you can't find out a way to do this, maybe go with the option of creating a separate service. |
Makes sense, I tried doing something along these lines, though I don't differentiate between the intents yet. The code also still creates the player when the service is created, but I can change it as you wrote. I also tried adding some browsing capabilities, but for some reason it only worked on the desktop HUD and not with my car. I'll have to check that out. |
OT: google also bought the "waze navigation" map app you are using, but they just don't show it anywhere as then, users will start abandoning even waze.
google and others are fiercely anti-competitive and purchase any company doing better than them. Remeber, |
Yes, that's what I've seen too. In Spotify for example the progress is shown as a circle that can't be manipulated, rather than a seek bar. Still, the circle shows playback progress with two colors. I'll try capturing a screenshot sometime.
I know, that's too bad because I liked Waze. Up until now, Google haven't merged it, even if they haven't grown it much. I think it was pretty good at predicting traffic, which is something that requires a popular service, so I might end up with Google Maps if they merge them. |
oh yeah, that thin two coloring of the circle's border stroke, right? i can imagine that. OT:
I know, that's too bad because I liked Waze. same, i found this too while searching for alternates to google maps some years ago. |
Hi, I've updated the pull request and tried to keep the MediaSession and its connector in the new helper object so that they outlive the player. I also tried making the media Ids more URI-like, inspired by VLC. By the way, I also tried implementing the APIs for voice search (I think it should just be onPlayFromSearch/onPrepareFromSearch), but it didn't work. As far as I understand, Google Assistant only works with apps from the Google Play Store. I can't even invoke "open NewPipe", and this shouldn't require any code. |
bc2ba86
to
bec3408
Compare
bec3408
to
59f3fd8
Compare
Apologies for my unfamiliarity with your development process, but is this likely to be included in the next release? It's a feature I'm very much interested in and looks to the untrained eye like it's ready to be merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for this feature!
Please apply the following changes and rebase your branch on top of the dev
one, so we can test the changes on the latest release. I am trusting you for your usage and your implementation of the feature.
However, there is an issue with this PR: the single ExoPlayer library is being discontinued, as the library has now its place in the Media3 one.
This change removes the media session extension, as media session management is directly done by Media3, so this code would have to be rewritten soon if and when we migrate to Media3 the app.
Even if this migration may happen soon, I don't think we will migrate to Media3 in the next release and I think we can merge this pull request, especially because of the feature it gives to the app.
Finally, I see several changes in this PR related to the player service, especially on command interception. Wouldn't your changes fix the player crash issues we currently have when using a media command on a Bluetooth device and maybe the player ANRs?
app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserConnector.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserConnector.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserConnector.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/player/mediabrowser/MediaBrowserConnector.java
Outdated
Show resolved
Hide resolved
Thanks for your feedback. I'm trying to rebase, but I came across a build issue with the updated dev branch. I'm getting the following error:
Do you know what causes this? Regarding moving to media3, I understand it can make most of this code obsolete. Once you are done with the move to media3, I could try to test its support for android auto and see when changes are needed. |
The recurrent Jitpack artifact |
Is there an issue about this I can read? Maybe I could try and reproduce this problem and see if my patches help. |
I tried reproducing #9095, and I believe it still happens with this PR. I tried using my car's console without Android Auto, only functioning as a Bluetooth headset. If I play a video on NewPipe, close it, and press "Play" on my car's console, the application crashes. I got the following in the crash log:
So I think closing the player must have left the service running but with the player object null. I haven't tested the |
Ialso tried the build from CI. does not work with Android 13 and Peugeot e2008. The Newpipe icon is visible in the launcher, but it does not even display the UI framework in the screenshot by asentes2 above. Please let me know if there's logs I could pull to help out. I'd really like to get this working. AFAIK this would be the first Android Auto capable player with You Tube music out there. |
Hi,
Does this work to change provider with adb on non root device ?
|
i just tested it to make newpipe-androidauto as google play app with adb, app say its installed from google play |
Sorry, forgot to mention that it does require root making it even more niche case. Rooting a daily driver phone leads to issues with banking apps etc. I have old phone separately just for AA use. Not really a valid use case for average users I would assume, although Newpipe users are not exactly mainstream users. |
"> + disposePrepareOrPlayCommands()
As far as I understand, Google Assistant uses these APIs when it acts as a controller of a media application (e.g., following a command like "play music on NewPipe". Unfortunately, I believe only Play Store apps can be controlled by Google Assistant, so there wasn't much point in implementing them." To me that looks like the necessary API end points have not been implemented in NewPipe, so just changing the app source does nothing. The code would also need to have support for it, so that Google Assistant could be used to control it via voice. |
@asentes2, @notbasa, I believe I fixed the issue. It seems that the onBind fix was removed at some point from the PR. Once the update version builds in CI, could you test again? @snaik20, @Stypox as far as I understand, the custom IBinder that PlayerService has doesn't forward the MediaBrowserService messages to the superclass's implementation, so to fix it I had to check the intent and decide on which IBinder to return according to the intent. If there's a better or common way of implementing this, I'd be happy to learn. |
Regarding changing the app to be listed as if it's from Google Play store, I think it's worth checking, though I don't know if that would be enough for all of Google Assistant's functionality to work. Anyway, I would suggest addressing this in a separate PR, if the maintainers agree. |
Quality Gate passedIssues Measures |
OMG ITS WORKING |
I can confirm that it now works on my Android 14 & Peugeot e2008 combination as well. Great work and thank you all. |
Thanks @notbasa for testing. When you say suggest a way to browse content, you mean in addition to the saved playlists and history? Something like the home screen of the app? |
Yes. Currently the main screen is just empty. It would be great if if could show the same/similar stuff like the Youtube Music app, so recommended lists & personal library contents. (Not really sure what the right terms are). However showing just the same content as on the Newpipe main screen does not seem to make much sense, as at least for me it shows mostly just latest YT content and not sure if there's a setting somewhere to make it focus on music content. |
That makes sense, though I agree it sounds like a separate feature. The current version of this PR does show the saved playlists, though that would be empty for a new install (and the CI version would probably be considered as such). I don't think NewPipe normally plays local music files, right? So that part wouldn't really fit, but it does make sense to show some suggestions based on what is currently playing or has been played recently, maybe similarly to the auto-play feature. |
Fully agree. Just to clarify by personal library files I was referring to music uploaded to YouTube Music (or moved from Google Music when it closed) like YT Music does, not local files on the phone. Anyway agree it is out of scope for this ticket. |
What is it?
Description of the changes in your PR
Issues, bugs, and missing features
Future enhancements
Screenshots/Screen Record
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence