StefanLobbenmeier / youtube-dl-gui

A cross-platform GUI for youtube-dl made in Electron and node.js
GNU Affero General Public License v3.0
1.55k stars 62 forks source link

Allow download with duplicate filenames #22

Closed Zenodeon closed 1 year ago

Zenodeon commented 1 year ago

PR Fixes files not being saved when a file with the same name exists in the download folder.

It is fixed by creating a new temp folder (named by the unique identifier) in the download folder for each video.

Then all the embedding and processing are done in this folder and once done, the final file (be it mp4, opus, or whatever the user has requested) will be moved out of the temp folder.

While moving it out, it checks for any pre-existing file with the same name, and if it exists, it adds "(n)" to the file name where n represents the duplicate file name count.

After Moving the processed file, the temp folder is forced Deleted.

StefanLobbenmeier commented 1 year ago

Before we merge this, I want to check if yt-dlp can do this for us.

This is the output when you download a video for the second time:

[youtube] Extracting URL: https://www.youtube.com/watch?v=7vZ020mPm3k
[youtube] 7vZ020mPm3k: Downloading webpage
[youtube] 7vZ020mPm3k: Downloading android player API JSON
[info] 7vZ020mPm3k: Downloading 1 format(s): 597+140
[download] C:\Users\Stefan\Downloads\  \DAEGHO - ENTER THE FUEGO-(144p12).mp4 has already been downloaded
[Metadata] Adding metadata to "C:\Users\Stefan\Downloads\  \DAEGHO - ENTER THE FUEGO-(144p12).mp4"
Download finished
StefanLobbenmeier commented 1 year ago

Checked through yt-dlp source code, and I cannot see an additional condition to not skip the download. So you are right, we will have to implement it ourselves

StefanLobbenmeier commented 1 year ago

Will also test this on my PC now

StefanLobbenmeier commented 1 year ago

Local testing worked just fine. The biggest issue I have with this is that this behaviour might not be wanted by everyone.

For example it breaks resumability, since the files are in the temp folder. So we need some kind of toggle to turn this feature off or on. Would you also be able to add that?

Zenodeon commented 1 year ago

I like to keep this as a core feature as we can't control video names and it can be the same sometimes. Which won't be saved and the user might not notice even.

If Resumability is the only issue, I will take a look at that feature and make it compatible with the temp folder or else i will go with your plan of making it as an option.

Zenodeon commented 1 year ago

What is the Resumability feature btw? I thought it was being able to pause the download and start the download from where it stopped.

StefanLobbenmeier commented 1 year ago

Currently you can start a download, close the app, reopen it and download the file again. Yt-dlp will see that it has partially downloaded the file and resume where it left off. This is now broken, because we put the files in a new temp folder, invisible to yt-dlp

i also consider it a feature that downloading a file a second time instantly completes, because the file is already present.

In your case, with different videos having the same filename one might also consider it a feature that only one of them is kept, they are the same song after all. If a user does not want it, right now they can include the video id in the output template to make sure different video ids are saved to separate files. That would also be the easiest solution for your use case: %(id)s

If you really want to have enumeration when the file already exists, i would propose to add a special template in the output template. For example if the output template contains %(dup_index)s, we can do the logic checking if the file already exists and add a number to the file name. Also saves us from building any additional ui, and we do not affect users that do not want the feature

Zenodeon commented 1 year ago

yea that's fair, I will add an option below the "Keep unmerged files" option for this feature.

Zenodeon commented 1 year ago

Resumability Works with the folder system.

While Avoid Failing To Save Duplicate File Name Option won't but it will never have problems with a duplicate file name.

Zenodeon commented 1 year ago

if you can help add a note below that option stating that Resumability function won't work while Avoid Failing To Save Duplicate File Name Option is active will be great.

StefanLobbenmeier commented 1 year ago

if you can help add a note below that option stating that Resumability function won't work while Avoid Failing To Save Duplicate File Name Option is active will be great.

since we have this as an option now, I think we are ok without it. Otherwise I will just add a tooltip to it with the details

StefanLobbenmeier commented 1 year ago

I am not allowed to edit your branch, so I also created a branch in this repo

Edit: hmm no via Github user interface I am allowed to edit, strange

Zenodeon commented 1 year ago

Please check commit 7a5adc3

Isolating my feature from the standard/core system.

while at it, found a new bug, will create a new issue.

Zenodeon commented 1 year ago

nvm, will do it here as it is relevant to my feature only.

Screenshot_2658

Any idea why [Merger] or [Metadata] don't report the file name correctly?

Only happens when the video title has char like "/", so far from testing.

Zenodeon commented 1 year ago

Here is the video if you need to.

Excuse the video content. https://www.youtube.com/watch?v=PYeZX7X9tB0

StefanLobbenmeier commented 1 year ago

nvm, will do it here as it is relevant to my feature only.

Screenshot_2658

Any idea why [Merger] or [Metadata] don't report the file name correctly?

Only happens when the video title has char like "/", so far from testing.

the \u29f8 is unicode encoded it seems: https://unicode-table.com/en/29F8/

Also makes sense if you think about it, the / is not a valid character for files, since it marks a new folder instead. But I suspect it might be blocked by youtube, so the video uploader used Big Solidus instead of the normal Solidus https://unicode-table.com/en/002F/

Zenodeon commented 1 year ago

Yea ik about the new folder part, but it works fine for filename with extention, I think. And yt-dlp saves the file with that name, even reports it right while downloading but when it embeds the metadata or mergers, it reports the file name wrong.

Zenodeon commented 1 year ago

I can deal with this issue with a funky code, but checking if it can be fixed from yt-dlp.

StefanLobbenmeier commented 1 year ago

There are some code style issues, but I will fix them in a separate branch. For now, it looks good to me

StefanLobbenmeier commented 1 year ago

Reminder for the future: yt-dlp actually has this built-in and its called %(autonumber)s

actually not quite, it will always output 1 when downloading a single video