Correctly save stream progress at the end of a video

Stypox · Stypox · commit 0113ad5e1490 · 2021-06-09T15:53:51.000+02:00
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
@@ -21,7 +21,7 @@
 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 Flowable<List<StreamHistoryEntity>> listByService(final int serviceId) {
 
             + " 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)
     public abstract Flowable<List<StreamStatisticsEntry>> getStatistics();
diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistStreamEntry.kt b/app/src/main/java/org/schabi/newpipe/database/playlist/PlaylistStreamEntry.kt
@@ -12,8 +12,8 @@ data class PlaylistStreamEntry(
     @Embedded
     val streamEntity: StreamEntity,
 
-    @ColumnInfo(name = StreamStateEntity.STREAM_PROGRESS_TIME, defaultValue = "0")
-    val progressTime: Long,
+    @ColumnInfo(name = StreamStateEntity.STREAM_PROGRESS_MILLIS, defaultValue = "0")
+    val progressMillis: Long,
 
     @ColumnInfo(name = PlaylistStreamEntity.JOIN_STREAM_ID)
     val streamId: Long,
diff --git a/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistStreamDAO.java b/app/src/main/java/org/schabi/newpipe/database/playlist/dao/PlaylistStreamDAO.java
@@ -25,7 +25,7 @@
 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
@@ -64,7 +64,7 @@ default Flowable<List<PlaylistStreamEntity>> listByService(final int serviceId)
 
             + " 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
@@ -5,16 +5,16 @@ 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
 
 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
@@ -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
@@ -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 void setStreamUid(final long streamUid) {
         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 void setProgressTime(final long progressTime) {
      * @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 boolean isValid() {
      * @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
@@ -1669,7 +1669,7 @@ private void updateProgressInfo(@NonNull final StreamInfo info) {
                 .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
@@ -66,7 +66,7 @@ public void updateFromItem(final InfoItem infoItem,
                 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 void updateState(final InfoItem infoItem,
             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
@@ -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
@@ -228,14 +228,12 @@ public Maybe<StreamStateEntity> loadStreamState(final StreamInfo info) {
                 .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
@@ -68,11 +68,11 @@ public void updateFromItem(final LocalItem localItem,
                     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 void updateState(final LocalItem localItem,
         }
         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
@@ -96,11 +96,11 @@ public void updateFromItem(final LocalItem localItem,
                     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 void updateState(final LocalItem localItem,
         }
         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
@@ -674,7 +674,7 @@ && isPlaybackResumeEnabled(this)
                                 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 void onPlayerStateChanged(final boolean playWhenReady, final int playback
                 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 void onPositionDiscontinuity(@DiscontinuityReason final int discontinuity
             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 @@ private void registerStreamViewed() {
         }
     }
 
-    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
 

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,6 @@ data class StreamWithState(`
`9`	`9`	`@Embedded`
`10`	`10`	`val stream: StreamEntity,`
`11`	`11`
`12`		`- @ColumnInfo(name = StreamStateEntity.STREAM_PROGRESS_TIME)`
`13`		`- val stateProgressTime: Long?`
	`12`	`+ @ColumnInfo(name = StreamStateEntity.STREAM_PROGRESS_MILLIS)`
	`13`	`+ val stateProgressMillis: Long?`
`14`	`14`	`)`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ data class StreamItem(`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`private val stream: StreamEntity = streamWithState.stream`
`32`		`- private val stateProgressTime: Long? = streamWithState.stateProgressTime`
	`32`	`+ private val stateProgressTime: Long? = streamWithState.stateProgressMillis`
`33`	`33`
`34`	`34`	`override fun getId(): Long = stream.uid`
`35`	`35`
Original file line number	Diff line number	Diff line change
`@@ -228,14 +228,12 @@ public Maybe<StreamStateEntity> loadStreamState(final StreamInfo info) {`
`228`	`228`	`.subscribeOn(Schedulers.io());`
`229`	`229`	`}`
`230`	`230`
`231`		`- public Completable saveStreamState(@NonNull final StreamInfo info, final long progressTime) {`
	`231`	`+ public Completable saveStreamState(@NonNull final StreamInfo info, final long progressMillis) {`
`232`	`232`	`return Completable.fromAction(() -> database.runInTransaction(() -> {`
`233`	`233`	`final long streamId = streamTable.upsert(new StreamEntity(info));`
`234`		`- final StreamStateEntity state = new StreamStateEntity(streamId, progressTime);`
	`234`	`+ final StreamStateEntity state = new StreamStateEntity(streamId, progressMillis);`
`235`	`235`	`if (state.isValid()) {`
`236`	`236`	`streamStateTable.upsert(state);`
`237`		`- } else {`
`238`		`- streamStateTable.deleteState(streamId);`
`239`	`237`	`}`
`240`	`238`	`})).subscribeOn(Schedulers.io());`
`241`	`239`	`}`