Merge branch '463' into 'master'

Release candidate 4.6.3

See merge request ultrasonic/ultrasonic!1081

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

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

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()

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SearchFragment.kt

@@ -140,7 +140,11 @@ class SearchFragment : MultiListFragment<Identifiable>(), KoinComponent {
     private fun downloadBackground(save: Boolean, songs: List<Track?>) {
         val onValid = Runnable {
             networkAndStorageChecker.warnIfNetworkOrStorageUnavailable()
-            DownloadService.download(songs.filterNotNull(), save)
+            DownloadService.download(
+                songs.filterNotNull(),
+                save = save,
+                updateSaveFlag = true
+            )
         }
         onValid.run()
     }

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) {

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)
         }
     }
 

ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/DownloadService.kt

@@ -274,46 +274,26 @@ class DownloadService : Service(), KoinComponent {
         @Synchronized
         fun download(
             tracks: List<Track>,
-            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<Track>
-                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<Track>): List<Track> {
+            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<Track>
+        ): List<Track> {
+            // 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<Track> = 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)

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)
@@ -245,7 +248,10 @@ class MediaPlayerManager(
         mediaControllerFuture = MediaController.Builder(
             context,
             sessionToken
-        ).buildAsync()
+        )
+            // Specify mainThread explicitely
+            .setApplicationLooper(Looper.getMainLooper())
+            .buildAsync()
 
         mediaControllerFuture?.addListener({
             controller = mediaControllerFuture?.get()

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,14 @@ 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
-    */
+    /**
+     * 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 {
 
-        private fun mainThread() = AndroidSchedulers.from(Looper.getMainLooper())
+        fun mainThread(): Scheduler = AndroidSchedulers.mainThread()
 
         val shufflePlayPublisher: PublishSubject<Boolean> =
             PublishSubject.create()
@@ -57,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<List<Track>> =
             PublishSubject.create()
@@ -69,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<TrackDownloadState> =
             PublishSubject.create()

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)
                 }

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<String> {
         val filesToNotDelete: MutableSet<String> = HashSet(5)
-        val mediaController = inject<MediaPlayerManager>(
-            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())

ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Storage.kt

@@ -22,19 +22,23 @@ import timber.log.Timber
 object Storage {
 
     val mediaRoot: ResettableLazy<AbstractFile> = 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<AbstractFile, Boolean> {
         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)
+    }
 }