lucabtz / sync-tube.py

Sync YouTube playlists to your disc using youtube-dl
MIT License
11 stars 4 forks source link

--thumbnail and --quality arguments have no effect on outputted files #60

Closed Granshmeyr closed 3 years ago

Granshmeyr commented 3 years ago

I'm running this script from the command prompt on Windows 10 with Python 3.9.5 (virtual environment or not), latest pip, and latest ffmpeg-git binaries. The recommended packages are installed, but the youtube-dl package is manually updated to the latest version to fix errors.

When entering the command "C:\sync-tube.py" --playlist "PLcXVPIDduMaFtJ6FoRur2MrCKa8_x9wkK" --dest "C:\Music" --thumbnail --quality XXX, the outputted files all have no embedded thumbnails and the mp3 bitrate is always 128 kbps. Which is identical behavior as when the arguments are not entered at all.

It's not that big of a deal, but the script seems to be grabbing the webm container most of the time so it's a shame to not preserve the high opus bitrate as I've heard degradation from not encoding to 192 kbps mp3 in the past.

I've seen alternatives like TubeSync that might provide a more streamlined experience but this script is otherwise perfect and simple.

lucabtz commented 3 years ago

Hello thanks for your feedback and sorry for reaching out so late to you. The thumbnail feature was added by @Curtisdhi and I refactored his code to my likings maybe breaking it. I don't have much free time atm and I'm on a new system so I still dont have all of my development tools installed. Maybe if you could check @Curtisdhi commit with git checkout bdf5183to test if that is working. I merged it without testing so maybe the PR was broken by some youtube-dl update. Probably even setting up a CI for this kinda things would help

Curtisdhi commented 3 years ago

I tested the code from the master branch. I had to update youtubedl to the latest (2021.6.6). Thumbnails and bitrate seem to be working fine. Tested from Ubuntu 21.04

Edit: So i tested the files in Windows, and for some reason both Groove and Media Player doesn't show the coverart... but it's there in VLC.

lucabtz commented 3 years ago

Thanks @Curtisdhi for your tests. I updated the requirements to match the newest youtube-dl version but as stated in issue #3 it would be cool if the script automatically checked for updates of youtube-dl since it gets updates very often.

Also I might remove the version of youtube-dl in the requirements so it just gets the newer

lucabtz commented 3 years ago

I'm very busy with university atm. Can you try using VLC media player to see if that works for you @Granshmeyr ? It might even be a youtube-dl bug at this point

Granshmeyr commented 3 years ago

This procedure was performed on a different computer. I'm on Windows 10 with Python 3.8.8 (not virtual environment), latest pip, and ffmpeg version 4.2.3.

Firstly, there's a dependency conflict currently (I just went with 0.12.2): ERROR: Cannot install python-Levenshtein==0.12.0 and python-Levenshtein==0.12.2 because these package versions have conflicting dependencies.

This is the command I ran directly after requirements.txt installation (no manual youtube-dl update): sync-tube.py --playlist PLNd86KKX-_mGb2b81u9Y0u11kYLudmE6N --dest "C:\Users\Grindle\Documents\Downloads\test" --thumbnail --quality 320

Here is one of the downloaded files Dream lantern.mp3 open in the latest VLC, and Mp3tag: https://i.imgur.com/GGzDFb0.png There is no video track available in VLC, nor does Mp3tag display an embedded thumbnail.

Here is a waveform analysis in Spectro of Dream lantern.mp3 and Dream lantern (manual download).mp3, the latter of which I manually downloaded in the highest bitrate .webm format using the latest youtube-dl and exported as constant 320kbps MP3 using Audacity: https://i.imgur.com/su7aIm9.png As you can see, the average bitrate of the script-downloaded file is 128kbps, which are the same results I was getting before. This is detrimental to audio fidelity because there is a severe loss of detail in the upper frequencies (particularly in MPoop3 format), which is shown graphically in the screenshot.

In regards to your previous comment, I sadly have no idea how to do what you recommended.

FYI: I would also like to mention that I ran this script for some period of time on a Raspberry Pi 3 Model B and the arguments still had no effect.

Curtisdhi commented 3 years ago

This is very interesting. The variable handling in python is behaving different on Windows and Linux. On Windows, the command options aren't being applied to YOUTUBE_DL_OPTIONS

https://i.imgur.com/Tn5UJzp.png

I'm not familiar enough with python to resolve the issue. But I did test manually adding the options to the initialization of YOUTUBE_DL_OPTIONS, it does work.

Tested with python 3.8.8 and 3.9.6

Edit:

i found a fix, I think I discovered a pretty big bug with Python lol

apparently you have to declare a variable as global in a local function. But, it totally working fine in Ubuntu.

lucabtz commented 3 years ago

No it's not behaving differently. Indeed in python modifying a global variable is allowed only using the global keyword. This is something I had forgotten since usually my globals are just constants. YOUTUBE_DL_OPTIONS used to be a constant, but it changed with @Curtisdhi PR and so we fucked up. Probably just redesigning the code is the best solution in this case

I will also setup a workflow to automatically test the dependencies and probably I will just remove the youtube-dl version from the requirements so it always gets the newest. I never new this software was actually used by people and in the begining it was just very bare bones for my personal needs but indeed having people interested like this is very nice. Thank you

lucabtz commented 3 years ago

Can you test if its working now @Granshmeyr ? This needs to be redone in a better way but for the moment I will just keep this fix if it works. Making a class YoutubeDownloaderPool that holds the options would be a better way than using globals

Granshmeyr commented 3 years ago

I've re-downloaded the entire repository once again and ran the same command as before: sync-tube.py --playlist PLNd86KKX-_mGb2b81u9Y0u11kYLudmE6N --dest "C:\Users\Grindle\Documents\Downloads\test" --thumbnail --quality 320

Alas, the results are the same: https://i.imgur.com/PMNO9n9.png

This is unrelated but I would like to suggest appending a Windows tip to the "Installation" section of README.md. I personally don't even know what .env is and I don't think it applies to Windows. Just a short explanation to manually create the YOUTUBE_KEY variable in Environment Variables should help some people who want to use this script. Especially because this is probably the easiest and simplest script of its kind that can run on Windows and actually shows up on Google.

Curtisdhi commented 3 years ago

The fix is not in the master. But currently sitting in a pull request. You can go to my repo and test it.

And yes, you can use the .env in windows. Its just a plain text file that is used for local directory environment variables.

Granshmeyr commented 3 years ago

Oh, so I downloaded your repository from https://github.com/Curtisdhi/sync-tube.py and ran: sync-tube.py --playlist PLNd86KKX-_mGb2b81u9Y0u11kYLudmE6N --dest "C:\Users\Grindle\Documents\Downloads\test" --thumbnail --quality 320

And the results are good: https://i.imgur.com/L2NI7k5.png

I also checked the other files. Thanks a lot for helping me out sirs.

lucabtz commented 3 years ago

Yeah my fix was definitely wrong but i see why @Curtisdhi s one is working. The issue is probably related to multiprocessing that's why changes are not seen bc in different processes something happens and the global variable is reset i guess. I will just merge the pr since it's working.

Regarding the instructions indeed they could be better. Tbh i thought of removing the API key completely but i haven't thought of a good way to do it yet

lucabtz commented 3 years ago

@Granshmeyr @Curtisdhi we can move to another issue if you want to discuss what some features to make the program more user friendly would be. I never thought people would start using this and it was mostly meant for people with technical expertise, but indeed making it simple for everybody would be a nice thing.

For ditching the yt api key i have thought in the past of making a dummy google account and just leaking the key, but there is a monthly limit to the api requests one can make if i recall. A solution would be making our own api and requesting info to that one, adding new yt keys when needed. This would allow to track what people download though, which could be good to understand how much the software is used but somebody may argue for they privacy.

An alternative is just parsing the html and reading the titles from there but there would probably not be very efficient since parsing html is kinda expensive

lucabtz commented 3 years ago

Apparently it can be done using ydl https://github.com/ytdl-org/youtube-dl/issues/6945

lucabtz commented 3 years ago

Okay now we have that. The instructions needs to be changed as now you need to pass the while playlist url as parameter

lucabtz commented 3 years ago

And no more API key