Kenshin9977 / video-dl

A GUI for yt-dlp that aims to simplify its usage.
MIT License
32 stars 7 forks source link

Feature/youtube playlist download #10

Closed KnifeOnlyI closed 2 years ago

KnifeOnlyI commented 2 years ago

The PR brings fixes :

Fixes :

Fix an error with "post_process_dl for not AudioOnly" for playlist download Move the downloaded files into the user specified output directory

(Because I deleted by accident the branch on my fork, I recreate this pull request...)

Edited according to maintainer remarks :

KnifeOnlyI commented 2 years ago

For the GUI playlist tick I try this :

image

But now, want we need to do ? If yt_dlp detect a playlist in the url, he download all the videos. So we need to treat all videos in post_download (or just the first ?)

This is why i'm think the tick is not necessary for now without other options to parameterize the playlist download.

Kenshin9977 commented 2 years ago

The way I see it, when the box is ticked AND the item is a playlist we treat every video. Otherwise we only treat the first one. That way if a playlist item is in the Link field but we only want to download the video in the link, it will work as expected and won't download the whole playlist. A possible improvement is to have an Index field where we can put the indexes of the videos' playlist we want to download which will use the playlist-items option of yt-dlp:

 --playlist-items ITEM_SPEC          Playlist video items to download. Specify
                                     indices of the videos in the playlist
                                     separated by commas like: "--playlist-items
                                     1,2,5,8" if you want to download videos
                                     indexed 1, 2, 5, 8 in the playlist. You can
                                     specify range: "--playlist-items
                                     1-3,7,10-13", it will download the videos
                                     at index 1, 2, 3, 7, 10, 11, 12 and 13

The field would be greyed out when the playlist box is unticked and enabled when the box is ticked. We should parse the input when the download button is clicked to check if it is valid if the playlist box is ticked. If it's invalid, we display an error message to signal it. If the item isn't a playlist, we should just ignore the field and treat the download as a single video. On another note, I think we'll also have to slightly modify the way the download progress bar is displayed

KnifeOnlyI commented 2 years ago

I push a new commit. It add the expected behavior for "is playlist" tick and playlist download. The commit doesn't add the improvement yet. I think the feature can be used without improvement for now ?

"is playlist" tick behavior :

image

Here examples of links :

A possible improvement :

Kenshin9977 commented 2 years ago

How does the download progress bar bevahes with a playlist?

KnifeOnlyI commented 2 years ago

Actually the program open a new progress for every downloads (when the first is finished, the progress is closed and the second is opened).

I have to do some research to find how to update the existing behavior.

KnifeOnlyI commented 2 years ago

For the progress bar I have a suggestion (I doesn't yet check if it is possible with the used libraries). We can have a design like this :

image

We can have a one progress bar (0 - 100) for every videos in the playlist. In this image, the sixth video is downloaded at 70% (and the first five has been downloaded at 100% each one)

Kenshin9977 commented 2 years ago

The design is perfect 👍

KnifeOnlyI commented 2 years ago

Okay, thanks.

But you must don't merge this pull requests. I find a bug. When you try download the playlist without tick, he download all videos (but only the first is put in the user specified output directory, the rest of them is in the same directory of the executable...)

I'm looking for fix this.