Download playlist #2018

VishalNehra · 2019-01-18T18:41:19Z

I carefully read the contribution guidelines and agree to them.

Hello,
My first contribution here. Saw there's missing option to download playlist, so tried implementing it.
Open to suggestions.
Fixes #1058

Edit: Also noticed, channel specific playlists are not shown on the channel page. I'm interested in implementing a way to show them too. Please point me to the issue if still exists.

…nload-playlist

theScrabi · 2019-01-18T20:15:58Z

I will not add features right now. There are already so many features and bugs at the moment that I/we can not take care about this any time soon.
So this will have to wait, and propably need to be refactored then.

TobiGr · 2019-06-17T21:07:48Z

@VishalNehra I've just merged a bunch of downloader changes (see #2149). I'd like to take a look at your changes soon. Can you please rebase?

VishalNehra · 2019-06-18T09:17:16Z

@TobiGr sure, will try this weekend.

gcandrade10 · 2019-06-24T14:12:55Z

I tried to resolve the conflicts and I just realize that your download all implementation triggers a pop-up for each item of the playlist that was the idea?
I'm not a UX or something but I was expecting to choose just one time the download settings (e.g audio, 32 threads) and that's it 🤔

VishalNehra · 2019-06-24T14:32:04Z

There is a smart download option in the download dialog which I added, which will identify your last chose setting and based on that will download remaining items of playlist without opening dialog for each.

gcandrade10 · 2019-06-24T14:45:29Z

Yes! you are right, it's an 'auto' checkbox!

VishalNehra · 2019-09-05T22:52:32Z

Alright, after hours of trouble to merge the latest dev branch here, I'm expecting this to get reviewed this time. @theScrabi

strongwinderic

Nice and clean code!

app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java

TobiGr · 2019-09-24T16:30:33Z

@VishalNehra The code looks good. I'll test it in the during the week.
@kapodamy You are aware of all things regarding the download process. Can you please review it, too?

kapodamy · 2019-09-24T20:59:21Z

@TobiGr looks fine, i submited a review that was not necessary

VishalNehra · 2019-09-24T22:39:05Z

Sure @TobiGr I'll be rebasing on weekend, and also have a minor fix which I didn't commit.

VishalNehra · 2019-10-19T13:58:23Z

I've rebased dev and fixed some NPEs
I'm done from my side, there might be some problems if we rotate screen at the same time we tick the 'auto' option in dialog.. apart from that, it seems stable enough now.
Thanks for the patience and sorry again for dragging this but I've been caught up with my daily job.
cc @Stypox @TobiGr

Stypox · 2019-10-21T08:34:36Z

app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java

@@ -138,6 +155,231 @@ public static void playOnBackgroundPlayer(final Context context, final PlayQueue
 startService(context, getPlayerIntent(context, BackgroundPlayer.class, queue, resumePlayback));
 }

+ public static void downloadPlaylist(final PlaylistFragment context, final PlayQueue queue) {


Do all of these PlaylistFragment types always have to be like that? Couldn't a Fragment or Activity work?

What exactly do you mean?

What if one wants to download a playlist from a different fragment? It would be better if the type of the parameter context was the base class Fragment and not PlaylistFragment, so that the method can be used everywhere.

Theoretically I can get away with using FragmentActivity instead (there would be some implications; like I'm passing onError implementation of PlaylistFragment for reactive stream failure, while in FragmentActivity won't be able to do that. If there were an abstract class, I could've used that instead, for this specific use case.
Apart from that, the initial call to initialize DownloadDialog#newInstance requires PlaylistFragment#callback now. Again, an abstract class would help here too, so in future you would need to extend that class wherever you would want to initialize this download dialog.
Right now it's tightly coupled with PlaylistFragment, but the change you're suggesting would require a lot of changes in many parts of your source.
I'll leave it upto you to decide for this.

Mmhh OK, in case I ever need to use it from elsewhere I will implement it myself

Good, in this case it is time for a last round of testing and then we should be ready, finally :)
Thanks again for keeping your branch updated @VishalNehra. I am sorry, we've kept you waiting way too long.

TobiGr · 2019-11-04T10:40:58Z

Using KitKat (API 19, Android 4.4) I get this Exception when downloading a YouTube (upstream) playlist:

android.view.InflateException: Binary XML file line #64: Error inflating class android.support.v7.widget.AppCompatCheckBox
        at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:708)
        at android.view.LayoutInflater.rInflate(LayoutInflater.java:756)
        at android.view.LayoutInflater.inflate(LayoutInflater.java:492)
        at android.view.LayoutInflater.inflate(LayoutInflater.java:397)
        at android.view.LayoutInflater.inflate(LayoutInflater.java:353)
        at org.schabi.newpipe.download.DownloadDialog.onCreateView(DownloadDialog.java:254)
        at androidx.fragment.app.Fragment.performCreateView(Fragment.java:2600)
        at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManagerImpl.java:881)
        at androidx.fragment.app.FragmentManagerImpl.moveFragmentToExpectedState(FragmentManagerImpl.java:1238)
        at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManagerImpl.java:1303)
        at androidx.fragment.app.BackStackRecord.executeOps(BackStackRecord.java:439)
        at androidx.fragment.app.FragmentManagerImpl.executeOps(FragmentManagerImpl.java:2079)
        at androidx.fragment.app.FragmentManagerImpl.executeOpsTogether(FragmentManagerImpl.java:1869)
        at androidx.fragment.app.FragmentManagerImpl.removeRedundantOperationsAndExecute(FragmentManagerImpl.java:1824)
        at androidx.fragment.app.FragmentManagerImpl.execPendingActions(FragmentManagerImpl.java:1727)
        at androidx.fragment.app.FragmentManagerImpl$2.run(FragmentManagerImpl.java:150)
        at android.os.Handler.handleCallback(Handler.java:733)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:136)
        at android.app.ActivityThread.main(ActivityThread.java:5017)
        at java.lang.reflect.Method.invokeNative(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:515)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:779)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:595)
        at dalvik.system.NativeStart.main(Native Method)
     Caused by: java.lang.ClassNotFoundException: Didn't find class "android.support.v7.widget.AppCompatCheckBox" on path: DexPathList[[zip file "/data/app/org.schabi.newpipe.debug-1.apk", zip file "/data/data/org.schabi.newpipe.debug/code_cache/secondary-dexes/org.schabi.newpipe.debug-1.apk.classes2.zip", zip file "/data/data/org.schabi.newpipe.debug/code_cache/secondary-dexes/org.schabi.newpipe.debug-1.apk.classes3.zip"],nativeLibraryDirectories=[/data/app-lib/org.schabi.newpipe.debug-1, /system/lib]]
        at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:56)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:497)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:457)
        at android.view.LayoutInflater.createView(LayoutInflater.java:559)
        at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:697)
        at android.view.LayoutInflater.rInflate(LayoutInflater.java:756) 
        at android.view.LayoutInflater.inflate(LayoutInflater.java:492) 
        at android.view.LayoutInflater.inflate(LayoutInflater.java:397) 
        at android.view.LayoutInflater.inflate(LayoutInflater.java:353) 
        at org.schabi.newpipe.download.DownloadDialog.onCreateView(DownloadDialog.java:254) 
        at androidx.fragment.app.Fragment.performCreateView(Fragment.java:2600) 
        at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManagerImpl.java:881) 
        at androidx.fragment.app.FragmentManagerImpl.moveFragmentToExpectedState(FragmentManagerImpl.java:1238) 
        at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManagerImpl.java:1303) 
        at androidx.fragment.app.BackStackRecord.executeOps(BackStackRecord.java:439) 
        at androidx.fragment.app.FragmentManagerImpl.executeOps(FragmentManagerImpl.java:2079) 
        at androidx.fragment.app.FragmentManagerImpl.executeOpsTogether(FragmentManagerImpl.java:1869) 
        at androidx.fragment.app.FragmentManagerImpl.removeRedundantOperationsAndExecute(FragmentManagerImpl.java:1824) 
        at androidx.fragment.app.FragmentManagerImpl.execPendingActions(FragmentManagerImpl.java:1727) 
        at androidx.fragment.app.FragmentManagerImpl$2.run(FragmentManagerImpl.java:150) 
        at android.os.Handler.handleCallback(Handler.java:733) 
        at android.os.Handler.dispatchMessage(Handler.java:95) 
        at android.os.Looper.loop(Looper.java:136) 
        at android.app.ActivityThread.main(ActivityThread.java:5017) 
        at java.lang.reflect.Method.invokeNative(Native Method) 
        at java.lang.reflect.Method.invoke(Method.java:515) 
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:779) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:595) 
        at dalvik.system.NativeStart.main(Native Method)

mauriciocolli

Had an overview, and in my opinion we should not merge this as it is. Here are some reasons:

Coupling to the fragment, which definitely shouldn't be necessary.
Spaghetti code in the NavigationHelper.
Bad UX to the user.
- Rotated the phone? Switched apps? Problems ahead.
This don't deal with configurations changes.
As the playlist in the fragment is paged, it isn't completely loaded yet, so this don't download all the playlist.

Creating a new screen showing a list of items giving the user the option to select which one they wanted, and the preferred configurations for all downloads (per-item configuration as well), would be much better here.

PS: Downloading all the items (the stream pages) might have an impact on reCAPTCHAs. YouTube seems to rate limit those a lot, so it should handle this as well.

mauriciocolli · 2019-11-04T16:31:45Z

app/src/main/java/org/schabi/newpipe/download/DownloadSetting.java

+ private StoredFileHelper storedFileHelper;
+ private int threadCount;
+ private String[] urls;
+ private char kind;


Not a good idea using char for this, a enum would be much better.

mauriciocolli · 2019-11-04T16:32:19Z

app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java

@@ -304,6 +308,12 @@ public void handleResult(@NonNull final PlaylistInfo result) {
 return true;
 });

+ headerDownloadAllButton.setOnClickListener(view -> {
+ if (PermissionHelper.checkStoragePermissions(activity, PermissionHelper.DOWNLOAD_DIALOG_REQUEST_CODE)) {
+ NavigationHelper.downloadPlaylist(this, getPlayQueue());


Strange use of the PlayQueue items here. Why not the items themselves?

mauriciocolli · 2019-11-04T16:32:27Z

app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java

@@ -345,7 +365,7 @@ public void handleNextItems(ListExtractor.InfoItemsPage result) {
 //////////////////////////////////////////////////////////////////////////*/

 @Override
- protected boolean onError(Throwable exception) {
+ public boolean onError(Throwable exception) {


This should strictly remain for internal use only.

mauriciocolli · 2019-11-04T16:33:24Z

app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java

+ final Iterator<PlayQueueItem> itemIterator,
+ DownloadSetting downloadSetting) {
+ if (downloadSetting != null) {
+ Completable.create(emitter -> {


The create method should not be used. Also this isn't the ideal place to subscribe.

mauriciocolli · 2019-11-04T16:33:30Z

app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java

+ * @param streamInfo
+ * @return
+ */
+ private static DownloadSetting refactorDownloadSetting(PlaylistFragment activity, DownloadSetting downloadSetting,


This entire method just feels out of place, navigation should be the only thing done in this class.

mauriciocolli · 2019-11-04T16:33:44Z

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

@@ -61,11 +61,24 @@
 android:text="@string/caption_setting_title"/>
 </RadioGroup>

+ <android.support.v7.widget.AppCompatCheckBox


This should be just CheckBox, let the library replace that for you.

mauriciocolli · 2019-11-04T16:33:50Z

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

+ android:layout_below="@+id/video_audio_group"
+ android:layout_marginBottom="6dp"
+ android:layout_marginLeft="20dp"
+ android:gravity="left"


No need to set gravity here, it will mess up the alignment. It'll also cause issues with RTL languages.

Stypox · 2019-12-15T10:46:34Z

@VishalNehra could you address @mauriciocolli's suggestions? @TobiGr it would be good to have this merged after 0.18.0 is released, so that I can start working on #2583. If you @VishalNehra have no time to dedicate to this pr anymore, feel free to tell me, I would be ok creating a new pr myself ;-)

gcandrade10 · 2020-01-20T00:17:33Z

Any updates here?

VishalNehra · 2020-01-23T07:30:58Z

Sorry for late reply, I did agree with @mauriciocolli however, my current schedule doesn't permit me to address all the changes.
I'm hoping some regular contributor of this project will pick this up.
If not, then I'll try to find some time in near future.

realjustinwu · 2020-04-11T15:36:10Z

Any updates here?

ghost · 2020-05-21T19:55:30Z

I feel like this will never get implemented. I am really hoping this will be added on v0.21.0 so @Spypox's #2583 can be added aswell. :)

B0pol · 2020-05-21T20:54:23Z

@Stypox worked on that but he was blocked waiting for @wb9688 download manager refactoring.

Stypox · 2020-05-26T09:34:02Z

I decided that I will implement half of #2583 without bulk downloading for now, hopefully in a few days I should have something working with the proper code structure (i.e. not just a demonstration)

MD77MD · 2020-06-17T00:22:35Z

@Stypox your just amazing!

Stypox · 2021-01-16T08:56:29Z

I'm closing this as this is not really a good approach to bulk downloading. It would leak in many places and the current gigaget downloader is just crappy. We a new downloader for NewPipe will be built we will be able to finally implement this properly. @VishalNehra thank you for your work anyway ;-)

0-Kala-0 · 2021-07-31T11:28:14Z

anything new on this topic ?

Hereux · 2023-08-18T15:06:03Z

Anything new on this topic?

opusforlife2 · 2023-08-18T19:40:40Z

I think the standard forum etiquette applies here. "Don't necropost/Don't bump zombie threads" = "Don't add noise to zombie PRs or issues."

VishalNehra added 4 commits January 18, 2019 23:44

Add UI for smart download of playlist

213fb35

Merge branch 'dev' of https:/TeamNewPipe/NewPipe into dow…

4b07220

…nload-playlist

Fix auto starting download of remaining items when skip is pressed

e612142

Use subscribe instead of blockingGet for auto download

7e8674b

TobiGr mentioned this pull request Jun 16, 2019

Download all/multiple items at once in channel/playlist/feed #2380

Closed

Stypox mentioned this pull request Jun 17, 2019

[Feature request] Dowload a playlist #2405

Closed

3 tasks

TobiGr mentioned this pull request Jul 1, 2019

[feature request] Download entire playlist? #1058

Closed

This was referenced Jul 19, 2019

(Idea) Download entire channels and playlists while holding down option. #1180

Closed

[Feature Request] Playlist downloading, syncing. #1236

Closed

theScrabi added the feature request Issue is related to a feature in the app label Jul 21, 2019

Merge dev

82a004a

strongwinderic previously approved these changes Sep 6, 2019

View reviewed changes

app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java Outdated Show resolved Hide resolved

Fix comments

a538078

VishalNehra dismissed strongwinderic’s stale review via a538078 September 8, 2019 07:48

strongwinderic previously approved these changes Sep 9, 2019

View reviewed changes

TobiGr mentioned this pull request Sep 24, 2019

Fast actions from mini info item - UI / UX ideas #2583

Closed

theScrabi requested review from karyogamy and removed request for karyogamy September 24, 2019 19:37

kapodamy previously approved these changes Sep 24, 2019

View reviewed changes

Stypox mentioned this pull request Sep 27, 2019

[Feature request] - download all from background player #2303

Open

3 tasks

Merge dev

6015fbd

VishalNehra dismissed stale reviews from Stypox, kapodamy, and strongwinderic via 6015fbd October 19, 2019 13:54

Merge branch 'dev' into download-playlist

93782c7

Stypox reviewed Oct 21, 2019

View reviewed changes

mauriciocolli suggested changes Nov 4, 2019

View reviewed changes

Stypox mentioned this pull request Dec 3, 2019

Suggestion on playlists and general usability #2805

Closed

Stypox mentioned this pull request Jan 22, 2020

Download all videos by playlist #2978

Closed

3 tasks

B0pol mentioned this pull request Apr 14, 2020

download playlist feature? #3424

Closed

Pokemetti mentioned this pull request Sep 9, 2020

Download playlist #4197

Closed

This was linked to issues Nov 8, 2020

[Feature request] - download all from background player #2303

Open

Download all/multiple items at once in channel/playlist/feed #2380

Closed

opusforlife2 mentioned this pull request Nov 22, 2020

Playlist Download and playback #4964

Closed

3 tasks

Stypox closed this Jan 16, 2021

TeamNewPipe locked as spam and limited conversation to collaborators Aug 18, 2023

Download playlist #2018

Download playlist #2018

Conversation

VishalNehra commented Jan 18, 2019 • edited Loading

theScrabi commented Jan 18, 2019

TobiGr commented Jun 17, 2019

VishalNehra commented Jun 18, 2019

gcandrade10 commented Jun 24, 2019

VishalNehra commented Jun 24, 2019

gcandrade10 commented Jun 24, 2019

VishalNehra commented Sep 5, 2019

strongwinderic left a comment

Choose a reason for hiding this comment

TobiGr commented Sep 24, 2019

kapodamy commented Sep 24, 2019

VishalNehra commented Sep 24, 2019

VishalNehra commented Oct 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TobiGr commented Nov 4, 2019

mauriciocolli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stypox commented Dec 15, 2019 • edited Loading

gcandrade10 commented Jan 20, 2020

VishalNehra commented Jan 23, 2020

realjustinwu commented Apr 11, 2020

ghost commented May 21, 2020

B0pol commented May 21, 2020

Stypox commented May 26, 2020

MD77MD commented Jun 17, 2020

Stypox commented Jan 16, 2021 • edited Loading

0-Kala-0 commented Jul 31, 2021

Hereux commented Aug 18, 2023

opusforlife2 commented Aug 18, 2023

VishalNehra commented Jan 18, 2019 •

edited

Loading

Stypox commented Dec 15, 2019 •

edited

Loading

Stypox commented Jan 16, 2021 •

edited

Loading