-
Notifications
You must be signed in to change notification settings - Fork 217
Misc Fixes #1467
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
base: redesign
Are you sure you want to change the base?
Misc Fixes #1467
Conversation
…uilder menu entry, update mix-related icons
this helps to recover from queue-related errors, if those ever appear
- this fixes #1466
…ack action row. Prevent use of -1 as initial row position.
…evious tracks open and a short queue.
- still makes excessive requests, but at least it doesn't break anymore
- this is disabled by default because of known incorrect negotiations between ExoPlayer and the device, so we default to off: > IMPORTANT: activation of audio offload depends on a negotiation between ExoPlayer and the device to determine whether offload can be supported for a given format and with given constraints (gapless, speed change). However, several instances have been reported where the device incorrectly confirms support for audio offload when it doesn't, and this can result in buggy audio playback. Therefore, it is advised that you programmatically enable audio offload only on device/OS combinations that you have tested and verified to work. (from https://pub.dev/documentation/just_audio/latest/just_audio/AndroidAudioOffloadPreferences-class.html) - even if enabled, we only actually offload if both gapless and speed control are (supposedly) available, for full compatibility
the first three modes now source new tracks, while the other modes "recycle" tracks from the source or current queue
…properly on stop if radio is enabled (and cache is filled)
|
I noticed your note about rearanging tracks while shuffled. Can't you just use the same trick as the restore, where you override our shuffle order with updated indices and then call shuffle? Does player fail to apply the update or something? |
…t synced with media notification
…orce chosen radio mode before starting queue
|
You're right, overwriting the Also, I'm not entirely sure if the |
| } finally { | ||
| if (localRadioState?.isStillValid() ?? false) { | ||
| _radioCacheStateStream.add(localRadioState?.copyWith(fallbackMode: fallbackMode)); | ||
| if (localRadioState != null && identical(localRadioState, _radioCacheStateStream.valueOrNull)) { |
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.
We shouldn't be updating the localRadioState if we have an overrideSeed. But if we don't, we should be able to call .updateAlbumMixFallbackMode unconditionally.
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.
But that would mean that if we end up in a fallback for the initial queue generation (which could happen e.g. if the user only has singles by that artist), then we'd "forget" about that and only remember the fallback for the next scheduled fetch of radio tracks? Nothing terrible of course.
But I guess that won't work anyway since we're invalidating the cache after generating the initial queue track in startRadioPlayback()
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.
Yeah, I don't think there's a way we could persist the fallback mode from new radio queues without just returning the fallback mode and letting the caller handle persisting, because the new queue cache state isn't fully set up early enough. Doing this would complicate the interface, but it would also let the album fallback persisting be cleanly handled along with the generation lock and would get all cache state back out of generateTracks(). Probably not worthwhile?
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.
Let's see how the requirements for radio (new modes, etc.) develop based on feedback, and then we can probably better judge which optimizations are worthwhile and which patterns we actually need to support.
Changes
Various fixes to close a few more issues with the next release...
Todo before merging
Fix MediaSources assertion errorSeems to indeed be related to download errors and not a flukeFinampSettingsdesktopShuffleWarning)Nope, still broken. Updating the_shuffleOrderdoesn't persist, and using_player.moveAudioSource()seems to overwrite indices, so that un-shuffling isn't possible anymorerestartDownloadsgetting called when opening the downloads screen, so unless that happens downloads don't sync on startupresyncto be called?Try to hook up audio session to system equalizerWill have to wait for another PRRelated Issues