Added the ability to remove all watched videos from a local playlist #3065

GradyTheDev · 2020-02-08T06:41:44Z

Features:

Added the ability to remove all watched videos from a local playlist

Tested on:

Pixel
I carefully read the contribution guidelines and agree to them.

EDIT: Changed Location of 'Remove Watched' button to the three dot menu, see below comments for photos.

Before:

Loading Screen:

After:

TobiGr · 2020-02-08T16:55:54Z

Thanks for the contribution. I guess the idea is to have a dynamic playlist which contains all videos you want to watch later. Is this what you want to achieve? If this is a case, we might want to create a "watch later" playlist which comes with this feature by default.

Otherwise, I'd suggest to move this action to the menu in the toolbar.

GradyTheDev · 2020-02-08T17:25:27Z

Yeah, a 'watch later' playlist was the idea.

I'll look into making the a 'watch later' playlist.
But a few questions first.

Questions:

What do you think would be better?
- A. A 'watch later' playlist type, like [ local and remote ] playlists.
  - So that you could have multiple 'Watch Later' playlists for different occasions.
- B. A single default playlist that everyone gets.
  - Like YouTube's watch later playlist.

Do you think the removal of video should be automatic or manual.
- A. Automatic
  - Keeps things simple, but people might want to later add the videos to other playlists,
    or to watch them again, and they would have to go through their history to find the videos.
- B. Manual
  - Allows the user to come back to it later,
    but the removal is manual, albeit easier then going video by video.

Stypox · 2020-03-05T09:12:18Z

Here is my opinion, sorry for the late response @GradyClark:

Playlists should all be of the same type (i.e. local type).
There should be a button in the dot-menu that removes all the watched videos from any local playlist.

This requires an action by the user, but the automatic alternative has this problem: what if one wanted to watch a video, but then changed his mind after opening it? That video would be removed from the watch-later playlist, which was not intended. So I think we should leave this manual. The other advantage of this approach is that there is no need to implement different types of playlists, or such. Basically you just have to move the button you added to the three-dot menu and we are good to go.

If this is a case, we might want to create a "watch later" playlist which comes with this feature by default.

@TobiGr For the reasons explained above I think an automatic-removal watch later playlist would be problematic, and the alternative of pressing a button is not that much of a hassle.

GradyTheDev · 2020-03-05T14:08:50Z

Changed the location of the 'Remove Watched' button, to the menu.

Stypox · 2020-03-05T20:39:42Z

@GradyClark could you provide a test apk based on the latest changes? Just zip it and upload it here

GradyTheDev · 2020-03-06T05:35:10Z

Here's the zipped apk.

NewPipe-debug.zip

Stypox

Are we sure we want to use an AsyncTask? In the other parts of the repository we use ReactiveX. In this case it would greatly simplify your code, since you could plug in directly to the playlistManager.getPlaylistStreams(playlistId); flowable.
Anyways, the APK works correctly, thank you :-D

app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java

app/src/main/res/layout/local_playlist_header.xml

app/src/main/res/menu/menu_local_playlist.xml

app/src/main/res/values/strings.xml

GradyTheDev · 2020-03-08T13:15:56Z

Merged it with current 'dev' branch. 8fa29ff
Tested on Pixel, no issues occurred.

APK:
NewPipe-debug.zip

Stypox

Thank you for implementing the ReactiveX method. Now the approach is correct, the things I pointed out are more about style and code organization. :-D
@GradyClark

app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java

Stypox

I just realized the algorithm to check for history has a complexity of O(m*n), where n is the number of watched videos and m is the length of the playlist. This is quadratic and grows fast, needing more and more time to complete as history grows. Take a look at HistoryRecordManage.loadLocalStreamStateBatch(). With that method you can load the watched state of every stream at once and the complexity would be lowered to O(m*log(n)) which is a great improvement.

app/src/main/res/values/strings.xml

app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java

GradyTheDev · 2020-03-18T07:12:21Z

HistoryRecordManage.loadLocalStreamStateBatch()
Only returns videos with a, where you left off, marker.
I had to do it manually in 6223c36

Thanks for the advice!
For a ~400 video playlist.
It was around 4x faster!
Dropped from 39 -> 9 seconds

Changes: - local_playlist_control.xml * A copy of playlist_control.xml * To hold the 'Remove Watched Videos' buttton - local_playlist_header.xml * Changed the include layout to now include local_playlist_control.xml - strings.xml * added string 'remove_watched' with value 'Remove Watched' - LocalPlaylistFragment.java * Added the functionality to remove watched videos, to the 'Remove Watched Videos' button in local_playlist_control.xml. In the background via AsyncTask. This will also change the playlist's thumbnail, if the thumbnail video is removed. Tested on: - Pixel

…videos. - Removed redundant file 'local_playlist_control' - Fixed grammer issue

…tchedStreams` Replaced unnecessary nested class. Fixed formating issues

Removed merge mistake Reordered code Refactored 'removeWatchedWorker' to 'removeWatchedDisposable'

Stypox · 2020-04-03T18:02:04Z

@GradyClark I pushed a commit that really makes the algoritm O(m*log(n)): it took ~2s to remove watched videos from a playlist with ~2500 videos and with with a history size of ~2500 videos. I did this by getting the stream ids in ascending order from the database, so that I could apply a binary search on it. Thus, the algorithm uses log(n) operations to search for the id every playlisted video inside history.
I would have told you how to do this, but I didn't know either, so I decided to experiment myself and after a while I pushed a commit; hope this is not a problem. 😄 What do you think about my changes? Could you do a review?
Also, sorry for responding so late, there have been 2 versions to review and release lately 😅

Stypox · 2020-04-10T13:55:44Z

@TobiGr can I merge this? The work now is polished and fast, as explained in the comment above.

GradyTheDev · 2020-04-10T20:30:27Z

Sorry for the late reply, I've been busy.

Also I haven't been able to test the new code, Android Studio has been troublesome.
It's refusing to compile. saying
"Unfortunately you can't have non-Gradle Java modules and Android-Gradle modules in one project."

Once I get that sorted out I will test it, and get back with you on that.

As for how the code looks, it looks great!
I especially like the efficiency enhancements you made.

Stypox · 2020-04-10T20:40:22Z

"Unfortunately you can't have non-Gradle Java modules and Android-Gradle modules in one project."

Try Build -> Clean Project, and if even that does not work try File -> Invalidate caches / restart

TobiGr · 2020-04-10T20:46:19Z

Two things to note which might be good to be displayed to the user in a dialog:

When removing watched videos, videos which have not been completed yet, are also removed. Is this a bug or intended. If it's intended, we should tell the user about this.
when a video was watched before it was added to the playlist, it is also removed. In other words, the "remove watched" is global and not restrict to the current playlist.

The dialog should have an option to "do not show again". @Stypox @GradyClark @wb9688 What do you think?

wb9688 · 2020-04-10T20:51:42Z

Here is my comment on this PR's usage of ReactiveX that I sent to @Stypox on IRC:

GradyTheDev · 2020-04-10T20:51:46Z

Try Build -> Clean Project

That did the trick
Thanks @Stypox

I just tested the code, it works without a hitch.
92ca1e6

GradyTheDev · 2020-04-10T21:15:16Z

When removing watched videos, videos which have not been completed yet, are also removed. Is this a bug or intended. If it's intended, we should tell the user about this.

I don't think there is a way to know for sure that the video watched watched completely.
We can tell at which time they stopped playing the video, and if the video has been played at all.
But we don't have any way of knowing how much of it was watched.
As far as I know.

We could have it ignore any video with a 'where you left off' marker.
But also have the option to remove those as well.

when a video was watched before it was added to the playlist, it is also removed. In other words, the "remove watched" is global and not restrict to the current playlist.

That was the intended function.
But having a "remove only the videos that were added to the playlist and then watched", I think would be a good additional option.

Solution:
Perhaps when the "remove watched videos" is pressed a dialog pops up and asks what you want to do

1st dialog
"Do you want to remove video that have been partially watched? Yes / No ?"
2nd dialog
"Do you want to remove only the videos that have been watched after being added to the playlist? Yes / No?"

What are your opinions?
@Stypox, @TobiGr, @wb9688

wb9688 · 2020-04-10T21:19:59Z

Show one "are you sure?" dialog with those 2 things as checkboxes.

Stypox · 2020-04-11T07:46:37Z

But wouldn't the third option require a database restructuring, to keep track of when every video has been added to a playlist?

GradyTheDev · 2020-04-11T15:13:14Z

That would require changes to the database, perhaps just warning the user will suffice.

- Will now execute on the io thread - Added confirmation dialog - Warning the user, and asking if they also want to remove partially watched videos

GradyTheDev · 2020-04-14T18:38:53Z

Latest and Greatest
APK: NewPipe-debug.zip

Stypox

I'm not sure about one thing: videos that have been watched for <=5s would have a null stream state, since the player saves the timestamp only if it is >5s. This could cause issues when the user presses "Delete all completely watched videos", since it would also erase videos that have been watched for <=5s.
Edit: the above concern should not be fixed here, I think. See #3371 (comment)

app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java

app/src/main/res/values/strings.xml

app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java

reorganized code

wb9688

In general the code looks good though

app/src/main/res/menu/menu_local_playlist.xml

app/src/main/res/values/strings.xml

app/src/main/java/org/schabi/newpipe/local/playlist/LocalPlaylistFragment.java

Also do not show any dialog if the user is aready removing watched videos in a local playlist

Stypox

Thank you again @GradyClark :-D

outdated

GradyTheDev · 2020-04-25T10:05:57Z

Your welcome.
NewPipe has been awesome to use, and I wanted to contribute.

Stypox self-assigned this Mar 5, 2020

Stypox requested changes Mar 6, 2020

View reviewed changes

Stypox requested changes Mar 11, 2020

View reviewed changes

Stypox requested changes Mar 17, 2020

View reviewed changes

GradyTheDev and others added 6 commits April 3, 2020 19:47

Moved the 'Remove Watched' button to the three dot menu button.

954399b

- Will now use ReactiveX instead of AsyncTask, when removing watched …

8cab790

…videos. - Removed redundant file 'local_playlist_control' - Fixed grammer issue

Simplified the removal of watched videos, into the function `removeWa…

98fc88d

…tchedStreams` Replaced unnecessary nested class. Fixed formating issues

Optimised 'removeWatchedStreams'

0ac2865

Removed merge mistake Reordered code Refactored 'removeWatchedWorker' to 'removeWatchedDisposable'

Use binary search to remove watched items from playlists

9c3f138

Stypox added 2 commits April 3, 2020 20:13

Fix checkstyle errors

6571fdb

Check if already running before removing watched

92ca1e6

Stypox added this to the 0.19.3 milestone Apr 10, 2020

GradyTheDev added 2 commits April 14, 2020 13:15

Changed "Remove Watched":

fe1646c

- Will now execute on the io thread - Added confirmation dialog - Warning the user, and asking if they also want to remove partially watched videos

Merge branch 'dev' of https://github.com/TeamNewPipe/NewPipe into dev

1d0c3de

Stypox requested changes Apr 15, 2020

View reviewed changes

Removed redundant code

db335d5

reorganized code

wb9688 previously requested changes Apr 19, 2020

View reviewed changes

GradyTheDev and others added 3 commits April 21, 2020 01:03

fixed naming and formatting issues

776dddd

Code cleanup, and best practices

7361100

Use centralized CompositeDisposable instead of custom Disposable

437b86d

Also do not show any dialog if the user is aready removing watched videos in a local playlist

Stypox approved these changes Apr 23, 2020

View reviewed changes

Stypox merged commit 2db0d63 into TeamNewPipe:dev Apr 23, 2020

This was referenced Apr 24, 2020

Release 0.19.3 #3478

Merged

Release 0.19.3 #3479

Closed

wb9688 mentioned this pull request Apr 26, 2020

Sorting Playlist via time added #3486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added the ability to remove all watched videos from a local playlist #3065

Added the ability to remove all watched videos from a local playlist #3065

GradyTheDev commented Feb 8, 2020 •

edited

Loading

TobiGr commented Feb 8, 2020

GradyTheDev commented Feb 8, 2020 •

edited

Loading

Stypox commented Mar 5, 2020 •

edited

Loading

GradyTheDev commented Mar 5, 2020

Stypox commented Mar 5, 2020 •

edited

Loading

GradyTheDev commented Mar 6, 2020

Stypox left a comment

GradyTheDev commented Mar 8, 2020

Stypox left a comment •

edited

Loading

Stypox left a comment •

edited

Loading

GradyTheDev commented Mar 18, 2020 •

edited

Loading

Stypox commented Apr 3, 2020 •

edited

Loading

Stypox commented Apr 10, 2020

GradyTheDev commented Apr 10, 2020 •

edited

Loading

Stypox commented Apr 10, 2020

TobiGr commented Apr 10, 2020

wb9688 commented Apr 10, 2020

GradyTheDev commented Apr 10, 2020 •

edited

Loading

GradyTheDev commented Apr 10, 2020 •

edited

Loading

wb9688 commented Apr 10, 2020

Stypox commented Apr 11, 2020

GradyTheDev commented Apr 11, 2020

GradyTheDev commented Apr 14, 2020

Stypox left a comment •

edited

Loading

wb9688 left a comment

Stypox left a comment

GradyTheDev commented Apr 25, 2020

Added the ability to remove all watched videos from a local playlist #3065

Added the ability to remove all watched videos from a local playlist #3065

Conversation

GradyTheDev commented Feb 8, 2020 • edited Loading

TobiGr commented Feb 8, 2020

GradyTheDev commented Feb 8, 2020 • edited Loading

Stypox commented Mar 5, 2020 • edited Loading

GradyTheDev commented Mar 5, 2020

Stypox commented Mar 5, 2020 • edited Loading

GradyTheDev commented Mar 6, 2020

Stypox left a comment

Choose a reason for hiding this comment

GradyTheDev commented Mar 8, 2020

Stypox left a comment • edited Loading

Choose a reason for hiding this comment

Stypox left a comment • edited Loading

Choose a reason for hiding this comment

GradyTheDev commented Mar 18, 2020 • edited Loading

Stypox commented Apr 3, 2020 • edited Loading

Stypox commented Apr 10, 2020

GradyTheDev commented Apr 10, 2020 • edited Loading

Stypox commented Apr 10, 2020

TobiGr commented Apr 10, 2020

wb9688 commented Apr 10, 2020

GradyTheDev commented Apr 10, 2020 • edited Loading

GradyTheDev commented Apr 10, 2020 • edited Loading

wb9688 commented Apr 10, 2020

Stypox commented Apr 11, 2020

GradyTheDev commented Apr 11, 2020

GradyTheDev commented Apr 14, 2020

Stypox left a comment • edited Loading

Choose a reason for hiding this comment

wb9688 left a comment

Choose a reason for hiding this comment

Stypox left a comment

Choose a reason for hiding this comment

GradyTheDev commented Apr 25, 2020

GradyTheDev commented Feb 8, 2020 •

edited

Loading

GradyTheDev commented Feb 8, 2020 •

edited

Loading

Stypox commented Mar 5, 2020 •

edited

Loading

Stypox commented Mar 5, 2020 •

edited

Loading

Stypox left a comment •

edited

Loading

Stypox left a comment •

edited

Loading

GradyTheDev commented Mar 18, 2020 •

edited

Loading

Stypox commented Apr 3, 2020 •

edited

Loading

GradyTheDev commented Apr 10, 2020 •

edited

Loading

GradyTheDev commented Apr 10, 2020 •

edited

Loading

GradyTheDev commented Apr 10, 2020 •

edited

Loading

Stypox left a comment •

edited

Loading