From c6e759a94c67f63a07a8f00d16e025ca92d8b143 Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Thu, 26 Oct 2017 13:42:50 -0700 Subject: [PATCH] -Fixed popup player not playing in foreground. -Fixed activity binder memory leak in popup and background players. -Fixed out of bound window after removing last item on play queue. -Fixed MediaSourceManager continues to process update after disposed. -Changed play queue append to shuffle if queue is already shuffled. --- .../newpipe/player/BackgroundPlayer.java | 26 +++--- .../player/BackgroundPlayerActivity.java | 7 -- .../org/schabi/newpipe/player/BasePlayer.java | 8 +- .../newpipe/player/MainVideoPlayer.java | 15 +++- .../newpipe/player/PlayerServiceBinder.java | 16 ++++ .../newpipe/player/PopupVideoPlayer.java | 79 +++++++++++-------- .../player/PopupVideoPlayerActivity.java | 7 -- .../newpipe/player/ServicePlayerActivity.java | 8 +- .../player/playback/MediaSourceManager.java | 5 +- .../schabi/newpipe/playlist/PlayQueue.java | 40 ++++++---- .../newpipe/playlist/PlayQueueAdapter.java | 11 +-- .../newpipe/playlist/events/ErrorEvent.java | 16 ++-- .../newpipe/playlist/events/RemoveEvent.java | 16 ++-- 13 files changed, 149 insertions(+), 105 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/player/PlayerServiceBinder.java 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 7a9a0eb3a..2881dd69c 100644 --- a/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayer.java @@ -27,7 +27,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.graphics.Bitmap; import android.net.wifi.WifiManager; -import android.os.Binder; import android.os.Build; import android.os.IBinder; import android.os.PowerManager; @@ -84,12 +83,6 @@ public final class BackgroundPlayer extends Service { private PlayerEventListener activityListener; private IBinder mBinder; - class LocalBinder extends Binder { - BasePlayerImpl getBackgroundPlayerInstance() { - return BackgroundPlayer.this.basePlayerImpl; - } - } - /*////////////////////////////////////////////////////////////////////////// // Notification //////////////////////////////////////////////////////////////////////////*/ @@ -116,10 +109,10 @@ public final class BackgroundPlayer extends Service { wifiManager = ((WifiManager) getApplicationContext().getSystemService(WIFI_SERVICE)); ThemeHelper.setTheme(this); - basePlayerImpl = new BasePlayerImpl(this); + basePlayerImpl = new BasePlayerImpl(getApplicationContext()); basePlayerImpl.setup(); - mBinder = new LocalBinder(); + mBinder = new PlayerServiceBinder(basePlayerImpl); shouldUpdateOnProgress = true; } @@ -155,16 +148,19 @@ public final class BackgroundPlayer extends Service { } private void onClose() { - stopForeground(true); + if (DEBUG) Log.d(TAG, "onClose() called"); + releaseWifiAndCpu(); if (basePlayerImpl != null) { basePlayerImpl.stopActivityBinding(); basePlayerImpl.destroy(); } - - basePlayerImpl = null; + if (notificationManager != null) notificationManager.cancel(NOTIFICATION_ID); mBinder = null; + basePlayerImpl = null; + + stopForeground(true); stopSelf(); } @@ -322,6 +318,7 @@ public final class BackgroundPlayer extends Service { updateNotification(-1); } + clearThumbnailCache(); } @Override @@ -460,8 +457,7 @@ public final class BackgroundPlayer extends Service { @Override public void shutdown() { super.shutdown(); - stopActivityBinding(); - stopSelf(); + onClose(); } /*////////////////////////////////////////////////////////////////////////// @@ -538,7 +534,7 @@ public final class BackgroundPlayer extends Service { onVideoPlayPause(); break; case ACTION_OPEN_DETAIL: - openControl(BackgroundPlayer.this); + openControl(getApplicationContext()); break; case ACTION_REPEAT: onRepeatClicked(); 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 bfd066885..c65e8aa2d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayerActivity.java +++ b/app/src/main/java/org/schabi/newpipe/player/BackgroundPlayerActivity.java @@ -1,7 +1,6 @@ package org.schabi.newpipe.player; import android.content.Intent; -import android.os.IBinder; import org.schabi.newpipe.R; @@ -19,12 +18,6 @@ public final class BackgroundPlayerActivity extends ServicePlayerActivity { return getResources().getString(R.string.title_activity_background_player); } - @Override - public BasePlayer playerFrom(IBinder binder) { - final BackgroundPlayer.LocalBinder mLocalBinder = (BackgroundPlayer.LocalBinder) binder; - return mLocalBinder.getBackgroundPlayerInstance(); - } - @Override public Intent getBindIntent() { return new Intent(this, BackgroundPlayer.class); diff --git a/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java b/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java index 2c0c05d66..67f4fc311 100644 --- a/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/BasePlayer.java @@ -315,6 +315,8 @@ public abstract class BasePlayer implements Player.EventListener, public void destroy() { if (DEBUG) Log.d(TAG, "destroy() called"); destroyPlayer(); + clearThumbnailCache(); + unregisterBroadcastReceiver(); if (playQueue != null) { playQueue.dispose(); @@ -325,8 +327,6 @@ public abstract class BasePlayer implements Player.EventListener, playbackManager = null; } - unregisterBroadcastReceiver(); - trackSelector = null; simpleExoPlayer = null; } @@ -902,6 +902,10 @@ public abstract class BasePlayer implements Player.EventListener, } } + protected void clearThumbnailCache() { + ImageLoader.getInstance().clearMemoryCache(); + } + protected void startProgressLoop() { if (progressUpdateReactor != null) progressUpdateReactor.dispose(); progressUpdateReactor = getProgressReactor(); diff --git a/app/src/main/java/org/schabi/newpipe/player/MainVideoPlayer.java b/app/src/main/java/org/schabi/newpipe/player/MainVideoPlayer.java index f41c88f54..230b963a0 100644 --- a/app/src/main/java/org/schabi/newpipe/player/MainVideoPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/MainVideoPlayer.java @@ -20,8 +20,10 @@ package org.schabi.newpipe.player; import android.app.Activity; +import android.content.Context; import android.content.Intent; import android.content.pm.ActivityInfo; +import android.graphics.Bitmap; import android.graphics.Color; import android.media.AudioManager; import android.os.Build; @@ -44,6 +46,7 @@ import android.widget.Toast; import com.google.android.exoplayer2.Player; import com.google.android.exoplayer2.trackselection.DefaultTrackSelector; +import com.nostra13.universalimageloader.core.ImageLoader; import org.schabi.newpipe.R; import org.schabi.newpipe.extractor.stream.StreamInfo; @@ -97,7 +100,7 @@ public final class MainVideoPlayer extends Activity { showSystemUi(); setContentView(R.layout.activity_main_player); - playerImpl = new VideoPlayerImpl(); + playerImpl = new VideoPlayerImpl(getApplicationContext()); playerImpl.setup(findViewById(android.R.id.content)); playerImpl.handleIntent(getIntent()); } @@ -243,8 +246,8 @@ public final class MainVideoPlayer extends Activity { private boolean queueVisible; - VideoPlayerImpl() { - super("VideoPlayerImpl" + MainVideoPlayer.TAG, MainVideoPlayer.this); + VideoPlayerImpl(final Context context) { + super("VideoPlayerImpl" + MainVideoPlayer.TAG, context); } @Override @@ -285,6 +288,12 @@ public final class MainVideoPlayer extends Activity { screenRotationButton.setOnClickListener(this); } + @Override + public void onThumbnailReceived(Bitmap thumbnail) { + super.onThumbnailReceived(thumbnail); + clearThumbnailCache(); + } + /*////////////////////////////////////////////////////////////////////////// // ExoPlayer Video Listener //////////////////////////////////////////////////////////////////////////*/ diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerServiceBinder.java b/app/src/main/java/org/schabi/newpipe/player/PlayerServiceBinder.java new file mode 100644 index 000000000..80c27be7f --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerServiceBinder.java @@ -0,0 +1,16 @@ +package org.schabi.newpipe.player; + +import android.os.Binder; +import android.support.annotation.NonNull; + +class PlayerServiceBinder extends Binder { + private final BasePlayer basePlayer; + + PlayerServiceBinder(@NonNull final BasePlayer basePlayer) { + this.basePlayer = basePlayer; + } + + BasePlayer getPlayerInstance() { + return basePlayer; + } +} diff --git a/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java b/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java index 9baee3a8b..7623b1d74 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayer.java @@ -30,7 +30,6 @@ import android.content.SharedPreferences; import android.content.res.Configuration; import android.graphics.Bitmap; import android.graphics.PixelFormat; -import android.os.Binder; import android.os.Build; import android.os.Handler; import android.os.IBinder; @@ -133,12 +132,6 @@ public final class PopupVideoPlayer extends Service { private PlayerEventListener activityListener; private IBinder mBinder; - class LocalBinder extends Binder { - VideoPlayerImpl getPopupPlayerInstance() { - return PopupVideoPlayer.this.playerImpl; - } - } - /*////////////////////////////////////////////////////////////////////////// // Service LifeCycle //////////////////////////////////////////////////////////////////////////*/ @@ -148,21 +141,20 @@ public final class PopupVideoPlayer extends Service { windowManager = (WindowManager) getSystemService(WINDOW_SERVICE); notificationManager = ((NotificationManager) getSystemService(NOTIFICATION_SERVICE)); - playerImpl = new VideoPlayerImpl(); + playerImpl = new VideoPlayerImpl(getApplicationContext()); ThemeHelper.setTheme(this); - mBinder = new LocalBinder(); + mBinder = new PlayerServiceBinder(playerImpl); } @Override - @SuppressWarnings("unchecked") public int onStartCommand(final Intent intent, int flags, int startId) { if (DEBUG) Log.d(TAG, "onStartCommand() called with: intent = [" + intent + "], flags = [" + flags + "], startId = [" + startId + "]"); if (playerImpl.getPlayer() == null) initPopup(); if (!playerImpl.isPlaying()) playerImpl.getPlayer().setPlayWhenReady(true); - if (intent.getStringExtra(Constants.KEY_URL) != null) { + if (intent != null && intent.getStringExtra(Constants.KEY_URL) != null) { final int serviceId = intent.getIntExtra(Constants.KEY_SERVICE_ID, 0); final String url = intent.getStringExtra(Constants.KEY_URL); @@ -200,14 +192,7 @@ public final class PopupVideoPlayer extends Service { @Override public void onDestroy() { if (DEBUG) Log.d(TAG, "onDestroy() called"); - stopForeground(true); - if (playerImpl != null) { - playerImpl.destroy(); - if (playerImpl.getRootView() != null) windowManager.removeView(playerImpl.getRootView()); - } - if (notificationManager != null) notificationManager.cancel(NOTIFICATION_ID); - if (currentWorker != null) currentWorker.dispose(); - savePositionAndSize(); + onClose(); } @Override @@ -251,7 +236,6 @@ public final class PopupVideoPlayer extends Service { MySimpleOnGestureListener listener = new MySimpleOnGestureListener(); gestureDetector = new GestureDetector(this, listener); - //gestureDetector.setIsLongpressEnabled(false); rootView.setOnTouchListener(listener); playerImpl.getLoadingPanel().setMinimumWidth(windowLayoutParams.width); playerImpl.getLoadingPanel().setMinimumHeight(windowLayoutParams.height); @@ -262,6 +246,10 @@ public final class PopupVideoPlayer extends Service { // Notification //////////////////////////////////////////////////////////////////////////*/ + private void resetNotification() { + notBuilder = createNotification(); + } + private NotificationCompat.Builder createNotification() { notRemoteView = new RemoteViews(BuildConfig.APPLICATION_ID, R.layout.player_popup_notification); @@ -303,9 +291,23 @@ public final class PopupVideoPlayer extends Service { // Misc //////////////////////////////////////////////////////////////////////////*/ - public void onVideoClose() { - if (DEBUG) Log.d(TAG, "onVideoClose() called"); - playerImpl.stopActivityBinding(); + public void onClose() { + if (DEBUG) Log.d(TAG, "onClose() called"); + + if (playerImpl != null) { + if (playerImpl.getRootView() != null) { + windowManager.removeView(playerImpl.getRootView()); + playerImpl.setRootView(null); + } + playerImpl.stopActivityBinding(); + playerImpl.destroy(); + } + if (notificationManager != null) notificationManager.cancel(NOTIFICATION_ID); + if (currentWorker != null) currentWorker.dispose(); + mBinder = null; + playerImpl = null; + + stopForeground(true); stopSelf(); } @@ -399,8 +401,16 @@ public final class PopupVideoPlayer extends Service { protected class VideoPlayerImpl extends VideoPlayer { private TextView resizingIndicator; - VideoPlayerImpl() { - super("VideoPlayerImpl" + PopupVideoPlayer.TAG, PopupVideoPlayer.this); + @Override + public void handleIntent(Intent intent) { + super.handleIntent(intent); + + resetNotification(); + startForeground(NOTIFICATION_ID, notBuilder.build()); + } + + VideoPlayerImpl(final Context context) { + super("VideoPlayerImpl" + PopupVideoPlayer.TAG, context); } @Override @@ -411,8 +421,8 @@ public final class PopupVideoPlayer extends Service { @Override public void destroy() { - super.destroy(); if (notRemoteView != null) notRemoteView.setImageViewBitmap(R.id.notificationCover, null); + super.destroy(); } @Override @@ -426,6 +436,7 @@ public final class PopupVideoPlayer extends Service { updateNotification(-1); } + clearThumbnailCache(); } @Override @@ -434,7 +445,7 @@ public final class PopupVideoPlayer extends Service { if (DEBUG) Log.d(TAG, "onFullScreenButtonClicked() called"); - playerImpl.setRecovery(); + setRecovery(); Intent intent; if (!getSharedPreferences().getBoolean(getResources().getString(R.string.use_old_player_key), false)) { intent = NavigationHelper.getPlayerIntent( @@ -457,9 +468,7 @@ public final class PopupVideoPlayer extends Service { intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); } context.startActivity(intent); - playerImpl.stopActivityBinding(); - destroyPlayer(); - stopSelf(); + onClose(); } @Override @@ -595,8 +604,7 @@ public final class PopupVideoPlayer extends Service { @Override public void shutdown() { super.shutdown(); - stopActivityBinding(); - stopSelf(); + onClose(); } /*////////////////////////////////////////////////////////////////////////// @@ -619,16 +627,17 @@ public final class PopupVideoPlayer extends Service { @Override public void onBroadcastReceived(Intent intent) { super.onBroadcastReceived(intent); + if (intent == null || intent.getAction() == null) return; if (DEBUG) Log.d(TAG, "onBroadcastReceived() called with: intent = [" + intent + "]"); switch (intent.getAction()) { case ACTION_CLOSE: - onVideoClose(); + onClose(); break; case ACTION_PLAY_PAUSE: onVideoPlayPause(); break; case ACTION_OPEN_DETAIL: - openControl(PopupVideoPlayer.this); + openControl(getApplicationContext()); break; case ACTION_REPEAT: onRepeatClicked(); @@ -800,7 +809,7 @@ public final class PopupVideoPlayer extends Service { public boolean onFling(MotionEvent e1, MotionEvent e2, float velocityX, float velocityY) { if (Math.abs(velocityX) > SHUTDOWN_FLING_VELOCITY) { if (DEBUG) Log.d(TAG, "Popup close fling velocity= " + velocityX); - onVideoClose(); + onClose(); return true; } return false; diff --git a/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayerActivity.java b/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayerActivity.java index 80375c6fb..2230c9c52 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayerActivity.java +++ b/app/src/main/java/org/schabi/newpipe/player/PopupVideoPlayerActivity.java @@ -1,7 +1,6 @@ package org.schabi.newpipe.player; import android.content.Intent; -import android.os.IBinder; import org.schabi.newpipe.R; @@ -19,12 +18,6 @@ public final class PopupVideoPlayerActivity extends ServicePlayerActivity { return getResources().getString(R.string.title_activity_popup_player); } - @Override - public BasePlayer playerFrom(IBinder binder) { - final PopupVideoPlayer.LocalBinder mLocalBinder = (PopupVideoPlayer.LocalBinder) binder; - return mLocalBinder.getPopupPlayerInstance(); - } - @Override public Intent getBindIntent() { return new Intent(this, PopupVideoPlayer.class); diff --git a/app/src/main/java/org/schabi/newpipe/player/ServicePlayerActivity.java b/app/src/main/java/org/schabi/newpipe/player/ServicePlayerActivity.java index 2da095b3a..012f47cd4 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ServicePlayerActivity.java +++ b/app/src/main/java/org/schabi/newpipe/player/ServicePlayerActivity.java @@ -91,8 +91,6 @@ public abstract class ServicePlayerActivity extends AppCompatActivity public abstract void stopPlayerListener(); - public abstract BasePlayer playerFrom(final IBinder binder); - //////////////////////////////////////////////////////////////////////////// // Activity Lifecycle //////////////////////////////////////////////////////////////////////////// @@ -182,7 +180,11 @@ public abstract class ServicePlayerActivity extends AppCompatActivity @Override public void onServiceConnected(ComponentName name, IBinder service) { Log.d(getTag(), "Player service is connected"); - player = playerFrom(service); + + if (service instanceof PlayerServiceBinder) { + player = ((PlayerServiceBinder) service).getPlayerInstance(); + } + if (player == null || player.playQueue == null || player.playQueueAdapter == null || player.simpleExoPlayer == null) { unbind(); finish(); diff --git a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java index 979f7e96c..9a92dc27d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java +++ b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java @@ -139,7 +139,7 @@ public class MediaSourceManager implements DeferredMediaSource.Callback { @Override public void onNext(@NonNull PlayQueueEvent playQueueMessage) { - onPlayQueueChanged(playQueueMessage); + if (playQueueReactor != null) onPlayQueueChanged(playQueueMessage); } @Override @@ -153,6 +153,7 @@ public class MediaSourceManager implements DeferredMediaSource.Callback { private void onPlayQueueChanged(final PlayQueueEvent event) { if (playQueue.isEmpty()) { playbackListener.shutdown(); + return; } // why no pattern matching in Java =( @@ -170,7 +171,7 @@ public class MediaSourceManager implements DeferredMediaSource.Callback { break; case REMOVE: final RemoveEvent removeEvent = (RemoveEvent) event; - remove(removeEvent.index()); + remove(removeEvent.getRemoveIndex()); sync(); break; case MOVE: 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 09481cb71..4d73e1cfd 100644 --- a/app/src/main/java/org/schabi/newpipe/playlist/PlayQueue.java +++ b/app/src/main/java/org/schabi/newpipe/playlist/PlayQueue.java @@ -18,7 +18,6 @@ import org.schabi.newpipe.playlist.events.SelectEvent; import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -210,25 +209,30 @@ public abstract class PlayQueue implements Serializable { /** * Appends the given {@link PlayQueueItem}s to the current play queue. * - * Will emit a {@link AppendEvent} on any given context. + * @see #append(List items) * */ public synchronized void append(final PlayQueueItem... items) { - streams.addAll(Arrays.asList(items)); - if (backup != null) backup.addAll(Arrays.asList(items)); - - broadcast(new AppendEvent(items.length)); + append(Arrays.asList(items)); } /** * Appends the given {@link PlayQueueItem}s to the current play queue. * + * If the play queue is shuffled, then append the items to the backup queue as is and + * append the shuffle items to the play queue. + * * Will emit a {@link AppendEvent} on any given context. * */ - public synchronized void append(final Collection items) { - streams.addAll(items); - if (backup != null) backup.addAll(items); + public synchronized void append(final List items) { + List itemList = new ArrayList<>(items); - broadcast(new AppendEvent(items.size())); + if (isShuffled()) { + backup.addAll(itemList); + Collections.shuffle(itemList); + } + streams.addAll(itemList); + + broadcast(new AppendEvent(itemList.size())); } /** @@ -242,7 +246,7 @@ public abstract class PlayQueue implements Serializable { public synchronized void remove(final int index) { if (index >= streams.size() || index < 0) return; removeInternal(index); - broadcast(new RemoveEvent(index)); + broadcast(new RemoveEvent(index, getIndex())); } /** @@ -261,24 +265,28 @@ public abstract class PlayQueue implements Serializable { removeInternal(index); } - broadcast(new ErrorEvent(index, skippable)); + broadcast(new ErrorEvent(index, getIndex(), skippable)); } - private synchronized void removeInternal(final int index) { + private synchronized void removeInternal(final int removeIndex) { final int currentIndex = queueIndex.get(); final int size = size(); - if (currentIndex > index) { + if (currentIndex > removeIndex) { queueIndex.decrementAndGet(); + } else if (currentIndex >= size) { queueIndex.set(currentIndex % (size - 1)); + + } else if (currentIndex == removeIndex && currentIndex == size - 1){ + queueIndex.set(removeIndex - 1); } if (backup != null) { - final int backupIndex = backup.indexOf(getItem(index)); + final int backupIndex = backup.indexOf(getItem(removeIndex)); backup.remove(backupIndex); } - streams.remove(index); + streams.remove(removeIndex); } /** diff --git a/app/src/main/java/org/schabi/newpipe/playlist/PlayQueueAdapter.java b/app/src/main/java/org/schabi/newpipe/playlist/PlayQueueAdapter.java index 9a856cbb9..72711e4b3 100644 --- a/app/src/main/java/org/schabi/newpipe/playlist/PlayQueueAdapter.java +++ b/app/src/main/java/org/schabi/newpipe/playlist/PlayQueueAdapter.java @@ -82,7 +82,7 @@ public class PlayQueueAdapter extends RecyclerView.Adapter