Merge branch 'fix_serverchange' into 'develop'

Fixed server change

Closes #817

See merge request ultrasonic/ultrasonic!838

core/subsonic-api/build.gradle

@@ -10,7 +10,6 @@ dependencies {
     }
     implementation libs.kotlinReflect // for jackson kotlin, but to use the same version
     implementation libs.okhttpLogging
-    implementation libs.timber
 
     testImplementation libs.kotlinJunit
     testImplementation libs.mockito

ultrasonic/src/main/java/org/moire/ultrasonic/fragment/ChatFragment.java

@@ -83,15 +83,7 @@ public class ChatFragment extends Fragment {
         messageEditText = view.findViewById(R.id.chat_edittext);
         sendButton = view.findViewById(R.id.chat_send);
 
-        sendButton.setOnClickListener(new View.OnClickListener()
+        sendButton.setOnClickListener(view1 -> sendMessage());
-        {
-
-            @Override
-            public void onClick(View view)
-            {
-                sendMessage();
-            }
-        });
 
         chatListView = view.findViewById(R.id.chat_entries_list);
         chatListView.setTranscriptMode(ListView.TRANSCRIPT_MODE_ALWAYS_SCROLL);
@@ -125,20 +117,14 @@ public class ChatFragment extends Fragment {
             }
         });
 
-        messageEditText.setOnEditorActionListener(new TextView.OnEditorActionListener()
+        messageEditText.setOnEditorActionListener((v, actionId, event) -> {
-        {
+            if (actionId == EditorInfo.IME_ACTION_DONE || (actionId == EditorInfo.IME_NULL && event.getAction() == KeyEvent.ACTION_DOWN))
-
-            @Override
-            public boolean onEditorAction(TextView v, int actionId, KeyEvent event)
             {
-                if (actionId == EditorInfo.IME_ACTION_DONE || (actionId == EditorInfo.IME_NULL && event.getAction() == KeyEvent.ACTION_DOWN))
+                sendMessage();
-                {
+                return true;
-                    sendMessage();
-                    return true;
-                }
-
-                return false;
             }
+
+            return false;
         });
 
         load();
@@ -215,14 +201,7 @@ public class ChatFragment extends Fragment {
                 @Override
                 public void run()
                 {
-                    getActivity().runOnUiThread(new Runnable()
+                    getActivity().runOnUiThread(() -> load());
-                    {
-                        @Override
-                        public void run()
-                        {
-                            load();
-                        }
-                    });
                 }
             }, refreshInterval, refreshInterval);
         }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/activity/NavigationActivity.kt

@@ -211,7 +211,7 @@ class NavigationActivity : AppCompatActivity() {
             recreate()
         }
 
-        rxBusSubscription += RxBus.activeServerChangeObservable.subscribe {
+        rxBusSubscription += RxBus.activeServerChangedObservable.subscribe {
             updateNavigationHeaderForServer()
         }
 

ultrasonic/src/main/kotlin/org/moire/ultrasonic/data/ActiveServerProvider.kt

@@ -7,6 +7,8 @@
 
 package org.moire.ultrasonic.data
 
+import android.os.Handler
+import android.os.Looper
 import androidx.room.Room
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -234,12 +236,22 @@ class ActiveServerProvider(
          * @param serverId: The id of the desired server
          */
         fun setActiveServerById(serverId: Int) {
-            resetMusicService()
+            val oldServerId = Settings.activeServer
+            if (oldServerId == serverId) return
 
-            Settings.activeServer = serverId
+            // Notify components about the change before actually resetting the MusicService
+            // so they can react by e.g. stopping playback on the old server
+            RxBus.activeServerChangingPublisher.onNext(oldServerId)
 
-            Timber.i("setActiveServerById done, new id: %s", serverId)
+            // Post the server change to the end of the message queue,
-            RxBus.activeServerChangePublisher.onNext(serverId)
+            // so the cleanup have time to finish
+            Handler(Looper.getMainLooper()).post {
+                resetMusicService()
+                Settings.activeServer = serverId
+
+                RxBus.activeServerChangedPublisher.onNext(serverId)
+                Timber.i("setActiveServerById done, new id: %s", serverId)
+            }
         }
 
         /**

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

@@ -17,8 +17,6 @@ import org.moire.ultrasonic.data.ActiveServerProvider
 import org.moire.ultrasonic.data.ActiveServerProvider.Companion.OFFLINE_DB_ID
 import org.moire.ultrasonic.data.ServerSetting
 import org.moire.ultrasonic.model.ServerSettingsModel
-import org.moire.ultrasonic.service.DownloadService
-import org.moire.ultrasonic.service.MediaPlayerController
 import org.moire.ultrasonic.util.ErrorDialog
 import org.moire.ultrasonic.util.Util
 import timber.log.Timber
@@ -30,7 +28,6 @@ class ServerSelectorFragment : Fragment() {
 
     private var listView: ListView? = null
     private val serverSettingsModel: ServerSettingsModel by viewModel()
-    private val controller: MediaPlayerController by inject()
     private val activeServerProvider: ActiveServerProvider by inject()
     private var serverRowAdapter: ServerRowAdapter? = null
 
@@ -68,7 +65,7 @@ class ServerSelectorFragment : Fragment() {
         listView?.onItemClickListener = AdapterView.OnItemClickListener { parent, _, position, _ ->
 
             val server = parent.getItemAtPosition(position) as ServerSetting
-            setActiveServerById(server.id)
+            ActiveServerProvider.setActiveServerById(server.id)
             findNavController().popBackStack(R.id.mainFragment, false)
         }
 
@@ -88,26 +85,6 @@ class ServerSelectorFragment : Fragment() {
         }
     }
 
-    /**
-     * Sets the active server when a list item is clicked
-     */
-    private fun setActiveServerById(id: Int) {
-        val oldId = activeServerProvider.getActiveServer().id
-
-        // Check if there is a change
-        if (oldId == id)
-            return
-
-        // Remove incomplete tracks if we are going offline, or changing between servers.
-        // If we are coming from offline there is no need to clear downloads etc.
-        if (oldId != OFFLINE_DB_ID) {
-            controller.removeIncompleteTracksFromPlaylist()
-            DownloadService.requestStop()
-        }
-
-        ActiveServerProvider.setActiveServerById(id)
-    }
-
     /**
      * This Callback handles the deletion of a Server Setting
      */
@@ -122,7 +99,7 @@ class ServerSelectorFragment : Fragment() {
                 val activeServerId = ActiveServerProvider.getActiveServerId()
 
                 // If the currently active server is deleted, go offline
-                if (id == activeServerId) setActiveServerById(OFFLINE_DB_ID)
+                if (id == activeServerId) ActiveServerProvider.setActiveServerById(OFFLINE_DB_ID)
 
                 serverSettingsModel.deleteItemById(id)
 

ultrasonic/src/main/kotlin/org/moire/ultrasonic/playback/PlaybackService.kt

@@ -37,6 +37,7 @@ import org.moire.ultrasonic.service.plusAssign
 import org.moire.ultrasonic.util.Constants
 import org.moire.ultrasonic.util.Settings
 import org.moire.ultrasonic.util.Util
+import org.moire.ultrasonic.util.Util.stopForegroundRemoveNotification
 import org.moire.ultrasonic.util.toTrack
 import timber.log.Timber
 
@@ -86,12 +87,7 @@ class PlaybackService : MediaLibraryService(), KoinComponent {
         mediaLibrarySession.release()
         rxBusSubscription.dispose()
         isStarted = false
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
+        stopForegroundRemoveNotification()
-            stopForeground(STOP_FOREGROUND_REMOVE)
-        } else {
-            @Suppress("DEPRECATION")
-            stopForeground(true)
-        }
         stopSelf()
     }
 
@@ -155,7 +151,7 @@ class PlaybackService : MediaLibraryService(), KoinComponent {
             .build()
 
         // Set a listener to update the API client when the active server has changed
-        rxBusSubscription += RxBus.activeServerChangeObservable.subscribe {
+        rxBusSubscription += RxBus.activeServerChangedObservable.subscribe {
             // Set the player wake mode
             player.setWakeMode(getWakeModeFlag())
         }

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

@@ -38,6 +38,7 @@ import org.moire.ultrasonic.util.Settings
 import org.moire.ultrasonic.util.SimpleServiceBinder
 import org.moire.ultrasonic.util.Storage
 import org.moire.ultrasonic.util.Util
+import org.moire.ultrasonic.util.Util.stopForegroundRemoveNotification
 import timber.log.Timber
 
 private const val NOTIFICATION_CHANNEL_ID = "org.moire.ultrasonic"
@@ -94,7 +95,7 @@ class DownloadService : Service(), KoinComponent {
 
         isShuttingDown = true
         isInForeground = false
-        stopForeground(true)
+        stopForegroundRemoveNotification()
 
         wifiLock?.release()
         wifiLock = null

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

@@ -60,11 +60,13 @@ import org.moire.ultrasonic.playback.MediaNotificationProvider
 import org.moire.ultrasonic.service.MusicServiceFactory.getMusicService
 import org.moire.ultrasonic.util.Util.getPendingIntentToShowPlayer
 import org.moire.ultrasonic.util.Util.sleepQuietly
+import org.moire.ultrasonic.util.Util.stopForegroundRemoveNotification
 import timber.log.Timber
 
 private const val STATUS_UPDATE_INTERVAL_SECONDS = 5L
 private const val SEEK_INCREMENT_SECONDS = 5L
 private const val SEEK_START_AFTER_SECONDS = 5
+private const val QUEUE_POLL_INTERVAL_SECONDS = 1L
 
 /**
  * Provides an asynchronous interface to the remote jukebox on the Subsonic server.
@@ -159,17 +161,15 @@ class JukeboxMediaPlayer : JukeboxUnimplementedFunctions(), Player {
     }
 
     override fun onDestroy() {
+        tasks.clear()
+        stop()
+
         if (!running.get()) return
         running.set(false)
 
-        sleepQuietly(1000)
+        serviceThread!!.join()
-        if (serviceThread != null) {
-            serviceThread!!.interrupt()
-        }
 
-        tasks.clear()
+        stopForegroundRemoveNotification()
-        stop()
-        stopForeground(true)
         mediaSession.release()
 
         super.onDestroy()
@@ -417,9 +417,8 @@ class JukeboxMediaPlayer : JukeboxUnimplementedFunctions(), Player {
         if (playlist.isEmpty()) return 0
         if (currentIndex < 0 || currentIndex >= playlist.size) return 0
 
-        return (
+        return (playlist[currentIndex].mediaMetadata.extras?.getInt("duration") ?: 0)
-            playlist[currentIndex].mediaMetadata.extras?.getInt("duration") ?: 0
+            .toLong() * 1000
-            ).toLong() * 1000
     }
 
     override fun getContentDuration(): Long {
@@ -449,6 +448,17 @@ class JukeboxMediaPlayer : JukeboxUnimplementedFunctions(), Player {
         seekTo(currentIndex, 0)
     }
 
+    override fun setMediaItems(
+        mediaItems: MutableList<MediaItem>,
+        startIndex: Int,
+        startPositionMs: Long
+    ) {
+        playlist.clear()
+        playlist.addAll(mediaItems)
+        updatePlaylist()
+        seekTo(startIndex, startPositionMs)
+    }
+
     private fun startProcessTasks() {
         serviceThread = object : Thread() {
             override fun run() {
@@ -481,22 +491,28 @@ class JukeboxMediaPlayer : JukeboxUnimplementedFunctions(), Player {
         }
     }
 
+    @Suppress("LoopWithTooManyJumpStatements")
     private fun processTasks() {
-        while (running.get()) {
+        Timber.d("JukeboxMediaPlayer processTasks starting")
+        while (true) {
             // Sleep a bit to spare processor time if we loop a lot
             sleepQuietly(10)
+            // This is only necessary if Ultrasonic goes offline sooner than the thread stops
+            if (isOffline()) continue
             var task: JukeboxTask? = null
             try {
-                if (!isOffline()) {
+                task = tasks.poll()
-                    task = tasks.take()
+                // If running is false, exit when the queue is empty
-                    val status = task.execute()
+                if (task == null && !running.get()) break
-                    onStatusUpdate(status)
+                if (task == null) continue
-                }
+                Timber.v("JukeBoxMediaPlayer processTasks processes Task %s", task::class)
-            } catch (ignored: InterruptedException) {
+                val status = task.execute()
+                onStatusUpdate(status)
             } catch (x: Throwable) {
                 onError(task, x)
             }
         }
+        Timber.d("JukeboxMediaPlayer processTasks stopped")
     }
 
     private fun onStatusUpdate(jukeboxStatus: JukeboxStatus) {
@@ -621,9 +637,8 @@ class JukeboxMediaPlayer : JukeboxUnimplementedFunctions(), Player {
             queue.add(jukeboxTask)
         }
 
-        @Throws(InterruptedException::class)
+        fun poll(): JukeboxTask? {
-        fun take(): JukeboxTask {
+            return queue.poll(QUEUE_POLL_INTERVAL_SECONDS, TimeUnit.SECONDS)
-            return queue.take()
         }
 
         fun remove(taskClass: Class<out JukeboxTask?>) {

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

@@ -36,14 +36,6 @@ abstract class JukeboxUnimplementedFunctions : Service(), Player {
         TODO("Not yet implemented")
     }
 
-    override fun setMediaItems(
-        mediaItems: MutableList<MediaItem>,
-        startIndex: Int,
-        startPositionMs: Long
-    ) {
-        TODO("Not yet implemented")
-    }
-
     override fun setMediaItem(mediaItem: MediaItem) {
         TODO("Not yet implemented")
     }

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

@@ -33,6 +33,7 @@ import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
 import org.moire.ultrasonic.app.UApp
 import org.moire.ultrasonic.data.ActiveServerProvider
+import org.moire.ultrasonic.data.ActiveServerProvider.Companion.OFFLINE_DB_ID
 import org.moire.ultrasonic.domain.Track
 import org.moire.ultrasonic.playback.PlaybackService
 import org.moire.ultrasonic.provider.UltrasonicAppWidgetProvider4X1
@@ -162,9 +163,28 @@ class MediaPlayerController(
             switchToLocalPlayer(onCreated)
         }
 
-        rxBusSubscription += RxBus.activeServerChangeObservable.subscribe {
+        rxBusSubscription += RxBus.activeServerChangingObservable.subscribe { oldServer ->
-            // Update the Jukebox state when the active server has changed
+            if (oldServer != OFFLINE_DB_ID) {
-            isJukeboxEnabled = activeServerProvider.getActiveServer().jukeboxByDefault
+                // 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.
+                releaseJukebox(controller)
+                controller = null
+            }
+        }
+
+        rxBusSubscription += RxBus.activeServerChangedObservable.subscribe {
+            val jukebox = activeServerProvider.getActiveServer().jukeboxByDefault
+            // Remove all songs when changing servers before turning on Jukebox.
+            // Jukebox wouldn't find the songs on the new server.
+            if (jukebox) controller?.clearMediaItems()
+            // Update the Jukebox state as the new server requires
+            isJukeboxEnabled = jukebox
         }
 
         rxBusSubscription += RxBus.throttledPlaylistObservable.subscribe {
@@ -550,7 +570,7 @@ class MediaPlayerController(
         }
 
     private fun switchToJukebox(onCreated: () -> Unit) {
-        if (JukeboxMediaPlayer.running.get()) return
+        if (controller is JukeboxMediaPlayer) return
         val currentPlaylist = playlist
         val currentIndex = controller?.currentMediaItemIndex ?: 0
         val currentPosition = controller?.currentPosition ?: 0
@@ -564,14 +584,14 @@ class MediaPlayerController(
         Handler(Looper.getMainLooper()).postDelayed({
             if (oldController != null) releaseLocalPlayer(oldController)
             setupJukebox {
-                controller?.addMediaItems(0, currentPlaylist)
+                controller?.setMediaItems(currentPlaylist, currentIndex, currentPosition)
-                controller?.seekTo(currentIndex, currentPosition)
                 onCreated()
             }
         }, CONTROLLER_SWITCH_DELAY)
     }
 
     private fun switchToLocalPlayer(onCreated: () -> Unit) {
+        if (controller is MediaController) return
         val currentPlaylist = playlist
         val currentIndex = controller?.currentMediaItemIndex ?: 0
         val currentPosition = controller?.currentPosition ?: 0
@@ -582,8 +602,7 @@ class MediaPlayerController(
         Handler(Looper.getMainLooper()).postDelayed({
             if (oldController != null) releaseJukebox(oldController)
             setupLocalPlayer {
-                controller?.addMediaItems(0, currentPlaylist)
+                controller?.setMediaItems(currentPlaylist, currentIndex, currentPosition)
-                controller?.seekTo(currentIndex, currentPosition)
                 onCreated()
             }
         }, CONTROLLER_SWITCH_DELAY)

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

@@ -20,10 +20,17 @@ class RxBus {
 
         private fun mainThread() = AndroidSchedulers.from(Looper.getMainLooper())
 
-        var activeServerChangePublisher: PublishSubject<Int> =
+        var activeServerChangingPublisher: PublishSubject<Int> =
             PublishSubject.create()
-        var activeServerChangeObservable: Observable<Int> =
+
-            activeServerChangePublisher.observeOn(mainThread())
+        // Subscribers should be called synchronously, not on another thread
+        var activeServerChangingObservable: Observable<Int> =
+            activeServerChangingPublisher
+
+        var activeServerChangedPublisher: PublishSubject<Int> =
+            PublishSubject.create()
+        var activeServerChangedObservable: Observable<Int> =
+            activeServerChangedPublisher.observeOn(mainThread())
 
         val themeChangedEventPublisher: PublishSubject<Unit> =
             PublishSubject.create()

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

@@ -11,6 +11,8 @@ import android.Manifest.permission.POST_NOTIFICATIONS
 import android.annotation.SuppressLint
 import android.app.Activity
 import android.app.PendingIntent
+import android.app.Service
+import android.app.Service.STOP_FOREGROUND_REMOVE
 import android.content.ContentResolver
 import android.content.Context
 import android.content.Intent
@@ -33,6 +35,7 @@ import android.util.TypedValue
 import android.view.Gravity
 import android.view.inputmethod.InputMethodManager
 import android.widget.Toast
+import androidx.activity.ComponentActivity
 import androidx.activity.result.contract.ActivityResultContracts
 import androidx.annotation.AnyRes
 import androidx.appcompat.app.AppCompatActivity
@@ -512,7 +515,7 @@ object Util {
         }
     }
 
-    fun ensurePermissionToPostNotification(fragment: AppCompatActivity) {
+    fun ensurePermissionToPostNotification(fragment: ComponentActivity) {
         if (ContextCompat.checkSelfPermission(
                 applicationContext(),
                 POST_NOTIFICATIONS,
@@ -751,4 +754,13 @@ object Util {
             // Ignored
         }
     }
+
+    fun Service.stopForegroundRemoveNotification() {
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
+            stopForeground(STOP_FOREGROUND_REMOVE)
+        } else {
+            @Suppress("DEPRECATION")
+            stopForeground(true)
+        }
+    }
 }