From 770dcc1832454635f1db735c26960bcd318a20cc Mon Sep 17 00:00:00 2001 From: John Zhen M Date: Tue, 10 Oct 2017 12:25:48 -0700 Subject: [PATCH] -Fixed incorrect indexing due to item removed after shuffle. -Fixed activity binding not unbound after service shutdown. --- .../newpipe/player/BackgroundPlayer.java | 2 +- .../player/BackgroundPlayerActivity.java | 54 ++++++++++++------- .../schabi/newpipe/playlist/PlayQueue.java | 25 +++++++-- 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java b/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java index fd8687d97..c48de1ed4 100644 --- a/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java @@ -469,8 +469,8 @@ public final class BackgroundPlayer extends Service { @Override public void shutdown() { super.shutdown(); - stopSelf(); stopActivityBinding(); + stopSelf(); } /*////////////////////////////////////////////////////////////////////////// diff --git a/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayerActivity.java b/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayerActivity.java index cc632a473..05ba67cdd 100644 --- a/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayerActivity.java +++ b/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayerActivity.java @@ -53,7 +53,7 @@ public class BackgroundPlayerActivity extends AppCompatActivity //////////////////////////////////////////////////////////////////////////// private static final int RECYCLER_ITEM_POPUP_MENU_GROUP_ID = 47; - private static final int PLAYBACK_SPEED_POPUP_MENU_GROUP_ID = 61; + private static final int PLAYBACK_SPEED_POPUP_MENU_GROUP_ID = 61; private static final int PLAYBACK_PITCH_POPUP_MENU_GROUP_ID = 97; private View rootView; @@ -92,8 +92,10 @@ public class BackgroundPlayerActivity extends AppCompatActivity final Toolbar toolbar = rootView.findViewById(R.id.toolbar); setSupportActionBar(toolbar); - getSupportActionBar().setDisplayHomeAsUpEnabled(true); - getSupportActionBar().setTitle(R.string.title_activity_background_player); + if (getSupportActionBar() != null) { + getSupportActionBar().setDisplayHomeAsUpEnabled(true); + getSupportActionBar().setTitle(R.string.title_activity_background_player); + } serviceConnection = backgroundPlayerConnection(); } @@ -101,9 +103,7 @@ public class BackgroundPlayerActivity extends AppCompatActivity @Override protected void onStart() { super.onStart(); - final Intent mIntent = new Intent(this, BackgroundPlayer.class); - final boolean success = bindService(mIntent, serviceConnection, BIND_AUTO_CREATE); - if (!success) unbindService(serviceConnection); + bind(); } @Override @@ -123,24 +123,36 @@ public class BackgroundPlayerActivity extends AppCompatActivity @Override protected void onStop() { super.onStop(); - if(serviceBound) { - unbindService(serviceConnection); - serviceBound = false; - } + unbind(); } //////////////////////////////////////////////////////////////////////////// // Service Connection //////////////////////////////////////////////////////////////////////////// + private void bind() { + final Intent mIntent = new Intent(this, BackgroundPlayer.class); + final boolean success = bindService(mIntent, serviceConnection, BIND_AUTO_CREATE); + if (!success) { + unbindService(serviceConnection); + } + serviceBound = success; + } + + private void unbind() { + if(serviceBound) { + unbindService(serviceConnection); + serviceBound = false; + player = null; + finish(); + } + } + private ServiceConnection backgroundPlayerConnection() { return new ServiceConnection() { @Override public void onServiceDisconnected(ComponentName name) { Log.d(TAG, "Background player service is disconnected"); - serviceBound = false; - player = null; - finish(); } @Override @@ -149,12 +161,9 @@ public class BackgroundPlayerActivity extends AppCompatActivity final BackgroundPlayer.LocalBinder mLocalBinder = (BackgroundPlayer.LocalBinder) service; player = mLocalBinder.getBackgroundPlayerInstance(); if (player == null) { - finish(); + unbind(); } else { - serviceBound = true; buildComponents(); - - player.setActivityListener(BackgroundPlayerActivity.this); } } }; @@ -169,6 +178,7 @@ public class BackgroundPlayerActivity extends AppCompatActivity buildMetadata(); buildSeekBar(); buildControls(); + buildListeners(); } private void buildQueue() { @@ -220,6 +230,10 @@ public class BackgroundPlayerActivity extends AppCompatActivity buildPlaybackPitchMenu(); } + private void buildListeners() { + player.setActivityListener(this); + } + private void buildPlaybackSpeedMenu() { if (playbackSpeedPopupMenu == null) return; @@ -241,11 +255,11 @@ public class BackgroundPlayerActivity extends AppCompatActivity private void buildPlaybackPitchMenu() { if (playbackPitchPopupMenu == null) return; - playbackPitchPopupMenu.getMenu().removeGroup(PLAYBACK_SPEED_POPUP_MENU_GROUP_ID); + playbackPitchPopupMenu.getMenu().removeGroup(PLAYBACK_PITCH_POPUP_MENU_GROUP_ID); for (int i = 0; i < BasePlayer.PLAYBACK_PITCHES.length; i++) { final float playbackPitch = BasePlayer.PLAYBACK_PITCHES[i]; final String formattedPitch = player.formatPitch(playbackPitch); - final MenuItem item = playbackPitchPopupMenu.getMenu().add(PLAYBACK_SPEED_POPUP_MENU_GROUP_ID, i, Menu.NONE, formattedPitch); + final MenuItem item = playbackPitchPopupMenu.getMenu().add(PLAYBACK_PITCH_POPUP_MENU_GROUP_ID, i, Menu.NONE, formattedPitch); item.setOnMenuItemClickListener(new MenuItem.OnMenuItemClickListener() { @Override public boolean onMenuItemClick(MenuItem menuItem) { @@ -488,6 +502,6 @@ public class BackgroundPlayerActivity extends AppCompatActivity @Override public void onServiceStopped() { - finish(); + unbind(); } } diff --git a/app/src/main/java/org/schabi/newpipe/playlist/PlayQueue.java b/app/src/main/java/org/schabi/newpipe/playlist/PlayQueue.java index 4d0372be1..e52bca98a 100644 --- a/app/src/main/java/org/schabi/newpipe/playlist/PlayQueue.java +++ b/app/src/main/java/org/schabi/newpipe/playlist/PlayQueue.java @@ -274,11 +274,11 @@ public abstract class PlayQueue implements Serializable { queueIndex.set(0); } - streams.remove(index); if (backup != null) { final int backupIndex = backup.indexOf(getItem(index)); backup.remove(backupIndex); } + streams.remove(index); } public synchronized void move(final int source, final int target) { @@ -303,7 +303,8 @@ public abstract class PlayQueue implements Serializable { * * This method first backs up the existing play queue and item being played. * Then a newly shuffled play queue will be generated along with the index of - * the previously playing item. + * the previously playing item if it is found in the shuffled play queue. If + * not found, the current index will reset to 0. * * Will emit a {@link ReorderEvent} in any context. * */ @@ -313,7 +314,13 @@ public abstract class PlayQueue implements Serializable { } final PlayQueueItem current = getItem(); Collections.shuffle(streams); - queueIndex.set(streams.indexOf(current)); + + final int newIndex = streams.indexOf(current); + if (newIndex != -1) { + queueIndex.set(newIndex); + } else { + queueIndex.set(0); + } broadcast(new ReorderEvent()); } @@ -321,17 +328,25 @@ public abstract class PlayQueue implements Serializable { /** * Unshuffles the current play queue if a backup play queue exists. * - * This method undoes shuffling and index will be set to the previously playing item. + * This method undoes shuffling and index will be set to the previously playing item if found, + * otherwise, the index will reset to 0. * * Will emit a {@link ReorderEvent} if a backup exists. * */ public synchronized void unshuffle() { if (backup == null) return; final PlayQueueItem current = getItem(); + streams.clear(); streams = backup; backup = null; - queueIndex.set(streams.indexOf(current)); + + final int newIndex = streams.indexOf(current); + if (newIndex != -1) { + queueIndex.set(newIndex); + } else { + queueIndex.set(0); + } broadcast(new ReorderEvent()); }