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

Android Auto #578

Open
wants to merge 36 commits into
base: redesign
Choose a base branch
from
Open

Android Auto #578

wants to merge 36 commits into from

Conversation

puff
Copy link

@puff puff commented Jan 17, 2024

Adds basic Android Auto and Android Automotive support.

Features:

  • Browse & play albums, playlists, and albums in each genre.
  • Artists start an instant mix when playing them.
  • Jump to albums/artists by letter (click A-Z on scroll bar).
  • Seek forwards and backwards in songs.
  • Skip to previous and next songs in queue, as well as directly to any song in the queue.
  • Shuffle button
  • Offline mode support.
    • Even outside of offline mode, it will try to use downloaded items before going online, unless it is fetching all items for root category (e.g. albums, artists, etc).
  • Search (voice and keyboard, only online)

Future features/TODO?

  • Repeat button
  • Favorite button
  • Instant mix button
  • Home tab
  • Genres should start an instant mix (not yet implemented in Finamp)
  • Offline mode artists/genres instant mix (not yet implemented in Finamp)

Currently, there is a hard limit of 100 items displayed so the app doesn't crash on large libraries.

// dev notes

  • Investigate why it doesn't switch to the Now Playing screen sometimes when starting playback, or takes a while before switching to it. This is likely because of the playback states being wrong.
  • Add pagination so we can show more than 100 items. This and This may be useful.

@jmshrv
Copy link
Owner

jmshrv commented Jan 18, 2024

This is awesome to see finally :)

Is the 100 item limit just an awkward thing with Android Auto or a thing that specifically needs to be fixed here? Jellyfin luckily makes pagination very easy when fetching from the server.

Artwork sounds like an annoying issue to fix though 🙃

@puff
Copy link
Author

puff commented Jan 18, 2024

Is the 100 item limit just an awkward thing with Android Auto or a thing that specifically needs to be fixed here? Jellyfin luckily makes pagination very easy when fetching from the server.

I put that limit because someone reported Android Auto crashing on large libraries (1000+) I do have an idea on how to add pagination to it, so I'll probably work on that tonight.

Artwork sounds like an annoying issue to fix though 🙃

Yeah, I'm gonna have to do some experimentation with that.

@Chaphasilor
Copy link
Collaborator

Thanks for the PR! I can try to take a look at it in the coming days, I just need to find a car to test with ^^

I'm guessing there is no built-in pagination support in Android Auto, and I don't think it would be super useful anyway.
I believe focussing on quick actions, like shuffle all songs, instant mixes and playing recently played items is a good approach.

Search and some voice interactions would definitely be useful down the road (hah), but if basic playback and browsing is working then I guess it qualifies for a beta.
Maybe it attracts some additional contributors :P

@puff
Copy link
Author

puff commented Jan 18, 2024

I just need to find a car to test with ^^

You can use the emulator! #24 (comment) It's very convenient for debugging too.

I'm guessing there is no built-in pagination support in Android Auto, and I don't think it would be super useful anyway. I believe focussing on quick actions, like shuffle all songs, instant mixes and playing recently played items is a good approach.

Yeah, I think I agree with you. Not sure that pagination should be a priority. I typically just use a playlist anyway lol.

Search and some voice interactions would definitely be useful down the road (hah)

Yeah, 100%. I wanted to incorporate that into this PR, but I was having an issue I didn't really understand where it crashes when searching. EDIT: fixed the crash, was a problem with changing the package name

@Chaphasilor
Copy link
Collaborator

Yeah I know about the emulator and have used it last time, but in the end I want to make sure everything works on an actual car too ^^

Regarding voice, how flexible is that? Can it only be used for voice search, or could we enable custom actions, like saying "Start playing Hello by Adele" and it will start an instant mix?

@puff
Copy link
Author

puff commented Jan 18, 2024

Can it only be used for voice search, or could we enable custom actions, like saying "Start playing Hello by Adele" and it will start an instant mix?

I know you can do stuff like that with app actions. There's also a version for Android Auto/Automotive.

@Chaphasilor
Copy link
Collaborator

Chaphasilor commented Jan 18, 2024

The second link you provided seems to be geared towards navigation/Points-of-Interest apps. Here's the one for media apps: https://developer.android.com/training/cars/media#support_voice

Seems like it has everything we need. If the user indicates an item type (album, playlist, etc.) we can use that to deal with Jellyfin's poor search, and if they omit the type "Play some music" we could resort to shuffle all for now. Would you like to look into it? :)

@puff
Copy link
Author

puff commented Jan 19, 2024

Would you like to look into it? :)

Sure, I have some time the next few days so I'll see if I can understand this and get something usable.

@Chaphasilor
Copy link
Collaborator

Chaphasilor commented Jan 19, 2024

Awesome. Let me know if you need help with anything!

Edit: Just checked it out on the emulator, here are a few things I noticed:

  • Clicking an item in the "Genres" tab should start an instant mix for that genre. Searching for albums by genre is probably not used that often and not really suitable for driving anyway
    • Same thing for artists maybe?
  • Placeholder images (especially for artists) would be nice to make sure the item names are aligned
  • For albums and playlists I'd really like to either be prompted if I want to play in-order or shuffled (that might not be ideal while driving), or have a way in the settings to set default behavior
  • The sorting is a bit different than in the phone app, not sure if you can do anything about it. Names that start with symbols are sorted by the first non-symbol character in the car, but not on the phone. Examples are "...Baby one more time", "& Friends" is "F", ".me", "(Un)Commentary"
  • The jump-to-letter function is neat, but only works with the loaded items. Is there a way to load items on-demand, like is done with the new "fast scroller" on the phone app?
  • I think we definitely need a "home" tab/section. I'll see if I can get something going for the entire app, since the phone app is supposed to also get a home screen with the redesign
    • There's also a "for you" section in the Android Auto home screen music widget (swiping left on the music widget) where the items from the home screen could be shown. Right now it just shows first 3 items from the albums tab, although strangely in the sorting of the phone app (symbols first)
  • There's a display bug when shuffle is active. The order of the list is correct, but the index of the currently playing song is not shown correctly in the list (the bars next to the active song are on the wrong item). Did you do any implementation for the queue? Otherwise I'll have to dive into just_audio once again to see if I can fix it...

@puff
Copy link
Author

puff commented Jan 20, 2024

Thanks for the feedback, I agree with your observations here.

Clicking an item in the "Genres" tab should start an instant mix for that genre

I was thinking we could replace the genre tab with the home tab. Android Auto by default only allows 4 tabs, and genres don't have instant mixes (though we could shuffle albums within the genre).

For albums and playlists I'd really like to either be prompted if I want to play in-order or shuffled

I don't think this is possible, but a setting for the default would work.

The sorting is a bit different than in the phone app
The jump-to-letter function is neat, but only works with the loaded items

I don't think I can do anything about these :(, they seem to be built into Android Auto.

There's also a "for you" section...

Seems like this is related to that section, looks to be possible.

There's a display bug when shuffle is active

I've done a little troubleshooting, and I'm confused. It seems there may be a bug somewhere causing a conflict between just_audio and the new queue service?

queueIndex: event.currentIndex,

The queue service expects queueIndex to be the non-shuffled, original index:

_queueAudioSourceIndex = event.queueIndex ?? 0;
if (previousIndex != _queueAudioSourceIndex) {
int adjustedQueueIndex = (playbackOrder ==
FinampPlaybackOrder.shuffled &&
_queueAudioSource.shuffleIndices.isNotEmpty)
? _queueAudioSource.shuffleIndices.indexOf(_queueAudioSourceIndex)
: _queueAudioSourceIndex;

int adjustedQueueIndex = (playbackOrder == FinampPlaybackOrder.shuffled &&
_queueAudioSource.shuffleIndices.isNotEmpty)
? _queueAudioSource.shuffleIndices.indexOf(_queueAudioSourceIndex)
: _queueAudioSourceIndex;

I suspect somewhere in just_audio expects it to be the shuffled index and is sending it to the backend (ExoPlayer?), which results in the queue showing the wrong song as selected. To elaborate, the erroneous selected song in queue is in the position of the original (non-shuffled) index, which is why I suspect this.

Changing queueIndex to the shuffled index, and replacing both occurrences of adjustedQueueIndex in queue_service directly to _queueAudioSourceIndex fixes the issue, however I'm not sure of the implications of this.

music_player_background_task.dart

- queueIndex: event.currentIndex,
+ queueIndex: _player.shuffleModeEnabled && (shuffleIndices?.isNotEmpty ?? false) && event.currentIndex != null ? shuffleIndices!.indexOf(event.currentIndex!) : event.currentIndex,

queue_service.dart

-int adjustedQueueIndex = (playbackOrder == FinampPlaybackOrder.shuffled &&
-         _queueAudioSource.shuffleIndices.isNotEmpty)
-     ? _queueAudioSource.shuffleIndices.indexOf(_queueAudioSourceIndex)
-     : _queueAudioSourceIndex;
+ int adjustedQueueIndex = _queueAudioSourceIndex;

@Chaphasilor
Copy link
Collaborator

Chaphasilor commented Jan 21, 2024

We could reduce the number of tabs even further, with a home tab and a library tab? On the library tab we could have a grid with the different categories.

Genre mixes should be possible, at least there's a way to add a genre to a mix on the phone 🤔 But maybe that just adds all individual albums, not sure.

Can we change the sorting within Android Auto? If something like "Recently Added" is possible, then it should be possible to use the BaseItemDto's SortTitle too.

I'll take a look at the modifications to the queue service. just_audio is really opinionated and the layered architecture doesn't make things easier. I've struggled with it for many monhs 🙃

Edit: Genre instant mixes should be a thing, at least it works in the Jellyfin app. No idea why it's hidden in Finamp...

@Chaphasilor
Copy link
Collaborator

Chaphasilor commented Jan 21, 2024

@puff I managed to fix up the queue based on your proposed changes. There were a few other things I needed to adjust, and I fixed a bug that I introduced a while ago (items in next up were not being reported to the external/OS queue).
From my testing everything seems to work as it should, but please let me know if anything is off!
With the beta release approaching fast I would hate to break things, and there have been the strangest bugs with playback and queue management in the past...

@puff
Copy link
Author

puff commented Jan 22, 2024

We could reduce the number of tabs even further, with a home tab and a library tab?

Yeah, I like that idea.

Genre mixes should be possible

I didn't see a way already implemented in Finamp yet, but yeah it should be possible if the Jellyfin app has it.

Can we change the sorting within Android Auto?

Yes, we can sort the MediaItem list before returning from getChildren. While testing, I noticed the offline sort already used doesn't match output from the Jellyfin api when sorting names.

offlineSortedItems!.sort((a, b) {

Looks to be because the Jellyfin api is case-insensitive:
https://github.com/jellyfin/jellyfin/blob/7027bc00fc06314929011735e425763779cb4076/MediaBrowser.Controller/Entities/BaseItem.cs#L897

I've fixed it and added sort to Android Auto. It uses whatever sort option is selected in the phone app.
Before & After:
image image

@Chaphasilor
Copy link
Collaborator

Thanks for fixing that. There are some sorting-related issues that need to be tackled at some point anyway, like #444 or #581, so if you feel like taking a look while you're dealing with sorting anyway, that would be much appreciated.
Otherwise I'll try to fix it after some bigger issues have been fixed...

@puff
Copy link
Author

puff commented Jan 23, 2024

I managed to get artwork to work with offline items and (kind've) Android Automotive using the android_content_provider package.

The white icon in the middle is the placeholder art.
image

Only problem is that for Android Automotive, we can't use http(s) URIs. Because of this, we have to resolve them ourselves. My current implementation hangs until every item has a resolved image. I think the way to fix this is to offload it to the ContentProvider. For now, I've commented it out until I have a better solution.

@Chaphasilor
Copy link
Collaborator

Did you manage to take a look at the voice command stuff? Or is there anything that you need help with? :)

@puff
Copy link
Author

puff commented Feb 1, 2024

Did you manage to take a look at the voice command stuff? Or is there anything that you need help with? :)

I've been having some trouble with it. It doesn't default to finamp even when it's open (maybe I forgot to configure something though?). It really doesn't like to recognize the app's name, you have to say it slowly. Saying "album" or really anything else other than the name of the song/album makes it default to YouTube Music (probably your default music service) so extras is always null.

"play some music on finamp"
image
Seems like some generic phrases are stripped out, odd that the "on finamp" part wasn't though.

"play black ray on finamp"
image

If you want to try it out, you just need to add this to the manifest:

<intent-filter>
  <action android:name="android.media.action.MEDIA_PLAY_FROM_SEARCH" />
  <category android:name="android.intent.category.DEFAULT" />
</intent-filter>

and uncomment playFromSearch in music_player_background_task.dart

@Chaphasilor
Copy link
Collaborator

Sorry for not responding earlier, I was a bit busy. Just tested it out and didn't manage to invoke the playFromSearch() method even once, although I did manage to get it to recognize the app's name a few times. I did manage to open the app by saying "Play some music on Finamp", but I don't think that's related to the manifest changes.
If the problem would just be to properly recognize the app name, then this might help, if it still works. But I think the whole voice command stuff seems to be pretty flawed, with YouTube being the default for a lot of things even after explicitly not selecting a default in the Assistant settings.
I only tested with Google Assistant, not with Android Auto, but I'd expect results to stay the same.

I guess we should skip this feature for now, including regular search. Search is due for a rewrite soon, and it makes little sense to integrate the old version now...

@Chaphasilor
Copy link
Collaborator

I did take the build for a test drive this weekend, and even with only occasional skipping I managed to trigger the deadlock. Audio kept playing (although I think the queue jumped to a different spot), but the app didn't respond via the Android Auto UI nor via the car's physical media buttons.
Nothing surprising here.

I have an exam coming up on Friday, and I hope to get some work on Finamp done this weekend :)

@puff
Copy link
Author

puff commented Mar 8, 2024

I've worked around the issue by implementing the ContentProvider in Kotlin instead of flutter. This doesn't seem ideal though because we can't easily pass things to it from flutter. I think it's probably fine for our use case, since we don't need anything other than the artwork URL.

@Chaphasilor
Copy link
Collaborator

So that means everything should be working now? 😁
If so, I should be able to take a look at the merge conflicts tomorrow, and maybe we can push it with the next update!

@puff
Copy link
Author

puff commented Mar 9, 2024

Yeah, everything should be working now.

@Chaphasilor
Copy link
Collaborator

Sweet! I tested it an so far everything seems to be working great. Not a single crash or freeze yet.
I also made some more improvements to search, like using metadata/extras from previous voice searches to search and prioritize matches when using the "Search results" button on the player screen. Finamp now also searches playlists and songs if the item type is unspecified ("play xyz", "play song xyz", or "play playlist xyz" all have the same extras without metadata), and if a matching playlist is found, that is prioritized over matching songs. If the user wanted to play a song with that name instead, they can then use the "Search results" button.
So everything should be relatively flexible now.

I also finished up the merge, currently testing that.

@Chaphasilor
Copy link
Collaborator

Would be nice if you could do another round of testing, focusing especially on the offline stuff you implemented, since I tried to migrate everything to the new system :)
I'll also try to test, and then we can merge it (maybe tomorrow?)

@puff
Copy link
Author

puff commented Mar 10, 2024

Had to fix includeItemTypes in getBaseItems(), but now it looks like everything is working again.

@Chaphasilor
Copy link
Collaborator

I noticed that when you tried to use downloaded items by default even if online, it resulted in Finamp only playing downloaded songs from i.e. albums, even though we could just fetch the other songs from the server.
Since the stream URL is automatically resolved from downloads if available when creating a MediaItem for the queue, I changed this so Finamp always prefers the server info.

Maybe we could fall back to only downloaded items in case fetching the items from the server fails to due a spotty connection, but I'm not sure if we want to do that in general. We don't do it in the regular app after all, but maybe driving justifies the changed behavior? What do you think?

@Chaphasilor
Copy link
Collaborator

@puff do you know of any way to get the currently selected tab in such a way that we could use it do decide what to search for?
E.g. if on the albums tab and clicking the search icon, albums will be searched, but on the artists tab artists.

I'm asking because there are many users that listen to albums, and there currently is no good way to pick albums that are not in the first 100.

@puff
Copy link
Author

puff commented Mar 10, 2024

Maybe we could fall back to only downloaded items...

I think we should just retry fetching it until we get a response, or change the timeout to be longer.

do you know of any way to get the currently selected tab

We could store it during the getChildren() call. If it's a root collection, it should be our current tab.

@Chaphasilor
Copy link
Collaborator

We could store it during the getChildren() call. If it's a root collection, it should be our current tab.

That was my initial idea as well, but there are also cases where we can enter search (i.e. from the player screen), where it's not clear which tab should be used. Ideally we should just search for all item types, but I think that's a bit too involved for now...

@Chaphasilor
Copy link
Collaborator

I also noticed a lot of these errors in the logs:

I/flutter ( 5291): Error loading artUri: Invalid argument(s): Unsupported scheme 'content' in URI content://com.unicornsonlsd.finamp/data/user/0/com.unicornsonlsd.finamp.debug/files/images/7a1c3729c8866e731912b055ccef98d5.jpg
I/flutter ( 5291): #0      _HttpClient._openUrl (dart:_http/http_impl.dart:2756:9)
I/flutter ( 5291): #1      _HttpClient.openUrl (dart:_http/http_impl.dart:2623:7)
I/flutter ( 5291): #2      IOClient.send (package:http/src/io_client.dart:117:38)
I/flutter ( 5291): #3      HttpFileService.get (package:flutter_cache_manager/src/web/file_service.dart:37:44)
I/flutter ( 5291): #4      WebHelper._download (package:flutter_cache_manager/src/web/web_helper.dart:115:24)
I/flutter ( 5291): #5      WebHelper._updateFile (package:flutter_cache_manager/src/web/web_helper.dart:97:28)
I/flutter ( 5291): <asynchronous suspension>
I/flutter ( 5291): #6      WebHelper._downloadOrAddToQueue (package:flutter_cache_manager/src/web/web_helper.dart:65:7)
I/flutter ( 5291): <asynchronous suspension>

Any idea what's going on? I suspect it's something to do with the content provider...

@Chaphasilor
Copy link
Collaborator

I'll also try to tackle offline voice search tomorrow. That doesn't work at all yet, but should be doable.

- removes old limitations
@puff
Copy link
Author

puff commented Mar 11, 2024

I also noticed a lot of these errors in the logs: ...
Any idea what's going on? I suspect it's something to do with the content provider...

Seems like an incompatibility with our content provider and audio_service's preloadArtwork option. The _loadArtwork method used with preloading artwork in the queue doesn't handle content:// URIs. They are only handled when the current media item is being loaded. It'd probably require a small refactor to add support for preloading content:// artworks in audio_service (on our side too for efficiency), so I think we should just disable preloadArtwork.

but there are also cases where we can enter search (i.e. from the player screen), where it's not clear which tab should be used.

I think it should just prioritize certain types in the results.
Playlists -> Artists -> Albums -> Songs is what Jellyfin orders by in their app when searching (it doesn't include Genres), so I think it would make sense here too. includeItemTypes is comma-delimited so that should make it a little easier to search all types.

@Chaphasilor
Copy link
Collaborator

Ah, yeah I enabled that option recently because I was hoping it would avoid the artist sometimes not loading correctly in the Android notification. I'll disable it for now.

I'll see what I can do about the search.

@Chaphasilor Chaphasilor added feature A new feature and removed enhancement Improves an existing feature labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants