From 8c2896ea16baef6b518edb7d50628bc65e71dbff Mon Sep 17 00:00:00 2001 From: tzugen Date: Sat, 19 Jun 2021 23:09:38 +0200 Subject: [PATCH] Remove static field leaks on SeekBar, cleanup code and update baseline --- ultrasonic/lint-baseline.xml | 456 +++++++----------- .../ultrasonic/fragment/PlayerFragment.kt | 220 +++++---- .../ultrasonic/service/LocalMediaPlayer.kt | 23 +- .../service/MediaPlayerController.kt | 6 - .../ultrasonic/util/SilentBackgroundTask.kt | 21 +- 5 files changed, 329 insertions(+), 397 deletions(-) rename ultrasonic/src/main/{java => kotlin}/org/moire/ultrasonic/fragment/PlayerFragment.kt (89%) rename ultrasonic/src/main/{java => kotlin}/org/moire/ultrasonic/util/SilentBackgroundTask.kt (50%) diff --git a/ultrasonic/lint-baseline.xml b/ultrasonic/lint-baseline.xml index a0874cbb..037c3e49 100644 --- a/ultrasonic/lint-baseline.xml +++ b/ultrasonic/lint-baseline.xml @@ -183,21 +183,10 @@ errorLine2=" ^"> - - - - @@ -216,7 +205,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -227,7 +216,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -238,7 +227,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -249,7 +238,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -260,7 +249,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -271,7 +260,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -282,7 +271,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -293,7 +282,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -304,7 +293,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -315,7 +304,7 @@ errorLine2=" ^"> @@ -326,7 +315,7 @@ errorLine2=" ^"> @@ -337,7 +326,7 @@ errorLine2=" ^"> @@ -348,7 +337,7 @@ errorLine2=" ^"> @@ -359,7 +348,7 @@ errorLine2=" ^"> @@ -370,7 +359,7 @@ errorLine2=" ^"> @@ -381,7 +370,7 @@ errorLine2=" ^"> @@ -491,17 +480,6 @@ column="19"/> - - - - - + - + - + - + - + - + - + - + @@ -1219,7 +1197,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -1230,27 +1208,27 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> - + - + - + - + @@ -1469,43 +1447,43 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -1571,7 +1549,7 @@ errorLine2=" ^"> @@ -1582,7 +1560,7 @@ errorLine2=" ^"> @@ -1593,7 +1571,7 @@ errorLine2=" ^"> @@ -1604,7 +1582,7 @@ errorLine2=" ^"> @@ -1615,7 +1593,7 @@ errorLine2=" ^"> @@ -1626,7 +1604,7 @@ errorLine2=" ^"> @@ -1637,18 +1615,7 @@ errorLine2=" ^"> - - - - @@ -1659,7 +1626,7 @@ errorLine2=" ^"> @@ -1670,7 +1637,7 @@ errorLine2=" ^"> @@ -1782,61 +1749,6 @@ column="42"/> - - - - - - - - - - - - - - - - - - - - (activity) { override fun doInBackground(): Void? { - mediaPlayerController.seekTo(progressBar!!.progress) + mediaPlayerController.seekTo(progressBar.progress) return null } @@ -328,7 +348,7 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon override fun onStartTrackingTouch(seekBar: SeekBar) {} override fun onProgressChanged(seekBar: SeekBar, progress: Int, fromUser: Boolean) {} }) - + playlistView.setOnItemClickListener { _, _, position, _ -> networkAndStorageChecker.warnIfNetworkOrStorageUnavailable() object : SilentBackgroundTask(activity) { @@ -346,53 +366,59 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon registerForContextMenu(playlistView) if (arguments != null && requireArguments().getBoolean( - Constants.INTENT_EXTRA_NAME_SHUFFLE, - false - ) + Constants.INTENT_EXTRA_NAME_SHUFFLE, + false + ) ) { networkAndStorageChecker.warnIfNetworkOrStorageUnavailable() mediaPlayerController.isShufflePlayEnabled = true } visualizerViewLayout.visibility = View.GONE - VisualizerController.get().observe(requireActivity(), { visualizerController -> - if (visualizerController != null) { - Timber.d("VisualizerController Observer.onChanged received controller") - visualizerView = VisualizerView(context) - visualizerViewLayout.addView( - visualizerView, - LinearLayout.LayoutParams( - LinearLayout.LayoutParams.MATCH_PARENT, - LinearLayout.LayoutParams.MATCH_PARENT + VisualizerController.get().observe( + requireActivity(), + { visualizerController -> + if (visualizerController != null) { + Timber.d("VisualizerController Observer.onChanged received controller") + visualizerView = VisualizerView(context) + visualizerViewLayout.addView( + visualizerView, + LinearLayout.LayoutParams( + LinearLayout.LayoutParams.MATCH_PARENT, + LinearLayout.LayoutParams.MATCH_PARENT + ) ) - ) - if (!visualizerView.isActive) { - visualizerViewLayout.visibility = View.GONE + if (!visualizerView.isActive) { + visualizerViewLayout.visibility = View.GONE + } else { + visualizerViewLayout.visibility = View.VISIBLE + } + visualizerView.setOnTouchListener { _, _ -> + visualizerView.isActive = !visualizerView.isActive + mediaPlayerController.showVisualization = visualizerView.isActive + true + } + isVisualizerAvailable = true } else { - visualizerViewLayout.visibility = View.VISIBLE + Timber.d("VisualizerController Observer.onChanged has no controller") + visualizerViewLayout.visibility = View.GONE + isVisualizerAvailable = false } - visualizerView.setOnTouchListener { _, _ -> - visualizerView.isActive = !visualizerView.isActive - mediaPlayerController.showVisualization = visualizerView.isActive - true - } - isVisualizerAvailable = true - } else { - Timber.d("VisualizerController Observer.onChanged has no controller") - visualizerViewLayout.visibility = View.GONE - isVisualizerAvailable = false } - }) + ) - EqualizerController.get().observe(requireActivity(), { equalizerController -> - isEqualizerAvailable = if (equalizerController != null) { - Timber.d("EqualizerController Observer.onChanged received controller") - true - } else { - Timber.d("EqualizerController Observer.onChanged has no controller") - false + EqualizerController.get().observe( + requireActivity(), + { equalizerController -> + isEqualizerAvailable = if (equalizerController != null) { + Timber.d("EqualizerController Observer.onChanged received controller") + true + } else { + Timber.d("EqualizerController Observer.onChanged has no controller") + false + } } - }) + ) Thread { try { jukeboxAvailable = mediaPlayerController.isJukeboxAvailable @@ -423,7 +449,9 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon requireActivity().window.clearFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) } - visualizerView.isActive = mediaPlayerController.showVisualization + if (::visualizerView.isInitialized) { + visualizerView.isActive = mediaPlayerController.showVisualization + } requireActivity().invalidateOptionsMenu() } @@ -452,7 +480,9 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon override fun onPause() { super.onPause() executorService.shutdown() - visualizerView.isActive = false + if (::visualizerView.isInitialized) { + visualizerView.isActive = mediaPlayerController.showVisualization + } } override fun onDestroyView() { @@ -583,7 +613,8 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon bundle.putString(Constants.INTENT_EXTRA_NAME_NAME, entry.artist) bundle.putString(Constants.INTENT_EXTRA_NAME_PARENT_ID, entry.artistId) bundle.putBoolean(Constants.INTENT_EXTRA_NAME_ARTIST, true) - Navigation.findNavController(view!!).navigate(R.id.playerToSelectAlbum, bundle) + Navigation.findNavController(requireView()) + .navigate(R.id.playerToSelectAlbum, bundle) } return true } @@ -597,7 +628,8 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon bundle.putString(Constants.INTENT_EXTRA_NAME_NAME, entry.album) bundle.putString(Constants.INTENT_EXTRA_NAME_PARENT_ID, entry.parent) bundle.putBoolean(Constants.INTENT_EXTRA_NAME_IS_ALBUM, true) - Navigation.findNavController(view!!).navigate(R.id.playerToSelectAlbum, bundle) + Navigation.findNavController(requireView()) + .navigate(R.id.playerToSelectAlbum, bundle) return true } R.id.menu_lyrics -> { @@ -607,7 +639,7 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon bundle = Bundle() bundle.putString(Constants.INTENT_EXTRA_NAME_ARTIST, entry.artist) bundle.putString(Constants.INTENT_EXTRA_NAME_TITLE, entry.title) - Navigation.findNavController(view!!).navigate(R.id.playerToLyrics, bundle) + Navigation.findNavController(requireView()).navigate(R.id.playerToLyrics, bundle) return true } R.id.menu_remove -> { @@ -616,11 +648,12 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon return true } R.id.menu_item_screen_on_off -> { + val window = requireActivity().window if (mediaPlayerController.keepScreenOn) { - activity!!.window.clearFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) + window.clearFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) mediaPlayerController.keepScreenOn = false } else { - activity!!.window.addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) + window.addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON) mediaPlayerController.keepScreenOn = true } return true @@ -631,7 +664,7 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon return true } R.id.menu_item_equalizer -> { - Navigation.findNavController(view!!).navigate(R.id.playerToEqualizer) + Navigation.findNavController(requireView()).navigate(R.id.playerToEqualizer) return true } R.id.menu_item_visualizer -> { @@ -645,7 +678,8 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon mediaPlayerController.showVisualization = visualizerView.isActive Util.toast( context, - if (active) R.string.download_visualizer_on else R.string.download_visualizer_off + if (active) R.string.download_visualizer_on + else R.string.download_visualizer_off ) return true } @@ -654,7 +688,8 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon mediaPlayerController.isJukeboxEnabled = jukeboxEnabled Util.toast( context, - if (jukeboxEnabled) R.string.download_jukebox_on else R.string.download_jukebox_off, + if (jukeboxEnabled) R.string.download_jukebox_on + else R.string.download_jukebox_off, false ) return true @@ -718,7 +753,10 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon Timber.e(all) } }.start() - val msg = resources.getString(R.string.download_bookmark_set_at_position, bookmarkTime) + val msg = resources.getString( + R.string.download_bookmark_set_at_position, + bookmarkTime + ) Util.toast(context, msg) return true } @@ -788,7 +826,8 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon override fun error(error: Throwable) { Timber.e(error, "Exception has occurred in savePlaylistInBackground") - val msg = String.format(Locale.ROOT, + val msg = String.format( + Locale.ROOT, "%s %s", resources.getString(R.string.download_playlist_error), getErrorMessage(error) @@ -818,8 +857,9 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon private fun start() { val service = mediaPlayerController val state = service.playerState - if (state === PlayerState.PAUSED - || state === PlayerState.COMPLETED || state === PlayerState.STOPPED) { + if (state === PlayerState.PAUSED || + state === PlayerState.COMPLETED || state === PlayerState.STOPPED + ) { service.start() } else if (state === PlayerState.IDLE) { networkAndStorageChecker.warnIfNetworkOrStorageUnavailable() @@ -947,16 +987,16 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon val millisTotal = if (duration == null) 0 else duration!! positionTextView.text = Util.formatTotalDuration(millisPlayed.toLong(), true) durationTextView.text = Util.formatTotalDuration(millisTotal.toLong(), true) - progressBar!!.max = + progressBar.max = if (millisTotal == 0) 100 else millisTotal // Work-around for apparent bug. - progressBar!!.progress = millisPlayed - progressBar!!.isEnabled = currentPlaying!!.isWorkDone || isJukeboxEnabled + progressBar.progress = millisPlayed + progressBar.isEnabled = currentPlaying!!.isWorkDone || isJukeboxEnabled } else { positionTextView.setText(R.string.util_zero_time) durationTextView.setText(R.string.util_no_time) - progressBar!!.progress = 0 - progressBar!!.max = 0 - progressBar!!.isEnabled = false + progressBar.progress = 0 + progressBar.max = 0 + progressBar.isEnabled = false } when (playerState) { @@ -1034,7 +1074,7 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon } override fun done(result: Void?) { - progressBar!!.progress = seekTo + progressBar.progress = seekTo } }.execute() } @@ -1109,8 +1149,12 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon } private fun displaySongRating() { - val rating = - if (currentSong == null || currentSong!!.userRating == null) 0 else currentSong!!.userRating!! + var rating = 0 + + if (currentSong?.userRating != null) { + rating = currentSong!!.userRating!! + } + fiveStar1ImageView.setImageDrawable(if (rating > 0) fullStar else hollowStar) fiveStar2ImageView.setImageDrawable(if (rating > 1) fullStar else hollowStar) fiveStar3ImageView.setImageDrawable(if (rating > 2) fullStar else hollowStar) @@ -1125,15 +1169,10 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon } private fun showSavePlaylistDialog() { - val layoutInflater = - requireContext().getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater - val layout = layoutInflater.inflate( - R.layout.save_playlist, - requireActivity().findViewById(R.id.save_playlist_root) as ViewGroup - ) - if (layout != null) { - playlistNameView = layout.findViewById(R.id.save_playlist_name) - } + val layout = LayoutInflater.from(this.context).inflate(R.layout.save_playlist, null) + + playlistNameView = layout.findViewById(R.id.save_playlist_name) + val builder: AlertDialog.Builder = AlertDialog.Builder(context) builder.setTitle(R.string.download_playlist_title) builder.setMessage(R.string.download_playlist_name) @@ -1160,8 +1199,5 @@ class PlayerFragment : Fragment(), GestureDetector.OnGestureListener, KoinCompon companion object { private const val PERCENTAGE_OF_SCREEN_FOR_SWIPE = 5 - var progressBar // TODO: Refactor this to not be static - : SeekBar? = null - private set } } diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/LocalMediaPlayer.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/LocalMediaPlayer.kt index bb22c8f0..a8eb34b0 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/LocalMediaPlayer.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/LocalMediaPlayer.kt @@ -20,6 +20,7 @@ import android.os.Looper import android.os.PowerManager import android.os.PowerManager.PARTIAL_WAKE_LOCK import android.os.PowerManager.WakeLock +import androidx.lifecycle.MutableLiveData import java.io.File import java.net.URLEncoder import java.util.Locale @@ -29,7 +30,6 @@ import org.moire.ultrasonic.audiofx.EqualizerController import org.moire.ultrasonic.audiofx.VisualizerController import org.moire.ultrasonic.data.ActiveServerProvider.Companion.isOffline import org.moire.ultrasonic.domain.PlayerState -import org.moire.ultrasonic.fragment.PlayerFragment import org.moire.ultrasonic.util.CancellableTask import org.moire.ultrasonic.util.Constants import org.moire.ultrasonic.util.StreamProxy @@ -79,10 +79,12 @@ class LocalMediaPlayer( private var proxy: StreamProxy? = null private var bufferTask: CancellableTask? = null private var positionCache: PositionCache? = null - private var secondaryProgress = -1 + private val pm = context.getSystemService(POWER_SERVICE) as PowerManager private val wakeLock: WakeLock = pm.newWakeLock(PARTIAL_WAKE_LOCK, this.javaClass.name) + val secondaryProgress: MutableLiveData = MutableLiveData(0) + fun init() { Thread { Thread.currentThread().name = "MediaPlayerThread" @@ -357,7 +359,6 @@ class LocalMediaPlayer( downloadFile.updateModificationDate() mediaPlayer.setOnCompletionListener(null) - secondaryProgress = -1 // Ensure seeking in non StreamProxy playback works setPlayerState(PlayerState.IDLE) setAudioAttributes(mediaPlayer) @@ -388,28 +389,28 @@ class LocalMediaPlayer( setPlayerState(PlayerState.PREPARING) mediaPlayer.setOnBufferingUpdateListener { mp, percent -> - val progressBar = PlayerFragment.progressBar val song = downloadFile.song if (percent == 100) { mp.setOnBufferingUpdateListener(null) } - secondaryProgress = (percent.toDouble() / 100.toDouble() * progressBar.max).toInt() - + // The secondary progress is an indicator of how far the song is cached. if (song.transcodedContentType == null && Util.getMaxBitRate() == 0) { - progressBar?.secondaryProgress = secondaryProgress + val progress = (percent.toDouble() / 100.toDouble() * playerDuration).toInt() + secondaryProgress.postValue(progress) } } mediaPlayer.setOnPreparedListener { Timber.i("Media player prepared") setPlayerState(PlayerState.PREPARED) - val progressBar = PlayerFragment.progressBar - if (progressBar != null && downloadFile.isWorkDone) { - // Populate seek bar secondary progress if we have a complete file for consistency - PlayerFragment.progressBar.secondaryProgress = 100 * progressBar.max + + // Populate seek bar secondary progress if we have a complete file for consistency + if (downloadFile.isWorkDone) { + secondaryProgress.postValue(playerDuration) } + synchronized(this@LocalMediaPlayer) { if (position != 0) { Timber.i("Restarting player from position %d", position) diff --git a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerController.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerController.kt index ac424e20..b18eb0fa 100644 --- a/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerController.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/MediaPlayerController.kt @@ -386,12 +386,6 @@ class MediaPlayerController( @get:Synchronized val playerDuration: Int get() { - if (localMediaPlayer.currentPlaying != null) { - val duration = localMediaPlayer.currentPlaying!!.song.duration - if (duration != null) { - return duration * 1000 - } - } val mediaPlayerService = runningInstance ?: return 0 return mediaPlayerService.playerDuration } diff --git a/ultrasonic/src/main/java/org/moire/ultrasonic/util/SilentBackgroundTask.kt b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/SilentBackgroundTask.kt similarity index 50% rename from ultrasonic/src/main/java/org/moire/ultrasonic/util/SilentBackgroundTask.kt rename to ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/SilentBackgroundTask.kt index 3693944e..3639aa2c 100644 --- a/ultrasonic/src/main/java/org/moire/ultrasonic/util/SilentBackgroundTask.kt +++ b/ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/SilentBackgroundTask.kt @@ -1,21 +1,10 @@ /* - This file is part of Subsonic. - - Subsonic is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - Subsonic is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with Subsonic. If not, see . - - Copyright 2010 (C) Sindre Mehus + * SilentBackgroundTask.kt + * Copyright (C) 2009-2021 Ultrasonic developers + * + * Distributed under terms of the GNU GPLv3 license. */ + package org.moire.ultrasonic.util import android.app.Activity