From 0113ad5e1490c483622dc7f4870ec2e18f9b8077 Mon Sep 17 00:00:00 2001 From: Stypox Date: Mon, 7 Jun 2021 09:35:40 +0200 Subject: [PATCH] Correctly save stream progress at the end of a video --- .../history/dao/StreamHistoryDAO.java | 4 +- .../database/playlist/PlaylistStreamEntry.kt | 4 +- .../playlist/dao/PlaylistStreamDAO.java | 4 +- .../database/stream/StreamStatisticsEntry.kt | 6 +- .../database/stream/StreamWithState.kt | 4 +- .../stream/model/StreamStateEntity.java | 26 +++--- .../fragments/detail/VideoDetailFragment.java | 2 +- .../holder/StreamMiniInfoItemHolder.java | 6 +- .../newpipe/local/feed/item/StreamItem.kt | 2 +- .../local/history/HistoryRecordManager.java | 6 +- .../holder/LocalPlaylistStreamItemHolder.java | 10 +-- .../LocalStatisticStreamItemHolder.java | 10 +-- .../org/schabi/newpipe/player/Player.java | 82 ++++++++----------- 13 files changed, 74 insertions(+), 92 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/database/history/dao/StreamHistoryDAO.java b/app/src/main/java/org/schabi/newpipe/database/history/dao/StreamHistoryDAO.java index 535f2d2d0..0a765ed4e 100644 --- a/app/src/main/java/org/schabi/newpipe/database/history/dao/StreamHistoryDAO.java +++ b/app/src/main/java/org/schabi/newpipe/database/history/dao/StreamHistoryDAO.java @@ -21,7 +21,7 @@ import static org.schabi.newpipe.database.stream.StreamStatisticsEntry.STREAM_WA import static org.schabi.newpipe.database.stream.model.StreamEntity.STREAM_ID; import static org.schabi.newpipe.database.stream.model.StreamEntity.STREAM_TABLE; import static org.schabi.newpipe.database.stream.model.StreamStateEntity.JOIN_STREAM_ID_ALIAS; -import static org.schabi.newpipe.database.stream.model.StreamStateEntity.STREAM_PROGRESS_TIME; +import static org.schabi.newpipe.database.stream.model.StreamStateEntity.STREAM_PROGRESS_MILLIS; import static org.schabi.newpipe.database.stream.model.StreamStateEntity.STREAM_STATE_TABLE; @Dao @@ -80,7 +80,7 @@ public abstract class StreamHistoryDAO implements HistoryDAO { + " LEFT JOIN " + "(SELECT " + JOIN_STREAM_ID + " AS " + JOIN_STREAM_ID_ALIAS + ", " - + STREAM_PROGRESS_TIME + + STREAM_PROGRESS_MILLIS + " FROM " + STREAM_STATE_TABLE + " )" + " ON " + STREAM_ID + " = " + JOIN_STREAM_ID_ALIAS diff --git a/app/src/main/java/org/schabi/newpipe/database/stream/StreamStatisticsEntry.kt b/app/src/main/java/org/schabi/newpipe/database/stream/StreamStatisticsEntry.kt index eca12f584..9a622f643 100644 --- a/app/src/main/java/org/schabi/newpipe/database/stream/StreamStatisticsEntry.kt +++ b/app/src/main/java/org/schabi/newpipe/database/stream/StreamStatisticsEntry.kt @@ -5,7 +5,7 @@ import androidx.room.Embedded import org.schabi.newpipe.database.LocalItem import org.schabi.newpipe.database.history.model.StreamHistoryEntity import org.schabi.newpipe.database.stream.model.StreamEntity -import org.schabi.newpipe.database.stream.model.StreamStateEntity.STREAM_PROGRESS_TIME +import org.schabi.newpipe.database.stream.model.StreamStateEntity.STREAM_PROGRESS_MILLIS import org.schabi.newpipe.extractor.stream.StreamInfoItem import java.time.OffsetDateTime @@ -13,8 +13,8 @@ class StreamStatisticsEntry( @Embedded val streamEntity: StreamEntity, - @ColumnInfo(name = STREAM_PROGRESS_TIME, defaultValue = "0") - val progressTime: Long, + @ColumnInfo(name = STREAM_PROGRESS_MILLIS, defaultValue = "0") + val progressMillis: Long, @ColumnInfo(name = StreamHistoryEntity.JOIN_STREAM_ID) val streamId: Long, diff --git a/app/src/main/java/org/schabi/newpipe/database/stream/StreamWithState.kt b/app/src/main/java/org/schabi/newpipe/database/stream/StreamWithState.kt index 759dbae29..abeabf888 100644 --- a/app/src/main/java/org/schabi/newpipe/database/stream/StreamWithState.kt +++ b/app/src/main/java/org/schabi/newpipe/database/stream/StreamWithState.kt @@ -9,6 +9,6 @@ data class StreamWithState( @Embedded val stream: StreamEntity, - @ColumnInfo(name = StreamStateEntity.STREAM_PROGRESS_TIME) - val stateProgressTime: Long? + @ColumnInfo(name = StreamStateEntity.STREAM_PROGRESS_MILLIS) + val stateProgressMillis: Long? ) diff --git a/app/src/main/java/org/schabi/newpipe/database/stream/model/StreamStateEntity.java b/app/src/main/java/org/schabi/newpipe/database/stream/model/StreamStateEntity.java index fd8dc0a42..63a21fa9a 100644 --- a/app/src/main/java/org/schabi/newpipe/database/stream/model/StreamStateEntity.java +++ b/app/src/main/java/org/schabi/newpipe/database/stream/model/StreamStateEntity.java @@ -23,7 +23,7 @@ public class StreamStateEntity { // This additional field is required for the SQL query because 'stream_id' is used // for some other joins already public static final String JOIN_STREAM_ID_ALIAS = "stream_id_alias"; - public static final String STREAM_PROGRESS_TIME = "progress_time"; + public static final String STREAM_PROGRESS_MILLIS = "progress_time"; /** * Playback state will not be saved, if playback time is less than this threshold. @@ -39,12 +39,12 @@ public class StreamStateEntity { @ColumnInfo(name = JOIN_STREAM_ID) private long streamUid; - @ColumnInfo(name = STREAM_PROGRESS_TIME) - private long progressTime; + @ColumnInfo(name = STREAM_PROGRESS_MILLIS) + private long progressMillis; - public StreamStateEntity(final long streamUid, final long progressTime) { + public StreamStateEntity(final long streamUid, final long progressMillis) { this.streamUid = streamUid; - this.progressTime = progressTime; + this.progressMillis = progressMillis; } public long getStreamUid() { @@ -55,12 +55,12 @@ public class StreamStateEntity { this.streamUid = streamUid; } - public long getProgressTime() { - return progressTime; + public long getProgressMillis() { + return progressMillis; } - public void setProgressTime(final long progressTime) { - this.progressTime = progressTime; + public void setProgressMillis(final long progressMillis) { + this.progressMillis = progressMillis; } /** @@ -69,7 +69,7 @@ public class StreamStateEntity { * @return whether this stream state entity should be saved or not */ public boolean isValid() { - return progressTime > PLAYBACK_SAVE_THRESHOLD_START_MILLISECONDS; + return progressMillis > PLAYBACK_SAVE_THRESHOLD_START_MILLISECONDS; } /** @@ -82,15 +82,15 @@ public class StreamStateEntity { * @return whether the stream is finished or not */ public boolean isFinished(final long durationInSeconds) { - return progressTime >= durationInSeconds * 1000 - PLAYBACK_FINISHED_END_MILLISECONDS - && progressTime >= durationInSeconds * 1000 * 3 / 4; + return progressMillis >= durationInSeconds * 1000 - PLAYBACK_FINISHED_END_MILLISECONDS + && progressMillis >= durationInSeconds * 1000 * 3 / 4; } @Override public boolean equals(@Nullable final Object obj) { if (obj instanceof StreamStateEntity) { return ((StreamStateEntity) obj).streamUid == streamUid - && ((StreamStateEntity) obj).progressTime == progressTime; + && ((StreamStateEntity) obj).progressMillis == progressMillis; } else { return false; } 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 784a1c3be..ee9037537 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 @@ -1669,7 +1669,7 @@ public final class VideoDetailFragment .onErrorComplete() .observeOn(AndroidSchedulers.mainThread()) .subscribe(state -> { - showPlaybackProgress(state.getProgressTime(), info.getDuration() * 1000); + showPlaybackProgress(state.getProgressMillis(), info.getDuration() * 1000); animate(binding.positionView, true, 500); animate(binding.detailPositionView, true, 500); }, e -> { 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 227c11f91..98699eb95 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 @@ -66,7 +66,7 @@ public class StreamMiniInfoItemHolder extends InfoItemHolder { itemProgressView.setVisibility(View.VISIBLE); itemProgressView.setMax((int) item.getDuration()); itemProgressView.setProgress((int) TimeUnit.MILLISECONDS - .toSeconds(state2.getProgressTime())); + .toSeconds(state2.getProgressMillis())); } else { itemProgressView.setVisibility(View.GONE); } @@ -121,10 +121,10 @@ public class StreamMiniInfoItemHolder extends InfoItemHolder { itemProgressView.setMax((int) item.getDuration()); if (itemProgressView.getVisibility() == View.VISIBLE) { itemProgressView.setProgressAnimated((int) TimeUnit.MILLISECONDS - .toSeconds(state.getProgressTime())); + .toSeconds(state.getProgressMillis())); } else { itemProgressView.setProgress((int) TimeUnit.MILLISECONDS - .toSeconds(state.getProgressTime())); + .toSeconds(state.getProgressMillis())); ViewUtils.animate(itemProgressView, true, 500); } } else if (itemProgressView.getVisibility() == View.VISIBLE) { diff --git a/app/src/main/java/org/schabi/newpipe/local/feed/item/StreamItem.kt b/app/src/main/java/org/schabi/newpipe/local/feed/item/StreamItem.kt index dbd675161..13ba7592b 100644 --- a/app/src/main/java/org/schabi/newpipe/local/feed/item/StreamItem.kt +++ b/app/src/main/java/org/schabi/newpipe/local/feed/item/StreamItem.kt @@ -29,7 +29,7 @@ data class StreamItem( } private val stream: StreamEntity = streamWithState.stream - private val stateProgressTime: Long? = streamWithState.stateProgressTime + private val stateProgressTime: Long? = streamWithState.stateProgressMillis override fun getId(): Long = stream.uid diff --git a/app/src/main/java/org/schabi/newpipe/local/history/HistoryRecordManager.java b/app/src/main/java/org/schabi/newpipe/local/history/HistoryRecordManager.java index a0bd2f479..d2ffbfc27 100644 --- a/app/src/main/java/org/schabi/newpipe/local/history/HistoryRecordManager.java +++ b/app/src/main/java/org/schabi/newpipe/local/history/HistoryRecordManager.java @@ -228,14 +228,12 @@ public class HistoryRecordManager { .subscribeOn(Schedulers.io()); } - public Completable saveStreamState(@NonNull final StreamInfo info, final long progressTime) { + public Completable saveStreamState(@NonNull final StreamInfo info, final long progressMillis) { return Completable.fromAction(() -> database.runInTransaction(() -> { final long streamId = streamTable.upsert(new StreamEntity(info)); - final StreamStateEntity state = new StreamStateEntity(streamId, progressTime); + final StreamStateEntity state = new StreamStateEntity(streamId, progressMillis); if (state.isValid()) { streamStateTable.upsert(state); - } else { - streamStateTable.deleteState(streamId); } })).subscribeOn(Schedulers.io()); } 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 fd6c8d1d1..903f10440 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 @@ -68,11 +68,11 @@ public class LocalPlaylistStreamItemHolder extends LocalItemHolder { R.color.duration_background_color)); itemDurationView.setVisibility(View.VISIBLE); - if (item.getProgressTime() > 0) { + if (item.getProgressMillis() > 0) { itemProgressView.setVisibility(View.VISIBLE); itemProgressView.setMax((int) item.getStreamEntity().getDuration()); itemProgressView.setProgress((int) TimeUnit.MILLISECONDS - .toSeconds(item.getProgressTime())); + .toSeconds(item.getProgressMillis())); } else { itemProgressView.setVisibility(View.GONE); } @@ -109,14 +109,14 @@ public class LocalPlaylistStreamItemHolder extends LocalItemHolder { } final PlaylistStreamEntry item = (PlaylistStreamEntry) localItem; - if (item.getProgressTime() > 0 && item.getStreamEntity().getDuration() > 0) { + if (item.getProgressMillis() > 0 && item.getStreamEntity().getDuration() > 0) { itemProgressView.setMax((int) item.getStreamEntity().getDuration()); if (itemProgressView.getVisibility() == View.VISIBLE) { itemProgressView.setProgressAnimated((int) TimeUnit.MILLISECONDS - .toSeconds(item.getProgressTime())); + .toSeconds(item.getProgressMillis())); } else { itemProgressView.setProgress((int) TimeUnit.MILLISECONDS - .toSeconds(item.getProgressTime())); + .toSeconds(item.getProgressMillis())); ViewUtils.animate(itemProgressView, true, 500); } } else if (itemProgressView.getVisibility() == View.VISIBLE) { 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 7c4e47c36..adf6bd5c2 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 @@ -96,11 +96,11 @@ public class LocalStatisticStreamItemHolder extends LocalItemHolder { R.color.duration_background_color)); itemDurationView.setVisibility(View.VISIBLE); - if (item.getProgressTime() > 0) { + if (item.getProgressMillis() > 0) { itemProgressView.setVisibility(View.VISIBLE); itemProgressView.setMax((int) item.getStreamEntity().getDuration()); itemProgressView.setProgress((int) TimeUnit.MILLISECONDS - .toSeconds(item.getProgressTime())); + .toSeconds(item.getProgressMillis())); } else { itemProgressView.setVisibility(View.GONE); } @@ -140,14 +140,14 @@ public class LocalStatisticStreamItemHolder extends LocalItemHolder { } final StreamStatisticsEntry item = (StreamStatisticsEntry) localItem; - if (item.getProgressTime() > 0 && item.getStreamEntity().getDuration() > 0) { + if (item.getProgressMillis() > 0 && item.getStreamEntity().getDuration() > 0) { itemProgressView.setMax((int) item.getStreamEntity().getDuration()); if (itemProgressView.getVisibility() == View.VISIBLE) { itemProgressView.setProgressAnimated((int) TimeUnit.MILLISECONDS - .toSeconds(item.getProgressTime())); + .toSeconds(item.getProgressMillis())); } else { itemProgressView.setProgress((int) TimeUnit.MILLISECONDS - .toSeconds(item.getProgressTime())); + .toSeconds(item.getProgressMillis())); ViewUtils.animate(itemProgressView, true, 500); } } else if (itemProgressView.getVisibility() == View.VISIBLE) { 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 55e2e2dd7..a6be078be 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -674,7 +674,7 @@ public final class Player implements if (!state.isFinished(newQueue.getItem().getDuration())) { // resume playback only if the stream was not played to the end newQueue.setRecovery(newQueue.getIndex(), - state.getProgressTime()); + state.getProgressMillis()); } initPlayback(newQueue, repeatMode, playbackSpeed, playbackPitch, playbackSkipSilence, playWhenReady, isMuted); @@ -1939,9 +1939,7 @@ public final class Player implements break; case com.google.android.exoplayer2.Player.STATE_ENDED: // 4 changeState(STATE_COMPLETED); - if (currentMetadata != null) { - resetStreamProgressState(currentMetadata.getMetadata()); - } + saveStreamProgressStateCompleted(); isPrepared = false; break; } @@ -2402,7 +2400,7 @@ public final class Player implements case DISCONTINUITY_REASON_SEEK_ADJUSTMENT: case DISCONTINUITY_REASON_INTERNAL: if (playQueue.getIndex() != newWindowIndex) { - resetStreamProgressState(playQueue.getItem()); + saveStreamProgressStateCompleted(); // current stream has ended playQueue.setIndex(newWindowIndex); } break; @@ -2793,61 +2791,47 @@ public final class Player implements } } - private void saveStreamProgressState(final StreamInfo info, final long progress) { - if (info == null) { + private void saveStreamProgressState(final long progressMillis) { + if (currentMetadata == null + || !prefs.getBoolean(context.getString(R.string.enable_watch_history_key), true)) { return; } if (DEBUG) { - Log.d(TAG, "saveStreamProgressState() called"); + Log.d(TAG, "saveStreamProgressState() called with: progressMillis=" + progressMillis + + ", currentMetadata=[" + currentMetadata.getMetadata().getName() + "]"); } - if (prefs.getBoolean(context.getString(R.string.enable_watch_history_key), true)) { - final Disposable stateSaver = recordManager.saveStreamState(info, progress) - .observeOn(AndroidSchedulers.mainThread()) - .doOnError((e) -> { - if (DEBUG) { - e.printStackTrace(); - } - }) - .onErrorComplete() - .subscribe(); - databaseUpdateDisposable.add(stateSaver); - } - } - private void resetStreamProgressState(final PlayQueueItem queueItem) { - if (queueItem == null) { - return; - } - if (prefs.getBoolean(context.getString(R.string.enable_watch_history_key), true)) { - final Disposable stateSaver = queueItem.getStream() - .flatMapCompletable(info -> recordManager.saveStreamState(info, 0)) - .observeOn(AndroidSchedulers.mainThread()) - .doOnError((e) -> { - if (DEBUG) { - e.printStackTrace(); - } - }) - .onErrorComplete() - .subscribe(); - databaseUpdateDisposable.add(stateSaver); - } - } - - private void resetStreamProgressState(final StreamInfo info) { - saveStreamProgressState(info, 0); + databaseUpdateDisposable + .add(recordManager.saveStreamState(currentMetadata.getMetadata(), progressMillis) + .observeOn(AndroidSchedulers.mainThread()) + .doOnError((e) -> { + if (DEBUG) { + e.printStackTrace(); + } + }) + .onErrorComplete() + .subscribe()); } public void saveStreamProgressState() { - if (exoPlayerIsNull() || currentMetadata == null) { + if (exoPlayerIsNull() || currentMetadata == null || playQueue == null + || playQueue.getIndex() != simpleExoPlayer.getCurrentWindowIndex()) { + // Make sure play queue and current window index are equal, to prevent saving state for + // the wrong stream on discontinuity (e.g. when the stream just changed but the + // playQueue index and currentMetadata still haven't updated) return; } - final StreamInfo currentInfo = currentMetadata.getMetadata(); - if (playQueue != null) { - // Save current position. It will help to restore this position once a user - // wants to play prev or next stream from the queue - playQueue.setRecovery(playQueue.getIndex(), simpleExoPlayer.getContentPosition()); + // Save current position. It will help to restore this position once a user + // wants to play prev or next stream from the queue + playQueue.setRecovery(playQueue.getIndex(), simpleExoPlayer.getContentPosition()); + saveStreamProgressState(simpleExoPlayer.getCurrentPosition()); + } + + public void saveStreamProgressStateCompleted() { + if (currentMetadata != null) { + // current stream has ended, so the progress is its duration (+1 to overcome rounding) + saveStreamProgressState((currentMetadata.getMetadata().getDuration() + 1) * 1000); } - saveStreamProgressState(currentInfo, simpleExoPlayer.getCurrentPosition()); } //endregion