From 3660a8bd78f5690620a33aa565b41492391da4d2 Mon Sep 17 00:00:00 2001 From: Prathamesh More Date: Wed, 8 Jun 2022 23:17:15 +0530 Subject: [PATCH] Revert "[Playback] Code Cleanup" This reverts commit 7efbbc3f110935f8e5ee8bc9f6f35f6bb562d6ae. --- .../monkey/retromusic/service/CastPlayer.kt | 12 ++- .../retromusic/service/CrossFadePlayer.kt | 22 ++++-- .../retromusic/service/LocalPlayback.kt | 18 +++-- .../monkey/retromusic/service/MultiPlayer.kt | 50 +++++++----- .../monkey/retromusic/service/MusicService.kt | 76 ++++++++++--------- .../retromusic/service/PlaybackManager.kt | 10 ++- .../retromusic/service/playback/Playback.kt | 4 +- 7 files changed, 114 insertions(+), 78 deletions(-) diff --git a/app/src/main/java/code/name/monkey/retromusic/service/CastPlayer.kt b/app/src/main/java/code/name/monkey/retromusic/service/CastPlayer.kt index e3404f24a..2a67051d6 100644 --- a/app/src/main/java/code/name/monkey/retromusic/service/CastPlayer.kt +++ b/app/src/main/java/code/name/monkey/retromusic/service/CastPlayer.kt @@ -33,15 +33,19 @@ class CastPlayer(castSession: CastSession) : Playback, override var callbacks: Playback.PlaybackCallbacks? = null - override fun setDataSource(song: Song, force: Boolean): Boolean { - return try { + override fun setDataSource( + song: Song, + force: Boolean, + completion: (success: Boolean) -> Unit, + ) { + try { val mediaLoadOptions = MediaLoadOptions.Builder().setPlayPosition(0).setAutoplay(true).build() remoteMediaClient?.load(song.toMediaInfo()!!, mediaLoadOptions) - true + completion(true) } catch (e: Exception) { e.printStackTrace() - false + completion(false) } } diff --git a/app/src/main/java/code/name/monkey/retromusic/service/CrossFadePlayer.kt b/app/src/main/java/code/name/monkey/retromusic/service/CrossFadePlayer.kt index af9661feb..da27e6b36 100644 --- a/app/src/main/java/code/name/monkey/retromusic/service/CrossFadePlayer.kt +++ b/app/src/main/java/code/name/monkey/retromusic/service/CrossFadePlayer.kt @@ -120,17 +120,25 @@ class CrossFadePlayer(context: Context) : LocalPlayback(context) { override val isPlaying: Boolean get() = mIsInitialized && getCurrentPlayer()?.isPlaying == true - override fun setDataSource(song: Song, force: Boolean): Boolean { + override fun setDataSource( + song: Song, + force: Boolean, + completion: (success: Boolean) -> Unit, + ) { if (force) hasDataSource = false mIsInitialized = false /* We've already set DataSource if initialized is true in setNextDataSource */ - return if (!hasDataSource) { - mIsInitialized = setDataSourceImpl(getCurrentPlayer()!!, song.uri.toString()) + if (!hasDataSource) { + getCurrentPlayer()?.let { + setDataSourceImpl(it, song.uri.toString()) { success -> + mIsInitialized = success + completion(success) + } + } hasDataSource = true - mIsInitialized } else { + completion(true) mIsInitialized = true - true } } @@ -285,8 +293,8 @@ class CrossFadePlayer(context: Context) : LocalPlayback(context) { val nextSong = MusicPlayerRemote.nextSong // Switch to other player (Crossfade) only if next song exists if (nextSong != null) { - if (setDataSourceImpl(player, nextSong.uri.toString())) { - switchPlayer() + setDataSourceImpl(player, nextSong.uri.toString()) { success -> + if (success) switchPlayer() } } } diff --git a/app/src/main/java/code/name/monkey/retromusic/service/LocalPlayback.kt b/app/src/main/java/code/name/monkey/retromusic/service/LocalPlayback.kt index ee039f42c..fafbae0ec 100644 --- a/app/src/main/java/code/name/monkey/retromusic/service/LocalPlayback.kt +++ b/app/src/main/java/code/name/monkey/retromusic/service/LocalPlayback.kt @@ -109,10 +109,13 @@ abstract class LocalPlayback(val context: Context) : Playback, MediaPlayer.OnErr * @param path The path of the file, or the http/rtsp URL of the stream you want to play * @return True if the player has been prepared and is ready to play, false otherwise */ - fun setDataSourceImpl(player: MediaPlayer, path: String): Boolean { + fun setDataSourceImpl( + player: MediaPlayer, + path: String, + completion: (success: Boolean) -> Unit, + ) { + player.reset() try { - player.reset() - player.setOnPreparedListener(null) if (path.startsWith("content://")) { player.setDataSource(context, path.toUri()) } else { @@ -123,14 +126,17 @@ abstract class LocalPlayback(val context: Context) : Playback, MediaPlayer.OnErr .setContentType(AudioAttributes.CONTENT_TYPE_MUSIC) .build() ) - player.prepare() + player.setOnPreparedListener { + player.setOnPreparedListener(null) + completion(true) + } + player.prepareAsync() } catch (e: Exception) { + completion(false) e.printStackTrace() - return false } player.setOnCompletionListener(this) player.setOnErrorListener(this) - return true } private fun unregisterBecomingNoisyReceiver() { diff --git a/app/src/main/java/code/name/monkey/retromusic/service/MultiPlayer.kt b/app/src/main/java/code/name/monkey/retromusic/service/MultiPlayer.kt index d0e766b3f..8421b60f1 100644 --- a/app/src/main/java/code/name/monkey/retromusic/service/MultiPlayer.kt +++ b/app/src/main/java/code/name/monkey/retromusic/service/MultiPlayer.kt @@ -46,13 +46,19 @@ class MultiPlayer(context: Context) : LocalPlayback(context) { * @param song The song object you want to play * @return True if the `player` has been prepared and is ready to play, false otherwise */ - override fun setDataSource(song: Song, force: Boolean): Boolean { + override fun setDataSource( + song: Song, + force: Boolean, + completion: (success: Boolean) -> Unit, + ) { isInitialized = false - isInitialized = setDataSourceImpl(mCurrentMediaPlayer, song.uri.toString()) - if (isInitialized) { - setNextDataSource(null) + setDataSourceImpl(mCurrentMediaPlayer, song.uri.toString()) { success -> + isInitialized = success + if (isInitialized) { + setNextDataSource(null) + } + completion(isInitialized) } - return isInitialized } /** @@ -80,26 +86,28 @@ class MultiPlayer(context: Context) : LocalPlayback(context) { mNextMediaPlayer = MediaPlayer() mNextMediaPlayer?.setWakeMode(context, PowerManager.PARTIAL_WAKE_LOCK) mNextMediaPlayer?.audioSessionId = audioSessionId - if (setDataSourceImpl(mNextMediaPlayer!!, path)) { - try { - mCurrentMediaPlayer.setNextMediaPlayer(mNextMediaPlayer) - } catch (e: IllegalArgumentException) { - Log.e(TAG, "setNextDataSource: setNextMediaPlayer()", e) + setDataSourceImpl(mNextMediaPlayer!!, path) { success -> + if (success) { + try { + mCurrentMediaPlayer.setNextMediaPlayer(mNextMediaPlayer) + } catch (e: IllegalArgumentException) { + Log.e(TAG, "setNextDataSource: setNextMediaPlayer()", e) + if (mNextMediaPlayer != null) { + mNextMediaPlayer?.release() + mNextMediaPlayer = null + } + } catch (e: IllegalStateException) { + Log.e(TAG, "setNextDataSource: setNextMediaPlayer()", e) + if (mNextMediaPlayer != null) { + mNextMediaPlayer?.release() + mNextMediaPlayer = null + } + } + } else { if (mNextMediaPlayer != null) { mNextMediaPlayer?.release() mNextMediaPlayer = null } - } catch (e: IllegalStateException) { - Log.e(TAG, "setNextDataSource: setNextMediaPlayer()", e) - if (mNextMediaPlayer != null) { - mNextMediaPlayer?.release() - mNextMediaPlayer = null - } - } - } else { - if (mNextMediaPlayer != null) { - mNextMediaPlayer?.release() - mNextMediaPlayer = null } } } diff --git a/app/src/main/java/code/name/monkey/retromusic/service/MusicService.kt b/app/src/main/java/code/name/monkey/retromusic/service/MusicService.kt index cee0a47ed..512cd3839 100644 --- a/app/src/main/java/code/name/monkey/retromusic/service/MusicService.kt +++ b/app/src/main/java/code/name/monkey/retromusic/service/MusicService.kt @@ -117,11 +117,6 @@ class MusicService : MediaBrowserServiceCompat(), private var trackEndedByCrossfade = false private val serviceScope = CoroutineScope(Job() + Main) - // Every chromecast method needs to run on main thread or you are greeted with IllegalStateException - // So it will use Main dispatcher - // And by using Default dispatcher for local playback we are reducing the burden from main thread - private val playerDispatcher get() = if (playbackManager.isLocalPlayback) Default else Main - @JvmField var position = -1 private val appWidgetBig = AppWidgetBig.instance @@ -441,8 +436,11 @@ class MusicService : MediaBrowserServiceCompat(), } private fun setPosition(position: Int) { - openTrackAndPrepareNextAt(position) - notifyChange(PLAY_STATE_CHANGED) + openTrackAndPrepareNextAt(position) { success -> + if (success) { + notifyChange(PLAY_STATE_CHANGED) + } + } } private fun getPreviousPosition(force: Boolean): Int { @@ -757,15 +755,16 @@ class MusicService : MediaBrowserServiceCompat(), } @Synchronized - fun openTrackAndPrepareNextAt(position: Int): Boolean { + fun openTrackAndPrepareNextAt(position: Int, completion: (success: Boolean) -> Unit) { this.position = position - val prepared = openCurrent() - if (prepared) { - prepareNextImpl() + openCurrent { success -> + completion(success) + notifyChange(META_CHANGED) + notHandledMetaChangedForCurrentTrack = false + if (success) { + prepareNextImpl() + } } - notifyChange(META_CHANGED) - notHandledMetaChangedForCurrentTrack = false - return prepared } fun pause(force: Boolean = false) { @@ -794,11 +793,16 @@ class MusicService : MediaBrowserServiceCompat(), } fun playSongAt(position: Int) { - serviceScope.launch(playerDispatcher) { - if (openTrackAndPrepareNextAt(position)) { - play() - } else { - showToast(R.string.unplayable_file) + // Every chromecast method needs to run on main thread or you are greeted with IllegalStateException + // So it will use Main dispatcher + // And by using Default dispatcher for local playback we are reduce the burden of main thread + serviceScope.launch(if(playbackManager.isLocalPlayback) Default else Main) { + openTrackAndPrepareNextAt(position) { success -> + if (success) { + play() + } else { + showToast(resources.getString(R.string.unplayable_file)) + } } } } @@ -913,14 +917,15 @@ class MusicService : MediaBrowserServiceCompat(), originalPlayingQueue = ArrayList(restoredOriginalQueue) playingQueue = ArrayList(restoredQueue) position = restoredPosition - withContext(playerDispatcher) { - openCurrent() - prepareNext() - if (restoredPositionInTrack > 0) { - seek(restoredPositionInTrack) + withContext(Main) { + openCurrent { + prepareNext() + if (restoredPositionInTrack > 0) { + seek(restoredPositionInTrack) + } + notHandledMetaChangedForCurrentTrack = true + sendChangeInternal(META_CHANGED) } - notHandledMetaChangedForCurrentTrack = true - sendChangeInternal(META_CHANGED) if (receivedHeadsetConnected) { play() receivedHeadsetConnected = false @@ -1164,18 +1169,15 @@ class MusicService : MediaBrowserServiceCompat(), } @Synchronized - private fun openCurrent(): Boolean { + private fun openCurrent(completion: (success: Boolean) -> Unit) { val force = if (!trackEndedByCrossfade) { true } else { trackEndedByCrossfade = false false } - return try { - playbackManager.setDataSource(currentSong, force) - } catch (e: Exception) { - e.printStackTrace() - false + playbackManager.setDataSource(currentSong, force) { success -> + completion(success) } } @@ -1189,10 +1191,12 @@ class MusicService : MediaBrowserServiceCompat(), private fun restorePlaybackState(wasPlaying: Boolean, progress: Int) { playbackManager.setCallbacks(this) - if (openTrackAndPrepareNextAt(position)) { - seek(progress) - if (wasPlaying) { - play() + openTrackAndPrepareNextAt(position) { success -> + if (success) { + seek(progress) + if (wasPlaying) { + play() + } } } playbackManager.setCrossFadeDuration(crossFadeDuration) diff --git a/app/src/main/java/code/name/monkey/retromusic/service/PlaybackManager.kt b/app/src/main/java/code/name/monkey/retromusic/service/PlaybackManager.kt index f64b7d600..9186bd534 100644 --- a/app/src/main/java/code/name/monkey/retromusic/service/PlaybackManager.kt +++ b/app/src/main/java/code/name/monkey/retromusic/service/PlaybackManager.kt @@ -16,7 +16,7 @@ class PlaybackManager(val context: Context) { var playback: Playback? = null private var playbackLocation = PlaybackLocation.LOCAL - val isLocalPlayback get() = playbackLocation == PlaybackLocation.LOCAL + val isLocalPlayback get() = playbackLocation== PlaybackLocation.LOCAL val audioSessionId: Int get() = if (playback != null) { @@ -86,8 +86,12 @@ class PlaybackManager(val context: Context) { fun seek(millis: Int): Int = playback!!.seek(millis) - fun setDataSource(song: Song, force: Boolean): Boolean { - return playback?.setDataSource(song, force) == true + fun setDataSource( + song: Song, + force: Boolean, + completion: (success: Boolean) -> Unit, + ) { + playback?.setDataSource(song, force, completion) } fun setNextDataSource(trackUri: String) { diff --git a/app/src/main/java/code/name/monkey/retromusic/service/playback/Playback.kt b/app/src/main/java/code/name/monkey/retromusic/service/playback/Playback.kt index f3521ab96..3795e7634 100644 --- a/app/src/main/java/code/name/monkey/retromusic/service/playback/Playback.kt +++ b/app/src/main/java/code/name/monkey/retromusic/service/playback/Playback.kt @@ -25,7 +25,9 @@ interface Playback { val audioSessionId: Int - fun setDataSource(song: Song, force: Boolean):Boolean + fun setDataSource( + song: Song, force: Boolean, completion: (success: Boolean) -> Unit, + ) fun setNextDataSource(path: String?)