From 21e4293adb71062483fbd8aa160d1c0ee89193b7 Mon Sep 17 00:00:00 2001 From: birdbird <6892457-tzugen@users.noreply.gitlab.com> Date: Thu, 6 Jul 2023 20:49:29 +0000 Subject: [PATCH 1/6] Merge branch 'root' into 'develop' Avoid rare NPE See merge request ultrasonic/ultrasonic!1073 (cherry picked from commit 7209779b64820572910059fdd24342bb73734b54) ecee57e1 Avoid rare NPE --- .../ultrasonic/activity/NavigationActivity.kt | 2 +- .../ultrasonic/fragment/SettingsFragment.kt | 2 +- .../org/moire/ultrasonic/util/Storage.kt | 33 ++++++++++++------- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/NavigationActivity.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/NavigationActivity.kt index 76df1ea5..160677b1 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/NavigationActivity.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/NavigationActivity.kt @@ -306,7 +306,7 @@ class NavigationActivity : AppCompatActivity() { Storage.reset() lifecycleScope.launch(Dispatchers.IO) { - Storage.ensureRootIsAvailable() + Storage.checkForErrorsWithCustomRoot() } setMenuForServerCapabilities() diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SettingsFragment.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SettingsFragment.kt index 700bde05..d007c399 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SettingsFragment.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SettingsFragment.kt @@ -344,7 +344,7 @@ class SettingsFragment : // Clear download queue. mediaPlayerManager.clear() Storage.reset() - Storage.ensureRootIsAvailable() + Storage.checkForErrorsWithCustomRoot() } private fun setDebugLogToFile(writeLog: Boolean) { diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Storage.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Storage.kt index f6b5b9d2..933730f4 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Storage.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Storage.kt @@ -22,19 +22,23 @@ import timber.log.Timber object Storage { val mediaRoot: ResettableLazy = ResettableLazy { - getRoot()!! + val ret = getRoot() + rootNotFoundError = ret.second + ret.first } + var rootNotFoundError: Boolean = false + fun reset() { StorageFile.storageFilePathDictionary.clear() StorageFile.notExistingPathDictionary.clear() mediaRoot.reset() + rootNotFoundError = false Timber.i("StorageFile caches were reset") } - fun ensureRootIsAvailable() { - val root = getRoot() - if (root == null) { + fun checkForErrorsWithCustomRoot() { + if (rootNotFoundError) { Settings.customCacheLocation = false Settings.cacheLocationUri = "" Util.toast(UApp.applicationContext(), R.string.settings_cache_location_error) @@ -98,18 +102,25 @@ object Storage { return success } - private fun getRoot(): AbstractFile? { + private fun getRoot(): Pair { return if (Settings.customCacheLocation) { - if (Settings.cacheLocationUri.isBlank()) return null + if (Settings.cacheLocationUri.isBlank()) return Pair(getDefaultRoot(), true) val documentFile = DocumentFile.fromTreeUri( UApp.applicationContext(), Uri.parse(Settings.cacheLocationUri) - ) ?: return null - if (!documentFile.exists()) return null - StorageFile(null, documentFile.uri, documentFile.name!!, documentFile.isDirectory) + ) ?: return Pair(getDefaultRoot(), true) + if (!documentFile.exists()) return Pair(getDefaultRoot(), true) + Pair( + StorageFile(null, documentFile.uri, documentFile.name!!, documentFile.isDirectory), + false + ) } else { - val file = File(FileUtil.defaultMusicDirectory.path) - JavaFile(null, file) + Pair(getDefaultRoot(), false) } } + + private fun getDefaultRoot(): JavaFile { + val file = File(FileUtil.defaultMusicDirectory.path) + return JavaFile(null, file) + } } From 1fa5f4c2f8d3d2265207248f0067d6d66bd2f9be Mon Sep 17 00:00:00 2001 From: birdbird <6892457-tzugen@users.noreply.gitlab.com> Date: Mon, 24 Jul 2023 18:38:08 +0000 Subject: [PATCH 2/6] Fix unpin --- .../ultrasonic/fragment/SearchFragment.kt | 6 +- .../ultrasonic/playback/PlaybackService.kt | 2 +- .../ultrasonic/service/DownloadService.kt | 103 +++++++++++++----- .../ultrasonic/subsonic/DownloadHandler.kt | 12 +- 4 files changed, 89 insertions(+), 34 deletions(-) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SearchFragment.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SearchFragment.kt index cd4d1104..4d2ffe2f 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SearchFragment.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SearchFragment.kt @@ -140,7 +140,11 @@ class SearchFragment : MultiListFragment(), KoinComponent { private fun downloadBackground(save: Boolean, songs: List) { val onValid = Runnable { networkAndStorageChecker.warnIfNetworkOrStorageUnavailable() - DownloadService.download(songs.filterNotNull(), save) + DownloadService.download( + songs.filterNotNull(), + save = save, + updateSaveFlag = true + ) } onValid.run() } diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/playback/PlaybackService.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/playback/PlaybackService.kt index 7965558e..952b8212 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/playback/PlaybackService.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/playback/PlaybackService.kt @@ -328,7 +328,7 @@ class PlaybackService : ).map { it.toTrack() } launch { - DownloadService.download(nextSongs, save = false, isHighPriority = true) + DownloadService.download(nextSongs, isHighPriority = true) } } diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadService.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadService.kt index 808c3bbe..dff3d5b3 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadService.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadService.kt @@ -274,46 +274,26 @@ class DownloadService : Service(), KoinComponent { @Synchronized fun download( tracks: List, - save: Boolean, - isHighPriority: Boolean = false + save: Boolean = false, + isHighPriority: Boolean = false, + updateSaveFlag: Boolean = false ) { CoroutineScope(Dispatchers.IO).launch { - // First handle and filter out those tracks that are already completed - var filteredTracks: List - if (save) { - tracks.filter { Storage.isPathExists(it.getCompleteFile()) }.forEach { track -> - Storage.getFromPath(track.getCompleteFile())?.let { - Storage.renameOrDeleteIfAlreadyExists(it, track.getPinnedFile()) - postState(track, DownloadState.PINNED) - } - } - filteredTracks = tracks.filter { !Storage.isPathExists(it.getPinnedFile()) } + // Remove tracks which are already downloaded and update the save flag + // if needed + var filteredTracks = if (updateSaveFlag) { + setSaveFlagForTracks(save, tracks) } else { - tracks.filter { Storage.isPathExists(it.getPinnedFile()) }.forEach { track -> - Storage.getFromPath(track.getPinnedFile())?.let { - Storage.renameOrDeleteIfAlreadyExists(it, track.getCompleteFile()) - postState(track, DownloadState.DONE) - } - } - filteredTracks = tracks.filter { !Storage.isPathExists(it.getCompleteFile()) } - } - - // Update Pinned flag of items in progress - downloadQueue.filter { item -> tracks.any { it.id == item.id } } - .forEach { it.pinned = save } - tracks.forEach { - activeDownloads[it.id]?.downloadTrack?.pinned = save - } - tracks.forEach { - failedList[it.id]?.pinned = save + removeDownloadedTracksFromList(tracks) } + // Remove tracks which are currently downloading filteredTracks = filteredTracks.filter { !downloadQueue.any { i -> i.id == it.id } && !activeDownloads.containsKey(it.id) } - // The remainder tracks should be added to the download queue + // The remaining tracks should be added to the download queue // By using the counter we ensure that the songs are added in the correct order var priority = 0 val tracksToDownload = @@ -334,6 +314,69 @@ class DownloadService : Service(), KoinComponent { } } + private fun removeDownloadedTracksFromList(tracks: List): List { + return tracks.filter { track -> + val pinnedFile = Storage.getFromPath(track.getPinnedFile()) + val completeFile = Storage.getFromPath(track.getCompleteFile()) + + completeFile?.let { + postState(track, DownloadState.DONE) + false + } + pinnedFile?.let { + postState(track, DownloadState.PINNED) + false + } + true + } + } + + private fun setSaveFlagForTracks( + shouldPin: Boolean, + tracks: List + ): List { + // Walk through the tracks. If a track is pinned or complete and needs to be changed + // to the other state, rename it, but don't return it, thereby excluding it from + // further processing. + // If it is neither pinned nor saved, return it, so that it can be processed. + val filteredTracks: List = tracks.map { track -> + val pinnedFile = Storage.getFromPath(track.getPinnedFile()) + val completeFile = Storage.getFromPath(track.getCompleteFile()) + + if (shouldPin) { + pinnedFile?.let { + null + } + completeFile?.let { + Storage.renameOrDeleteIfAlreadyExists(it, track.getPinnedFile()) + postState(track, DownloadState.PINNED) + null + } + } else { + completeFile?.let { + null + } + pinnedFile?.let { + Storage.renameOrDeleteIfAlreadyExists(it, track.getCompleteFile()) + postState(track, DownloadState.DONE) + null + } + } + track + } + + // Update Pinned flag of items in progress + downloadQueue.filter { item -> tracks.any { it.id == item.id } } + .forEach { it.pinned = shouldPin } + tracks.forEach { + activeDownloads[it.id]?.downloadTrack?.pinned = shouldPin + } + tracks.forEach { + failedList[it.id]?.pinned = shouldPin + } + return filteredTracks + } + fun requestStop() { val context = UApp.applicationContext() val intent = Intent(context, DownloadService::class.java) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/subsonic/DownloadHandler.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/subsonic/DownloadHandler.kt index 55bceca2..2bae06e8 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/subsonic/DownloadHandler.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/subsonic/DownloadHandler.kt @@ -53,8 +53,16 @@ class DownloadHandler( withContext(Dispatchers.Main) { // If we are just downloading tracks we don't need to add them to the controller when (action) { - DownloadAction.DOWNLOAD -> DownloadService.download(tracksToDownload, false) - DownloadAction.PIN -> DownloadService.download(tracksToDownload, true) + DownloadAction.DOWNLOAD -> DownloadService.download( + tracksToDownload, + save = false, + updateSaveFlag = true + ) + DownloadAction.PIN -> DownloadService.download( + tracksToDownload, + save = true, + updateSaveFlag = true + ) DownloadAction.UNPIN -> DownloadService.unpin(tracksToDownload) DownloadAction.DELETE -> DownloadService.delete(tracksToDownload) } From 4f55a2a4a5441c458e4a14720ba0ca6bc2e83f30 Mon Sep 17 00:00:00 2001 From: birdbird <6892457-tzugen@users.noreply.gitlab.com> Date: Mon, 24 Jul 2023 18:57:29 +0000 Subject: [PATCH 3/6] Fix an exception when removeIncompleteTracksFromPlaylist() could be called on the wrong thread. --- .../ultrasonic/service/MediaPlayerManager.kt | 43 ++++++++++--------- .../org/moire/ultrasonic/service/RxBus.kt | 9 +--- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerManager.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerManager.kt index d1ffd067..2a4cbbdc 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerManager.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerManager.kt @@ -181,19 +181,22 @@ class MediaPlayerManager( createMediaController(onCreated) - rxBusSubscription += RxBus.activeServerChangingObservable.subscribe { oldServer -> - if (oldServer != OFFLINE_DB_ID) { - // When the server changes, the playlist can retain the downloaded songs. - // Incomplete songs should be removed as the new server won't recognise them. - removeIncompleteTracksFromPlaylist() - DownloadService.requestStop() + rxBusSubscription += RxBus.activeServerChangingObservable + // All interaction with the Media3 needs to happen on the main thread + .subscribeOn(RxBus.mainThread()) + .subscribe { oldServer -> + if (oldServer != OFFLINE_DB_ID) { + // When the server changes, the playlist can retain the downloaded songs. + // Incomplete songs should be removed as the new server won't recognise them. + removeIncompleteTracksFromPlaylist() + DownloadService.requestStop() + } + if (controller is JukeboxMediaPlayer) { + // When the server changes, the Jukebox should be released. + // The new server won't understand the jukebox requests of the old one. + switchToLocalPlayer() + } } - if (controller is JukeboxMediaPlayer) { - // When the server changes, the Jukebox should be released. - // The new server won't understand the jukebox requests of the old one. - switchToLocalPlayer() - } - } rxBusSubscription += RxBus.activeServerChangedObservable.subscribe { val jukebox = activeServerProvider.getActiveServer().jukeboxByDefault @@ -204,19 +207,19 @@ class MediaPlayerManager( isJukeboxEnabled = jukebox } - rxBusSubscription += RxBus.throttledPlaylistObservable.subscribe { - // Even though Rx should launch on the main thread it doesn't always :( - mainScope.launch { + rxBusSubscription += RxBus.throttledPlaylistObservable + // All interaction with the Media3 needs to happen on the main thread + .subscribeOn(RxBus.mainThread()) + .subscribe { serializeCurrentSession() } - } - rxBusSubscription += RxBus.throttledPlayerStateObservable.subscribe { - // Even though Rx should launch on the main thread it doesn't always :( - mainScope.launch { + rxBusSubscription += RxBus.throttledPlayerStateObservable + // All interaction with the Media3 needs to happen on the main thread + .subscribeOn(RxBus.mainThread()) + .subscribe { serializeCurrentSession() } - } rxBusSubscription += RxBus.shutdownCommandObservable.subscribe { clear(false) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RxBus.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RxBus.kt index 9c87c519..9912cf0d 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RxBus.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RxBus.kt @@ -1,8 +1,8 @@ package org.moire.ultrasonic.service -import android.os.Looper import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import io.reactivex.rxjava3.core.Observable +import io.reactivex.rxjava3.core.Scheduler import io.reactivex.rxjava3.disposables.CompositeDisposable import io.reactivex.rxjava3.disposables.Disposable import io.reactivex.rxjava3.subjects.PublishSubject @@ -12,14 +12,9 @@ import org.moire.ultrasonic.domain.Track class RxBus { - /* - * TODO: mainThread() seems to be not equal to the "normal" main Thread, so it causes - * a lot of often unnecessary thread switching. It looks like observeOn can actually - * be removed in many cases - */ companion object { - private fun mainThread() = AndroidSchedulers.from(Looper.getMainLooper()) + fun mainThread(): Scheduler = AndroidSchedulers.mainThread() val shufflePlayPublisher: PublishSubject = PublishSubject.create() From 18fd8f64c60527383e407b6299bbfb0d8f777755 Mon Sep 17 00:00:00 2001 From: tzugen Date: Mon, 24 Jul 2023 21:14:22 +0200 Subject: [PATCH 4/6] Release 4.6.3 Features: - Search is accessible through a new icon on the main screen - Modernize Back Handling - Reenable R8 Code minification - Add a "Play Random Songs" shortcut Bug fixes: - Fix a few crashes - Avoid triggering a bug in Supysonic - Readd the "Star" button to the Now Playing screen - Fix a rare crash when shuffling playlists with duplicate entries - Fix a crash when choosing "Play next" on an empty playlist. - Tracks buttons flash a scrollbar sometimes in Android 13 - Fix EndlessScrolling in genre listing - Couldn't delete a track when shuffle was active --- .../metadata/android/en-US/changelogs/126.txt | 15 +++++++++++++++ ultrasonic/build.gradle | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 fastlane/metadata/android/en-US/changelogs/126.txt diff --git a/fastlane/metadata/android/en-US/changelogs/126.txt b/fastlane/metadata/android/en-US/changelogs/126.txt new file mode 100644 index 00000000..250031e5 --- /dev/null +++ b/fastlane/metadata/android/en-US/changelogs/126.txt @@ -0,0 +1,15 @@ +Features: +- Search is accessible through a new icon on the main screen +- Modernize Back Handling +- Reenable R8 Code minification +- Add a "Play Random Songs" shortcut + +Bug fixes: +- Fix a few crashes +- Avoid triggering a bug in Supysonic +- Readd the "Star" button to the Now Playing screen +- Fix a rare crash when shuffling playlists with duplicate entries +- Fix a crash when choosing "Play next" on an empty playlist. +- Tracks buttons flash a scrollbar sometimes in Android 13 +- Fix EndlessScrolling in genre listing +- Couldn't delete a track when shuffle was active \ No newline at end of file diff --git a/ultrasonic/build.gradle b/ultrasonic/build.gradle index 65f4863d..9787150a 100644 --- a/ultrasonic/build.gradle +++ b/ultrasonic/build.gradle @@ -9,8 +9,8 @@ android { defaultConfig { applicationId "org.moire.ultrasonic" - versionCode 125 - versionName "4.6.2" + versionCode 126 + versionName "4.6.3" minSdkVersion versions.minSdk targetSdkVersion versions.targetSdk From fc637166bb8d3094c480b3ad6b60411013b3a8d2 Mon Sep 17 00:00:00 2001 From: tzugen Date: Mon, 24 Jul 2023 23:20:44 +0200 Subject: [PATCH 5/6] Fix inject --- .../main/kotlin/org/moire/ultrasonic/util/CacheCleaner.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/CacheCleaner.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/CacheCleaner.kt index 3ec2cee6..83939ab8 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/CacheCleaner.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/CacheCleaner.kt @@ -235,11 +235,9 @@ class CacheCleaner : CoroutineScope by CoroutineScope(Dispatchers.IO), KoinCompo private fun findFilesToNotDelete(): Set { val filesToNotDelete: MutableSet = HashSet(5) - val mediaController = inject( - MediaPlayerManager::class.java - ) + val mediaPlayerManager: MediaPlayerManager by inject() - val playlist = mainScope.future { mediaController.value.playlist }.get() + val playlist = mainScope.future { mediaPlayerManager.playlist }.get() for (item in playlist) { val track = item.toTrack() filesToNotDelete.add(track.getPartialFile()) From 6bc09854ac3a63ca1ef2929eabb2978f8b0c16be Mon Sep 17 00:00:00 2001 From: tzugen Date: Tue, 25 Jul 2023 12:11:26 +0200 Subject: [PATCH 6/6] Fix wrong thread in throttled observers --- .../moire/ultrasonic/service/MediaPlayerManager.kt | 5 ++++- .../main/kotlin/org/moire/ultrasonic/service/RxBus.kt | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerManager.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerManager.kt index 2a4cbbdc..93588750 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerManager.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerManager.kt @@ -248,7 +248,10 @@ class MediaPlayerManager( mediaControllerFuture = MediaController.Builder( context, sessionToken - ).buildAsync() + ) + // Specify mainThread explicitely + .setApplicationLooper(Looper.getMainLooper()) + .buildAsync() mediaControllerFuture?.addListener({ controller = mediaControllerFuture?.get() diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RxBus.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RxBus.kt index 9912cf0d..8524fdff 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RxBus.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RxBus.kt @@ -12,6 +12,11 @@ import org.moire.ultrasonic.domain.Track class RxBus { + /** + * IMPORTANT: methods like .delay() or .throttle() will implicitly change the thread to the + * RxComputationScheduler. Always use the function call with the additional arguments of the + * desired scheduler + **/ companion object { fun mainThread(): Scheduler = AndroidSchedulers.mainThread() @@ -52,7 +57,8 @@ class RxBus { playerStatePublisher .replay(1) .autoConnect(0) - .throttleLatest(300, TimeUnit.MILLISECONDS) + // Need to specify thread, see comment at beginning + .throttleLatest(300, TimeUnit.MILLISECONDS, mainThread()) val playlistPublisher: PublishSubject> = PublishSubject.create() @@ -64,7 +70,8 @@ class RxBus { playlistPublisher .replay(1) .autoConnect(0) - .throttleLatest(300, TimeUnit.MILLISECONDS) + // Need to specify thread, see comment at beginning + .throttleLatest(300, TimeUnit.MILLISECONDS, mainThread()) val trackDownloadStatePublisher: PublishSubject = PublishSubject.create()