From 049bf9062028071ad567325ad7b1c0b1d98cf927 Mon Sep 17 00:00:00 2001 From: Karim Abou Zeid Date: Fri, 19 Jun 2015 12:29:40 +0200 Subject: [PATCH] Cleaned up MusicService and fixed ISE and NPEs there. Replaced thread with handler for updating the progress bar in MusicControllerActivity for simplicity and performance. --- .../gramophone/service/MultiPlayer.java | 5 +- .../gramophone/service/MusicService.java | 45 ++++--- .../activities/MusicControllerActivity.java | 117 ++++++++++++------ 3 files changed, 110 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/com/kabouzeid/gramophone/service/MultiPlayer.java b/app/src/main/java/com/kabouzeid/gramophone/service/MultiPlayer.java index 00fc8852..6136f96f 100644 --- a/app/src/main/java/com/kabouzeid/gramophone/service/MultiPlayer.java +++ b/app/src/main/java/com/kabouzeid/gramophone/service/MultiPlayer.java @@ -16,7 +16,7 @@ import com.kabouzeid.gramophone.util.PreferenceUtils; import java.io.IOException; import java.lang.ref.WeakReference; -public final class MultiPlayer implements MediaPlayer.OnErrorListener, +public class MultiPlayer implements MediaPlayer.OnErrorListener, MediaPlayer.OnCompletionListener { public static final String TAG = MultiPlayer.class.getSimpleName(); @@ -234,8 +234,7 @@ public final class MultiPlayer implements MediaPlayer.OnErrorListener, */ @Override public boolean onError(final MediaPlayer mp, final int what, final int extra) { - Toast.makeText(mService.get().getApplicationContext(), mService.get().getResources().getString(R.string.unplayable_file), Toast.LENGTH_SHORT).show(); - mService.get().playNextSong(true); + Toast.makeText(mService.get(), mService.get().getResources().getString(R.string.unplayable_file), Toast.LENGTH_SHORT).show(); return false; } diff --git a/app/src/main/java/com/kabouzeid/gramophone/service/MusicService.java b/app/src/main/java/com/kabouzeid/gramophone/service/MusicService.java index 298298e3..75a0f1ad 100644 --- a/app/src/main/java/com/kabouzeid/gramophone/service/MusicService.java +++ b/app/src/main/java/com/kabouzeid/gramophone/service/MusicService.java @@ -308,35 +308,45 @@ public class MusicService extends Service { playSongAt(getNextPosition(force)); } - private void openTrackAndPrepareNextAt(int position) { + private boolean openTrackAndPrepareNextAt(int position) { synchronized (this) { setPosition(position); - openCurrent(); + boolean prepared = openCurrent(); prepareNext(); notifyChange(META_CHANGED); + return prepared; } } - private void openCurrent() { + private boolean openCurrent() { synchronized (this) { - player.setDataSource(getTrackUri(getCurrentSong())); + try { + player.setDataSource(getTrackUri(getCurrentSong())); + return true; + } catch (Exception e) { + return false; + } } } - private void prepareNext() { + private boolean prepareNext() { synchronized (this) { - nextPosition = getNextPosition(false); - player.setNextDataSource(getTrackUri(getSongAt(nextPosition))); + try { + int nextPosition = getNextPosition(false); + player.setNextDataSource(getTrackUri(getSongAt(nextPosition))); + this.nextPosition = nextPosition; + return true; + } catch (Exception e) { + return false; + } } } private void closeAudioEffectSession() { - if (player != null) { - final Intent audioEffectsIntent = new Intent(AudioEffect.ACTION_CLOSE_AUDIO_EFFECT_CONTROL_SESSION); - audioEffectsIntent.putExtra(AudioEffect.EXTRA_AUDIO_SESSION, player.getAudioSessionId()); - audioEffectsIntent.putExtra(AudioEffect.EXTRA_PACKAGE_NAME, getPackageName()); - sendBroadcast(audioEffectsIntent); - } + final Intent audioEffectsIntent = new Intent(AudioEffect.ACTION_CLOSE_AUDIO_EFFECT_CONTROL_SESSION); + audioEffectsIntent.putExtra(AudioEffect.EXTRA_AUDIO_SESSION, player.getAudioSessionId()); + audioEffectsIntent.putExtra(AudioEffect.EXTRA_PACKAGE_NAME, getPackageName()); + sendBroadcast(audioEffectsIntent); } private boolean requestFocus() { @@ -344,7 +354,7 @@ public class MusicService extends Service { } private void updateRemoteControlClient() { - final Song song = playingQueue.get(getPosition()); + final Song song = getCurrentSong(); remoteControlClient .editMetadata(false) .putString(MediaMetadataRetriever.METADATA_KEY_ARTIST, song.artistName) @@ -580,8 +590,11 @@ public class MusicService extends Service { } private void playSongAtImpl(int position) { - openTrackAndPrepareNextAt(position); - play(false); + if (openTrackAndPrepareNextAt(position)) { + play(false); + } else { + Toast.makeText(this, getResources().getString(R.string.unplayable_file), Toast.LENGTH_SHORT).show(); + } } public void pause(boolean forceNoFading) { diff --git a/app/src/main/java/com/kabouzeid/gramophone/ui/activities/MusicControllerActivity.java b/app/src/main/java/com/kabouzeid/gramophone/ui/activities/MusicControllerActivity.java index 4529906f..8f6d91d3 100644 --- a/app/src/main/java/com/kabouzeid/gramophone/ui/activities/MusicControllerActivity.java +++ b/app/src/main/java/com/kabouzeid/gramophone/ui/activities/MusicControllerActivity.java @@ -9,6 +9,10 @@ import android.graphics.Color; import android.graphics.PorterDuff; import android.os.Build; import android.os.Bundle; +import android.os.Handler; +import android.os.HandlerThread; +import android.os.Looper; +import android.os.Message; import android.support.v7.graphics.Palette; import android.support.v7.widget.CardView; import android.support.v7.widget.Toolbar; @@ -53,11 +57,13 @@ import com.nostra13.universalimageloader.core.assist.FailReason; import com.nostra13.universalimageloader.core.listener.SimpleImageLoadingListener; import java.io.File; +import java.lang.ref.WeakReference; public class MusicControllerActivity extends AbsFabActivity { public static final String TAG = MusicControllerActivity.class.getSimpleName(); private static final int COLOR_TRANSITION_TIME = 400; + private static final int UPDATE_PROGRESS_VIEWS = 1; private Song song; private SquareIfPlaceImageView albumArt; @@ -77,7 +83,8 @@ public class MusicControllerActivity extends AbsFabActivity { private Toolbar toolbar; private int lastFooterColor = -1; private int lastTextColor = -2; - private Thread progressViewsUpdateThread; + private Handler progressViewsUpdateHandler; + private HandlerThread handlerThread; private boolean opaqueStatusBar; private boolean opaqueToolBar; @@ -92,7 +99,7 @@ public class MusicControllerActivity extends AbsFabActivity { super.onCreate(savedInstanceState); - initAppeareanceVars(); + initAppearanceVarsFromSharedPrefs(); setContentView(alternativeProgressSlider ? R.layout.activity_music_controller_alternative_progress_slider : R.layout.activity_music_controller); @@ -101,10 +108,14 @@ public class MusicControllerActivity extends AbsFabActivity { adjustTitleBoxSize(); setUpPlaybackControllerCard(); setUpMusicControllers(); + setUpAlbumArtViews(); + setUpToolbar(); + animateFabCircularRevealOnEnterTransitionEnd(); + } + private void setUpAlbumArtViews() { albumArtBackground.setAlpha(0.7f); albumArt.forceSquare(forceSquareAlbumArt); - if (opaqueStatusBar) { if (opaqueToolBar) { alignAlbumArtToToolbar(); @@ -114,11 +125,16 @@ public class MusicControllerActivity extends AbsFabActivity { } else { alignAlbumArtToTop(); } + } + private void setUpToolbar() { setSupportActionBar(toolbar); + //noinspection ConstantConditions getSupportActionBar().setTitle(null); getSupportActionBar().setDisplayHomeAsUpEnabled(true); + } + private void animateFabCircularRevealOnEnterTransitionEnd() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { getWindow().getEnterTransition().addListener(new SmallTransitionListener() { @Override @@ -143,7 +159,31 @@ public class MusicControllerActivity extends AbsFabActivity { } } - private void initAppeareanceVars() { + private void startHandler() { + handlerThread = new HandlerThread("MusicProgressViewUpdateHandler", + android.os.Process.THREAD_PRIORITY_BACKGROUND); + handlerThread.start(); + progressViewsUpdateHandler = new MusicProgressViewsUpdateHandler(this, handlerThread.getLooper()); + } + + private void stopHandler() { + progressViewsUpdateHandler.removeCallbacksAndMessages(null); + if (Build.VERSION.SDK_INT >= 18) { + handlerThread.quitSafely(); + } else { + handlerThread.quit(); + } + } + + private void startUpdatingProgressViews() { + progressViewsUpdateHandler.sendEmptyMessage(UPDATE_PROGRESS_VIEWS); + } + + private void stopUpdatingProgressViews() { + progressViewsUpdateHandler.removeMessages(UPDATE_PROGRESS_VIEWS); + } + + private void initAppearanceVarsFromSharedPrefs() { opaqueStatusBar = PreferenceUtils.getInstance(this).opaqueStatusbarNowPlaying(); opaqueToolBar = opaqueStatusBar && PreferenceUtils.getInstance(this).opaqueToolbarNowPlaying(); forceSquareAlbumArt = PreferenceUtils.getInstance(this).forceAlbumArtSquared(); @@ -320,8 +360,16 @@ public class MusicControllerActivity extends AbsFabActivity { protected void onResume() { super.onResume(); updateControllerState(); - startProgressViewsUpdateThread(); updateCurrentSong(); + startHandler(); + startUpdatingProgressViews(); + } + + @Override + protected void onPause() { + super.onPause(); + stopUpdatingProgressViews(); + stopHandler(); } private void updateCurrentSong() { @@ -440,38 +488,19 @@ public class MusicControllerActivity extends AbsFabActivity { } } - private void startProgressViewsUpdateThread() { - if (progressViewsUpdateThread != null) progressViewsUpdateThread.interrupt(); - progressViewsUpdateThread = new Thread(new Runnable() { - int totalMillis = 0; - int progressMillis = 0; + private void updateProgressViews() { + final int totalMillis = MusicPlayerRemote.getSongDurationMillis(); + final int progressMillis = MusicPlayerRemote.getSongProgressMillis(); + runOnUiThread(new Runnable() { @Override public void run() { - try { - while (!Thread.currentThread().isInterrupted()) { - totalMillis = MusicPlayerRemote.getSongDurationMillis(); - progressMillis = MusicPlayerRemote.getSongProgressMillis(); - - runOnUiThread(updateProgressViews); - - Thread.sleep(100); - } - } catch (InterruptedException ignored) { - } + progressSlider.setMax(totalMillis); + progressSlider.setProgress(progressMillis); + currentSongProgress.setText(MusicUtil.getReadableDurationString(progressMillis)); + totalSongDuration.setText(MusicUtil.getReadableDurationString(totalMillis)); } - - private Runnable updateProgressViews = new Runnable() { - @Override - public void run() { - progressSlider.setMax(totalMillis); - progressSlider.setProgress(progressMillis); - currentSongProgress.setText(MusicUtil.getReadableDurationString(progressMillis)); - totalSongDuration.setText(MusicUtil.getReadableDurationString(totalMillis)); - } - }; }); - progressViewsUpdateThread.start(); } protected void updateControllerState() { @@ -498,12 +527,6 @@ public class MusicControllerActivity extends AbsFabActivity { updateShuffleState(); } - @Override - protected void onPause() { - super.onPause(); - if (progressViewsUpdateThread != null) progressViewsUpdateThread.interrupt(); - } - @Override public boolean onCreateOptionsMenu(Menu menu) { getMenuInflater().inflate(R.menu.menu_music_playing, menu); @@ -573,4 +596,22 @@ public class MusicControllerActivity extends AbsFabActivity { RelativeLayout.LayoutParams params = (RelativeLayout.LayoutParams) findViewById(R.id.album_art_frame).getLayoutParams(); params.addRule(RelativeLayout.BELOW, R.id.status_bar); } + + private static class MusicProgressViewsUpdateHandler extends Handler { + private WeakReference activityReference; + + public MusicProgressViewsUpdateHandler(final MusicControllerActivity activity, final Looper looper) { + super(looper); + activityReference = new WeakReference<>(activity); + } + + @Override + public void handleMessage(Message msg) { + super.handleMessage(msg); + if (msg.what == UPDATE_PROGRESS_VIEWS) { + activityReference.get().updateProgressViews(); + sendEmptyMessageDelayed(UPDATE_PROGRESS_VIEWS, 100); + } + } + } } \ No newline at end of file