From ebe32e795a46539ed56d952bef4a5a8dec4ba858 Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Wed, 3 Apr 2019 22:03:47 +0200 Subject: [PATCH 1/4] Executing all ExoPlayer methods on main thread --- .../service/playback/ExoPlayerWrapper.java | 1 + .../core/service/playback/LocalPSMP.java | 48 ++++++++++++++++--- .../playback/PlaybackServiceTaskManager.java | 4 +- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/ExoPlayerWrapper.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/ExoPlayerWrapper.java index 281bd064b..f20525f73 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/ExoPlayerWrapper.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/ExoPlayerWrapper.java @@ -165,6 +165,7 @@ public class ExoPlayerWrapper implements IPlayer { @Override public void seekTo(int i) throws IllegalStateException { mExoPlayer.seekTo(i); + audioSeekCompleteListener.onSeekComplete(null); } @Override diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java index 9274b9a49..d258c993e 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java @@ -18,6 +18,7 @@ import java.io.File; import java.io.IOException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Future; +import java.util.concurrent.FutureTask; import java.util.concurrent.LinkedBlockingDeque; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -57,16 +58,42 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { private final ReentrantLock playerLock; private CountDownLatch seekLatch; - private final ThreadPoolExecutor executor; + private final PlayerExecutor executor; + + /** + * All ExoPlayer methods must be executed on the same thread. + * We use the main application thread. This class allows to + * "fake" an executor that just calls the method instead of + * submitting to another thread. Other players are still + * executed in a background thread. + */ + private class PlayerExecutor { + private boolean useMainThread = true; + private ThreadPoolExecutor threadPool; + + public Future submit(Runnable r) { + if (useMainThread) { + r.run(); + return new FutureTask(() -> {}, null); + } else { + return threadPool.submit(r); + } + } + + public void shutdown() { + threadPool.shutdown(); + } + } public LocalPSMP(@NonNull Context context, @NonNull PSMPCallback callback) { super(context, callback); - this.audioManager = (AudioManager) context.getSystemService(Context.AUDIO_SERVICE); this.playerLock = new ReentrantLock(); this.startWhenPrepared = new AtomicBoolean(false); - executor = new ThreadPoolExecutor(1, 1, 5, TimeUnit.MINUTES, new LinkedBlockingDeque<>(), + + executor = new PlayerExecutor(); + executor.threadPool = new ThreadPoolExecutor(1, 1, 5, TimeUnit.MINUTES, new LinkedBlockingDeque<>(), (r, executor) -> Log.d(TAG, "Rejected execution of runnable")); mediaPlayer = null; @@ -105,6 +132,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { @Override public void playMediaObject(@NonNull final Playable playable, final boolean stream, final boolean startWhenPrepared, final boolean prepareImmediately) { Log.d(TAG, "playMediaObject(...)"); + executor.useMainThread = true; // ExoPlayer needs to be initialized in main thread executor.submit(() -> { playerLock.lock(); try { @@ -755,10 +783,13 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { if (UserPreferences.useExoplayer()) { mediaPlayer = new ExoPlayerWrapper(context); + executor.useMainThread = true; } else if (media.getMediaType() == MediaType.VIDEO) { mediaPlayer = new VideoPlayer(); + executor.useMainThread = false; } else { mediaPlayer = new AudioPlayer(context); + executor.useMainThread = false; } mediaPlayer.setAudioStreamType(AudioManager.STREAM_MUSIC); @@ -1012,7 +1043,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { mp -> genericSeekCompleteListener(); private void genericSeekCompleteListener() { - Thread t = new Thread(() -> { + Runnable r = () -> { Log.d(TAG, "genericSeekCompleteListener"); if(seekLatch != null) { seekLatch.countDown(); @@ -1025,7 +1056,12 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { setPlayerStatus(statusBeforeSeeking, media, getPosition()); } playerLock.unlock(); - }); - t.start(); + }; + + if (mediaPlayer instanceof ExoPlayerWrapper) { + r.run(); + } else { + new Thread(r).start(); + } } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceTaskManager.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceTaskManager.java index 3d97e862a..1c20ec44d 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceTaskManager.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceTaskManager.java @@ -1,6 +1,7 @@ package de.danoeh.antennapod.core.service.playback; import android.content.Context; +import android.os.Handler; import android.os.Vibrator; import android.support.annotation.NonNull; import android.util.Log; @@ -125,7 +126,8 @@ public class PlaybackServiceTaskManager { */ public synchronized void startPositionSaver() { if (!isPositionSaverActive()) { - Runnable positionSaver = callback::positionSaverTick; + Handler handler = new Handler(); // Execute on main thread + Runnable positionSaver = () -> handler.post(callback::positionSaverTick); positionSaverFuture = schedExecutor.scheduleWithFixedDelay(positionSaver, POSITION_SAVER_WAITING_INTERVAL, POSITION_SAVER_WAITING_INTERVAL, TimeUnit.MILLISECONDS); From 156a20734a955d059516f791e54c78f29de24979 Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Sun, 7 Apr 2019 12:54:12 +0200 Subject: [PATCH 2/4] Fix Sonic playback --- .../core/service/playback/LocalPSMP.java | 9 ++++---- .../playback/PlaybackServiceTaskManager.java | 21 ++++++++++++++++--- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java index d258c993e..daee303ba 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java @@ -132,7 +132,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { @Override public void playMediaObject(@NonNull final Playable playable, final boolean stream, final boolean startWhenPrepared, final boolean prepareImmediately) { Log.d(TAG, "playMediaObject(...)"); - executor.useMainThread = true; // ExoPlayer needs to be initialized in main thread + executor.useMainThread = UserPreferences.useExoplayer(); executor.submit(() -> { playerLock.lock(); try { @@ -400,6 +400,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { */ @Override public void reinit() { + executor.useMainThread = UserPreferences.useExoplayer(); executor.submit(() -> { playerLock.lock(); Log.d(TAG, "reinit()"); @@ -783,13 +784,10 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { if (UserPreferences.useExoplayer()) { mediaPlayer = new ExoPlayerWrapper(context); - executor.useMainThread = true; } else if (media.getMediaType() == MediaType.VIDEO) { mediaPlayer = new VideoPlayer(); - executor.useMainThread = false; } else { mediaPlayer = new AudioPlayer(context); - executor.useMainThread = false; } mediaPlayer.setAudioStreamType(AudioManager.STREAM_MUSIC); @@ -852,6 +850,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { @Override protected Future endPlayback(final boolean hasEnded, final boolean wasSkipped, final boolean shouldContinue, final boolean toStoppedState) { + executor.useMainThread = UserPreferences.useExoplayer(); return executor.submit(() -> { playerLock.lock(); releaseWifiLockIfNecessary(); @@ -1058,7 +1057,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { playerLock.unlock(); }; - if (mediaPlayer instanceof ExoPlayerWrapper) { + if (executor.useMainThread) { r.run(); } else { new Thread(r).start(); diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceTaskManager.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceTaskManager.java index 1c20ec44d..bfc75a902 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceTaskManager.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceTaskManager.java @@ -2,6 +2,7 @@ package de.danoeh.antennapod.core.service.playback; import android.content.Context; import android.os.Handler; +import android.os.Looper; import android.os.Vibrator; import android.support.annotation.NonNull; import android.util.Log; @@ -126,8 +127,8 @@ public class PlaybackServiceTaskManager { */ public synchronized void startPositionSaver() { if (!isPositionSaverActive()) { - Handler handler = new Handler(); // Execute on main thread - Runnable positionSaver = () -> handler.post(callback::positionSaverTick); + Runnable positionSaver = callback::positionSaverTick; + positionSaver = useMainThreadIfNecessary(positionSaver); positionSaverFuture = schedExecutor.scheduleWithFixedDelay(positionSaver, POSITION_SAVER_WAITING_INTERVAL, POSITION_SAVER_WAITING_INTERVAL, TimeUnit.MILLISECONDS); @@ -160,6 +161,7 @@ public class PlaybackServiceTaskManager { public synchronized void startWidgetUpdater() { if (!isWidgetUpdaterActive()) { Runnable widgetUpdater = callback::onWidgetUpdaterTick; + widgetUpdater = useMainThreadIfNecessary(widgetUpdater); widgetUpdaterFuture = schedExecutor.scheduleWithFixedDelay(widgetUpdater, WIDGET_UPDATER_NOTIFICATION_INTERVAL, WIDGET_UPDATER_NOTIFICATION_INTERVAL, TimeUnit.MILLISECONDS); @@ -186,7 +188,8 @@ public class PlaybackServiceTaskManager { sleepTimerFuture.cancel(true); } sleepTimer = new SleepTimer(waitingTime, shakeToReset, vibrate); - sleepTimerFuture = schedExecutor.schedule(sleepTimer, 0, TimeUnit.MILLISECONDS); + Runnable runnable = useMainThreadIfNecessary(sleepTimer); + sleepTimerFuture = schedExecutor.schedule(runnable, 0, TimeUnit.MILLISECONDS); } /** @@ -269,6 +272,7 @@ public class PlaybackServiceTaskManager { } Log.d(TAG, "Chapter loader stopped"); }; + chapterLoader = useMainThreadIfNecessary(chapterLoader); chapterLoaderFuture = schedExecutor.submit(chapterLoader); } @@ -294,6 +298,17 @@ public class PlaybackServiceTaskManager { schedExecutor.shutdown(); } + private Runnable useMainThreadIfNecessary(Runnable runnable) { + if (Looper.myLooper() == Looper.getMainLooper()) { + // Called in main thread => ExoPlayer is used + // Run on ui thread even if called from schedExecutor + Handler handler = new Handler(); + return () -> handler.post(runnable); + } else { + return runnable; + } + } + /** * Sleeps for a given time and then pauses playback. */ From 5745da75a68b41d7089698bd1ad5544c07e9d2d9 Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Thu, 11 Apr 2019 20:11:40 +0200 Subject: [PATCH 3/4] Clarified that it is using caller thread. not main thread --- .../core/service/playback/LocalPSMP.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java index daee303ba..649082f6e 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/LocalPSMP.java @@ -63,16 +63,16 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { /** * All ExoPlayer methods must be executed on the same thread. * We use the main application thread. This class allows to - * "fake" an executor that just calls the method instead of - * submitting to another thread. Other players are still - * executed in a background thread. + * "fake" an executor that just calls the methods on the + * calling thread instead of submitting to an executor. + * Other players are still executed in a background thread. */ private class PlayerExecutor { - private boolean useMainThread = true; + private boolean useCallerThread = true; private ThreadPoolExecutor threadPool; public Future submit(Runnable r) { - if (useMainThread) { + if (useCallerThread) { r.run(); return new FutureTask(() -> {}, null); } else { @@ -132,7 +132,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { @Override public void playMediaObject(@NonNull final Playable playable, final boolean stream, final boolean startWhenPrepared, final boolean prepareImmediately) { Log.d(TAG, "playMediaObject(...)"); - executor.useMainThread = UserPreferences.useExoplayer(); + executor.useCallerThread = UserPreferences.useExoplayer(); executor.submit(() -> { playerLock.lock(); try { @@ -400,7 +400,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { */ @Override public void reinit() { - executor.useMainThread = UserPreferences.useExoplayer(); + executor.useCallerThread = UserPreferences.useExoplayer(); executor.submit(() -> { playerLock.lock(); Log.d(TAG, "reinit()"); @@ -850,7 +850,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { @Override protected Future endPlayback(final boolean hasEnded, final boolean wasSkipped, final boolean shouldContinue, final boolean toStoppedState) { - executor.useMainThread = UserPreferences.useExoplayer(); + executor.useCallerThread = UserPreferences.useExoplayer(); return executor.submit(() -> { playerLock.lock(); releaseWifiLockIfNecessary(); @@ -1057,7 +1057,7 @@ public class LocalPSMP extends PlaybackServiceMediaPlayer { playerLock.unlock(); }; - if (executor.useMainThread) { + if (executor.useCallerThread) { r.run(); } else { new Thread(r).start(); From 1d0e22135e57c7bcff8988c388d7f0f813ad7061 Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Thu, 11 Apr 2019 20:41:39 +0200 Subject: [PATCH 4/4] Making sure that ExternalPlayerFragment is updated when starting first media --- .../antennapod/activity/MainActivity.java | 9 ----- .../fragment/ExternalPlayerFragment.java | 38 +++++++++++++------ 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/de/danoeh/antennapod/activity/MainActivity.java b/app/src/main/java/de/danoeh/antennapod/activity/MainActivity.java index 9f4fbe271..3bbe206f8 100644 --- a/app/src/main/java/de/danoeh/antennapod/activity/MainActivity.java +++ b/app/src/main/java/de/danoeh/antennapod/activity/MainActivity.java @@ -776,15 +776,6 @@ public class MainActivity extends CastEnabledActivity implements NavDrawerActivi loadData(); } - public void onEventMainThread(ServiceEvent event) { - Log.d(TAG, "onEvent(" + event + ")"); - switch(event.action) { - case SERVICE_STARTED: - externalPlayerFragment.connectToPlaybackService(); - break; - } - } - public void onEventMainThread(ProgressEvent event) { Log.d(TAG, "onEvent(" + event + ")"); switch(event.action) { diff --git a/app/src/main/java/de/danoeh/antennapod/fragment/ExternalPlayerFragment.java b/app/src/main/java/de/danoeh/antennapod/fragment/ExternalPlayerFragment.java index de2f04590..2c35bdba4 100644 --- a/app/src/main/java/de/danoeh/antennapod/fragment/ExternalPlayerFragment.java +++ b/app/src/main/java/de/danoeh/antennapod/fragment/ExternalPlayerFragment.java @@ -18,11 +18,13 @@ import com.bumptech.glide.Glide; import com.bumptech.glide.request.RequestOptions; import de.danoeh.antennapod.R; +import de.danoeh.antennapod.core.event.ServiceEvent; import de.danoeh.antennapod.core.feed.MediaType; import de.danoeh.antennapod.core.glide.ApGlideSettings; import de.danoeh.antennapod.core.service.playback.PlaybackService; import de.danoeh.antennapod.core.util.playback.Playable; import de.danoeh.antennapod.core.util.playback.PlaybackController; +import de.greenrobot.event.EventBus; import io.reactivex.Maybe; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.Disposable; @@ -90,8 +92,11 @@ public class ExternalPlayerFragment extends Fragment { loadMediaInfo(); } - public void connectToPlaybackService() { - controller.init(); + public void onEventMainThread(ServiceEvent event) { + Log.d(TAG, "onEvent(" + event + ")"); + if (event.action == ServiceEvent.Action.SERVICE_STARTED) { + controller.init(); + } } private PlaybackController setupPlaybackController() { @@ -109,12 +114,12 @@ public class ExternalPlayerFragment extends Fragment { @Override public boolean loadMediaInfo() { - ExternalPlayerFragment fragment = ExternalPlayerFragment.this; - if (fragment != null) { - return fragment.loadMediaInfo(); - } else { - return false; - } + return ExternalPlayerFragment.this.loadMediaInfo(); + } + + @Override + public void setupGUI() { + ExternalPlayerFragment.this.loadMediaInfo(); } @Override @@ -133,17 +138,28 @@ public class ExternalPlayerFragment extends Fragment { public void onResume() { super.onResume(); onPositionObserverUpdate(); + } + @Override + public void onStart() { + super.onStart(); controller.init(); + EventBus.getDefault().register(this); + } + + @Override + public void onStop() { + super.onStop(); + if (controller != null) { + controller.release(); + } + EventBus.getDefault().unregister(this); } @Override public void onDestroy() { super.onDestroy(); Log.d(TAG, "Fragment is about to be destroyed"); - if (controller != null) { - controller.release(); - } if (disposable != null) { disposable.dispose(); }