From e947e86eae1af11027d8f57e64fa3b9433dcc8e5 Mon Sep 17 00:00:00 2001 From: Trust_04zh Date: Tue, 12 Apr 2022 15:45:35 +0800 Subject: [PATCH] Make positions in list depend on watch history, remove confusing animations The following is the list of all commits squashed together: Regain function for option `Positions in lists` use option `Resume playback` to control display of progress info in VideoDetailFragment, remove this (extra) function from option `Positions in lists`. remove extra check for live streams, live streams updates just as non-live streams. fix #8176 by eliminating exit delay Regain function for option `Positions in lists` update code with developer's comments apply static import to methods in util class DependentPreferenceHelper Regain function for option `Positions in lists` use option `Resume playback` to control display of progress info in VideoDetailFragment, remove this (extra) function from option `Positions in lists`. remove extra check for live streams, live streams updates just as non-live streams. fix behavior for displaying progress bar when autoplay off but video resume on not to retrieve unnecessary states when position in lists disabled fix mistake in code simplify conditional logic update doc comment and remove unused method Fix not showing duration if position indicators disabled Positions in lists only depends on watch history --- .../fragments/detail/VideoDetailFragment.java | 66 ++++++------------- .../holder/StreamMiniInfoItemHolder.java | 18 +++-- .../holder/LocalPlaylistStreamItemHolder.java | 7 +- .../LocalStatisticStreamItemHolder.java | 7 +- .../org/schabi/newpipe/player/Player.java | 4 +- .../newpipe/player/helper/PlayerHelper.java | 7 -- .../util/DependentPreferenceHelper.java | 51 ++++++++++++++ app/src/main/res/xml/history_settings.xml | 1 + 8 files changed, 98 insertions(+), 63 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/util/DependentPreferenceHelper.java diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 917d89e09de..d6de6dfca99 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -7,7 +7,7 @@ import static org.schabi.newpipe.ktx.ViewUtils.animateRotation; import static org.schabi.newpipe.player.helper.PlayerHelper.globalScreenOrientationLocked; import static org.schabi.newpipe.player.helper.PlayerHelper.isClearingQueueConfirmationRequired; -import static org.schabi.newpipe.player.playqueue.PlayQueueItem.RECOVERY_UNSET; +import static org.schabi.newpipe.util.DependentPreferenceHelper.getResumePlaybackEnabled; import static org.schabi.newpipe.util.ExtractorHelper.showMetaInfoInTextView; import static org.schabi.newpipe.util.ListHelper.getUrlAndNonTorrentStreams; import static org.schabi.newpipe.util.NavigationHelper.openPlayQueue; @@ -1456,8 +1456,8 @@ public void showLoading() { animate(binding.detailThumbnailPlayButton, false, 50); animate(binding.detailDurationView, false, 100); - animate(binding.detailPositionView, false, 100); - animate(binding.positionView, false, 50); + binding.detailPositionView.setVisibility(View.GONE); + binding.positionView.setVisibility(View.GONE); binding.detailVideoTitleView.setText(title); binding.detailVideoTitleView.setMaxLines(1); @@ -1574,7 +1574,7 @@ public void handleResult(@NonNull final StreamInfo info) { binding.detailToggleSecondaryControlsView.setVisibility(View.VISIBLE); binding.detailSecondaryControlPanel.setVisibility(View.GONE); - updateProgressInfo(info); + checkUpdateProgressInfo(info); initThumbnailViews(info); showMetaInfoInTextView(info.getMetaInfo(), binding.detailMetaInfoTextView, binding.detailMetaInfoSeparator, disposables); @@ -1673,67 +1673,43 @@ public void openDownloadDialog() { // Stream Results //////////////////////////////////////////////////////////////////////////*/ - private void updateProgressInfo(@NonNull final StreamInfo info) { + private void checkUpdateProgressInfo(@NonNull final StreamInfo info) { if (positionSubscriber != null) { positionSubscriber.dispose(); } - final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(activity); - final boolean playbackResumeEnabled = prefs - .getBoolean(activity.getString(R.string.enable_watch_history_key), true) - && prefs.getBoolean(activity.getString(R.string.enable_playback_resume_key), true); - final boolean showPlaybackPosition = prefs.getBoolean( - activity.getString(R.string.enable_playback_state_lists_key), true); - if (!playbackResumeEnabled) { - if (playQueue == null || playQueue.getStreams().isEmpty() - || playQueue.getItem().getRecoveryPosition() == RECOVERY_UNSET - || !showPlaybackPosition) { - binding.positionView.setVisibility(View.INVISIBLE); - binding.detailPositionView.setVisibility(View.GONE); - // TODO: Remove this check when separation of concerns is done. - // (live streams weren't getting updated because they are mixed) - if (!StreamTypeUtil.isLiveStream(info.getStreamType())) { - return; - } - } else { - // Show saved position from backStack if user allows it - showPlaybackProgress(playQueue.getItem().getRecoveryPosition(), - playQueue.getItem().getDuration() * 1000); - animate(binding.positionView, true, 500); - animate(binding.detailPositionView, true, 500); - } + if (!getResumePlaybackEnabled(activity)) { + binding.positionView.setVisibility(View.GONE); + binding.detailPositionView.setVisibility(View.GONE); return; } final HistoryRecordManager recordManager = new HistoryRecordManager(requireContext()); - - // TODO: Separate concerns when updating database data. - // (move the updating part to when the loading happens) positionSubscriber = recordManager.loadStreamState(info) .subscribeOn(Schedulers.io()) .onErrorComplete() .observeOn(AndroidSchedulers.mainThread()) .subscribe(state -> { - showPlaybackProgress(state.getProgressMillis(), info.getDuration() * 1000); - animate(binding.positionView, true, 500); - animate(binding.detailPositionView, true, 500); + updatePlaybackProgress( + state.getProgressMillis(), info.getDuration() * 1000); }, e -> { - if (DEBUG) { - e.printStackTrace(); - } + // impossible since the onErrorComplete() }, () -> { binding.positionView.setVisibility(View.GONE); binding.detailPositionView.setVisibility(View.GONE); }); } - private void showPlaybackProgress(final long progress, final long duration) { + private void updatePlaybackProgress(final long progress, final long duration) { + if (!getResumePlaybackEnabled(activity)) { + return; + } final int progressSeconds = (int) TimeUnit.MILLISECONDS.toSeconds(progress); final int durationSeconds = (int) TimeUnit.MILLISECONDS.toSeconds(duration); - // If the old and the new progress values have a big difference then use - // animation. Otherwise don't because it affects CPU - final boolean shouldAnimate = Math.abs(binding.positionView.getProgress() - - progressSeconds) > 2; + // If the old and the new progress values have a big difference then use animation. + // Otherwise don't because it affects CPU + final int progressDifference = Math.abs(binding.positionView.getProgress() + - progressSeconds); binding.positionView.setMax(durationSeconds); - if (shouldAnimate) { + if (progressDifference > 2) { binding.positionView.setProgressAnimated(progressSeconds); } else { binding.positionView.setProgress(progressSeconds); @@ -1828,7 +1804,7 @@ public void onProgressUpdate(final int currentProgress, } if (player.getPlayQueue().getItem().getUrl().equals(url)) { - showPlaybackProgress(currentProgress, duration); + updatePlaybackProgress(currentProgress, duration); } } diff --git a/app/src/main/java/org/schabi/newpipe/info_list/holder/StreamMiniInfoItemHolder.java b/app/src/main/java/org/schabi/newpipe/info_list/holder/StreamMiniInfoItemHolder.java index 8d17017d217..6dd06e47f67 100644 --- a/app/src/main/java/org/schabi/newpipe/info_list/holder/StreamMiniInfoItemHolder.java +++ b/app/src/main/java/org/schabi/newpipe/info_list/holder/StreamMiniInfoItemHolder.java @@ -14,6 +14,7 @@ import org.schabi.newpipe.info_list.InfoItemBuilder; import org.schabi.newpipe.ktx.ViewUtils; import org.schabi.newpipe.local.history.HistoryRecordManager; +import org.schabi.newpipe.util.DependentPreferenceHelper; import org.schabi.newpipe.util.Localization; import org.schabi.newpipe.util.PicassoHelper; import org.schabi.newpipe.util.StreamTypeUtil; @@ -60,8 +61,12 @@ public void updateFromItem(final InfoItem infoItem, R.color.duration_background_color)); itemDurationView.setVisibility(View.VISIBLE); - final StreamStateEntity state2 = historyRecordManager.loadStreamState(infoItem) - .blockingGet()[0]; + StreamStateEntity state2 = null; + if (DependentPreferenceHelper + .getPositionsInListsEnabled(itemProgressView.getContext())) { + state2 = historyRecordManager.loadStreamState(infoItem) + .blockingGet()[0]; + } if (state2 != null) { itemProgressView.setVisibility(View.VISIBLE); itemProgressView.setMax((int) item.getDuration()); @@ -111,9 +116,12 @@ public void updateState(final InfoItem infoItem, final HistoryRecordManager historyRecordManager) { final StreamInfoItem item = (StreamInfoItem) infoItem; - final StreamStateEntity state = historyRecordManager - .loadStreamState(infoItem) - .blockingGet()[0]; + StreamStateEntity state = null; + if (DependentPreferenceHelper.getPositionsInListsEnabled(itemProgressView.getContext())) { + state = historyRecordManager + .loadStreamState(infoItem) + .blockingGet()[0]; + } if (state != null && item.getDuration() > 0 && !StreamTypeUtil.isLiveStream(item.getStreamType())) { itemProgressView.setMax((int) item.getDuration()); diff --git a/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistStreamItemHolder.java b/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistStreamItemHolder.java index d3975832670..c98a8b60be4 100644 --- a/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistStreamItemHolder.java +++ b/app/src/main/java/org/schabi/newpipe/local/holder/LocalPlaylistStreamItemHolder.java @@ -14,6 +14,7 @@ import org.schabi.newpipe.ktx.ViewUtils; import org.schabi.newpipe.local.LocalItemBuilder; import org.schabi.newpipe.local.history.HistoryRecordManager; +import org.schabi.newpipe.util.DependentPreferenceHelper; import org.schabi.newpipe.util.Localization; import org.schabi.newpipe.util.PicassoHelper; import org.schabi.newpipe.util.ServiceHelper; @@ -68,7 +69,8 @@ public void updateFromItem(final LocalItem localItem, R.color.duration_background_color)); itemDurationView.setVisibility(View.VISIBLE); - if (item.getProgressMillis() > 0) { + if (DependentPreferenceHelper.getPositionsInListsEnabled(itemProgressView.getContext()) + && item.getProgressMillis() > 0) { itemProgressView.setVisibility(View.VISIBLE); itemProgressView.setMax((int) item.getStreamEntity().getDuration()); itemProgressView.setProgress((int) TimeUnit.MILLISECONDS @@ -109,7 +111,8 @@ public void updateState(final LocalItem localItem, } final PlaylistStreamEntry item = (PlaylistStreamEntry) localItem; - if (item.getProgressMillis() > 0 && item.getStreamEntity().getDuration() > 0) { + if (DependentPreferenceHelper.getPositionsInListsEnabled(itemProgressView.getContext()) + && item.getProgressMillis() > 0 && item.getStreamEntity().getDuration() > 0) { itemProgressView.setMax((int) item.getStreamEntity().getDuration()); if (itemProgressView.getVisibility() == View.VISIBLE) { itemProgressView.setProgressAnimated((int) TimeUnit.MILLISECONDS diff --git a/app/src/main/java/org/schabi/newpipe/local/holder/LocalStatisticStreamItemHolder.java b/app/src/main/java/org/schabi/newpipe/local/holder/LocalStatisticStreamItemHolder.java index 0d88eecba70..41f2df1d0e5 100644 --- a/app/src/main/java/org/schabi/newpipe/local/holder/LocalStatisticStreamItemHolder.java +++ b/app/src/main/java/org/schabi/newpipe/local/holder/LocalStatisticStreamItemHolder.java @@ -14,6 +14,7 @@ import org.schabi.newpipe.ktx.ViewUtils; import org.schabi.newpipe.local.LocalItemBuilder; import org.schabi.newpipe.local.history.HistoryRecordManager; +import org.schabi.newpipe.util.DependentPreferenceHelper; import org.schabi.newpipe.util.Localization; import org.schabi.newpipe.util.PicassoHelper; import org.schabi.newpipe.util.ServiceHelper; @@ -97,7 +98,8 @@ public void updateFromItem(final LocalItem localItem, R.color.duration_background_color)); itemDurationView.setVisibility(View.VISIBLE); - if (item.getProgressMillis() > 0) { + if (DependentPreferenceHelper.getPositionsInListsEnabled(itemProgressView.getContext()) + && item.getProgressMillis() > 0) { itemProgressView.setVisibility(View.VISIBLE); itemProgressView.setMax((int) item.getStreamEntity().getDuration()); itemProgressView.setProgress((int) TimeUnit.MILLISECONDS @@ -141,7 +143,8 @@ public void updateState(final LocalItem localItem, } final StreamStatisticsEntry item = (StreamStatisticsEntry) localItem; - if (item.getProgressMillis() > 0 && item.getStreamEntity().getDuration() > 0) { + if (DependentPreferenceHelper.getPositionsInListsEnabled(itemProgressView.getContext()) + && item.getProgressMillis() > 0 && item.getStreamEntity().getDuration() > 0) { itemProgressView.setMax((int) item.getStreamEntity().getDuration()); if (itemProgressView.getVisibility() == View.VISIBLE) { itemProgressView.setProgressAnimated((int) TimeUnit.MILLISECONDS diff --git a/app/src/main/java/org/schabi/newpipe/player/Player.java b/app/src/main/java/org/schabi/newpipe/player/Player.java index b6098a1ef7d..dcbebaedfb0 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -29,7 +29,6 @@ import static com.google.android.exoplayer2.Player.RepeatMode; import static org.schabi.newpipe.extractor.ServiceList.YouTube; import static org.schabi.newpipe.extractor.utils.Utils.isNullOrEmpty; -import static org.schabi.newpipe.player.helper.PlayerHelper.isPlaybackResumeEnabled; import static org.schabi.newpipe.player.helper.PlayerHelper.nextRepeatMode; import static org.schabi.newpipe.player.helper.PlayerHelper.retrievePlaybackParametersFromPrefs; import static org.schabi.newpipe.player.helper.PlayerHelper.retrieveSeekDurationFromPreferences; @@ -115,6 +114,7 @@ import org.schabi.newpipe.player.ui.PlayerUiList; import org.schabi.newpipe.player.ui.PopupPlayerUi; import org.schabi.newpipe.player.ui.VideoPlayerUi; +import org.schabi.newpipe.util.DependentPreferenceHelper; import org.schabi.newpipe.util.DeviceUtils; import org.schabi.newpipe.util.ListHelper; import org.schabi.newpipe.util.NavigationHelper; @@ -391,7 +391,7 @@ public void handleIntent(@NonNull final Intent intent) { simpleExoPlayer.setPlayWhenReady(playWhenReady); } else if (intent.getBooleanExtra(RESUME_PLAYBACK, false) - && isPlaybackResumeEnabled(this) + && DependentPreferenceHelper.getResumePlaybackEnabled(context) && !samePlayQueue && !newQueue.isEmpty() && newQueue.getItem() != null diff --git a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHelper.java b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHelper.java index abde7c3d12c..67089e425d5 100644 --- a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHelper.java +++ b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHelper.java @@ -425,13 +425,6 @@ private static SinglePlayQueue getAutoQueuedSinglePlayQueue( // Utils used by player //////////////////////////////////////////////////////////////////////////// - public static boolean isPlaybackResumeEnabled(final Player player) { - return player.getPrefs().getBoolean( - player.getContext().getString(R.string.enable_watch_history_key), true) - && player.getPrefs().getBoolean( - player.getContext().getString(R.string.enable_playback_resume_key), true); - } - @RepeatMode public static int nextRepeatMode(@RepeatMode final int repeatMode) { switch (repeatMode) { diff --git a/app/src/main/java/org/schabi/newpipe/util/DependentPreferenceHelper.java b/app/src/main/java/org/schabi/newpipe/util/DependentPreferenceHelper.java new file mode 100644 index 00000000000..9591beddb3c --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/util/DependentPreferenceHelper.java @@ -0,0 +1,51 @@ +package org.schabi.newpipe.util; + +import android.content.Context; +import android.content.SharedPreferences; + +import androidx.preference.PreferenceManager; + +import org.schabi.newpipe.R; + +/** + * For preferences with dependencies and multiple use case, + * this class can be used to reduce the lines of code. + */ +public final class DependentPreferenceHelper { + + private DependentPreferenceHelper() { + // no instance + } + + /** + * Option `Resume playback` depends on `Watch history`, this method can be used to retrieve if + * `Resume playback` and its dependencies are all enabled. + * + * @param context the Android context + * @return returns true if `Resume playback` and `Watch history` are both enabled + */ + public static boolean getResumePlaybackEnabled(final Context context) { + final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); + + return prefs.getBoolean(context.getString( + R.string.enable_watch_history_key), true) + && prefs.getBoolean(context.getString( + R.string.enable_playback_resume_key), true); + } + + /** + * Option `Position in lists` depends on `Watch history`, this method can be used to retrieve if + * `Position in lists` and its dependencies are all enabled. + * + * @param context the Android context + * @return returns true if `Positions in lists` and `Watch history` are both enabled + */ + public static boolean getPositionsInListsEnabled(final Context context) { + final SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); + + return prefs.getBoolean(context.getString( + R.string.enable_watch_history_key), true) + && prefs.getBoolean(context.getString( + R.string.enable_playback_state_lists_key), true); + } +} diff --git a/app/src/main/res/xml/history_settings.xml b/app/src/main/res/xml/history_settings.xml index b6126b10a73..46111635c20 100644 --- a/app/src/main/res/xml/history_settings.xml +++ b/app/src/main/res/xml/history_settings.xml @@ -23,6 +23,7 @@