Open SaptarshiSarkar12 opened 1 month ago
Meanwhile you can also discuss about the project in our Discord Server š
I want to work on this issue. Is anyone working on this? What is the intended behavior? Does the full playlist need to be downloaded or only the first video of the playlist? As per what I read above I think the application should ignore the playlist urls right?
I want to work on this issue. Is anyone working on this?
@Tarunkumarkanakam Thank you for your interest in the issue. This issue is still looking for contributors. I am assigning this issue to you.
What is the intended behavior? Does the full playlist need to be downloaded or only the first video of the playlist?
Only the video (identified from the ?watch?v=<videoID>
) needs to be downloaded. Not the whole playlist.
As per what I read above I think the application should ignore the playlist urls right?
Yeah, you are right. The YouTube url provided must either have watch?v=<videoID>
or playlist?list=<playlistID>
. Not both. So, you will need to remove either of them based on the following rule:
watch?v=<videoID>
gets precedence over playlist?list=<playlistID>
Well, I am a Windows user. To run in a docker environment I need to execute xhost +local:docker
right. So initially, I need to set up X Server on my machine right?
If yes can you give steps to do that?
Can you help me with this issue. I added DISPLAY variable in my WSL bash, and I also downloaded VcXSrv, Tested it with xclock
working without any issue. But still this error is still there in my console
@SaptarshiSarkar12
When I executed the CLI with code from the master branch using docker-compose run cli
. I am getting the error as in the image attached. Is that an issue that needs to be reported?
EDIT: Sorry, My bad it's my firewall issue. š
Well, I am a Windows user. To run in a docker environment I need to execute
xhost +local:docker
right. So initially, I need to set up X Server on my machine right?If yes can you give steps to do that?
@Tarunkumarkanakam I think yes. I don't have a windows machine and I tried multiple times to install wsl in my Windows VM but it failed each time and finally destructed the whole VM, so, I had to delete the whole setup :cry:. The docs regarding the docker setup for Windows also needs to be more detailed but, this project is lacking in contributors now, I suppose :laughing:.
Can you help me with this issue. I added DISPLAY variable in my WSL bash, and I also downloaded VcXSrv, Tested it with
xclock
working without any issue. But still this error is still there in my console
You might haven't run xhost +local:docker
or xhost +local:
(a more wide access control). Have you run those?
Also, which command did you use while starting this docker build? Docker compose or the latest docker image available?
Sorry, My bad it's my firewall issue. š
Yeah, sometimes YouTube might block successive requests but not as harshly as Instagram does :disappointed:.
Well, I am a Windows user. To run in a docker environment I need to execute
xhost +local:docker
right. So initially, I need to set up X Server on my machine right? If yes can you give steps to do that?@Tarunkumarkanakam I think yes. I don't have a windows machine and I tried multiple times to install wsl in my Windows VM but it failed each time and finally destructed the whole VM, so, I had to delete the whole setup š¢. The docs regarding the docker setup for Windows also needs to be more detailed but, this project is lacking in contributors now, I suppose š.
š Well I'm a Windows, Mac and a Linux user. LoL XD
Well I'm a Windows, Mac and a Linux user. LoL XD
How? So, you might have a mac with both windows and linux VM?
Can you help me with this issue. I added DISPLAY variable in my WSL bash, and I also downloaded VcXSrv, Tested it with
xclock
working without any issue. But still this error is still there in my consoleYou might haven't run
xhost +local:docker
orxhost +local:
(a more wide access control). Have you run those? Also, which command did you use while starting this docker build? Docker compose or the latest docker image available?
I followed as suggested in Readme file.
Additionally I installed vcxsrv and It's active too (confirmed by running xclock), and I ran xhost +local:docker
Well I'm a Windows, Mac and a Linux user. LoL XD
How? So, you might have a mac with both windows and linux VM?
Nah I use my macbook as my daily driver, I have a Windows machine in our office (internship) and in my home which I mostly use for development.
@Tarunkumarkanakam Can you try running the officially released docker images in wsl if it opens up the window?
Nah I use my macbook as my daily driver, I have a Windows machine in our office (internship) and in my home which I mostly use for development.
I see. Great :+1:
@Tarunkumarkanakam Can you try running the officially released docker images in wsl if it opens up the window?
Can you guide me
@Tarunkumarkanakam Yeah, sure. Follow the below steps. Ensure you have docker installed in wsl.
xhost +
docker pull ghcr.io/saptarshisarkar12/drifty-gui:master
docker run -e DISPLAY=$DISPLAY --net=host -v /tmp/.X11-unix:/tmp/.X11-unix ghcr.io/saptarshisarkar12/drifty-gui:master
@SaptarshiSarkar12 As per the issue description, using CLI the filename isn't getting fetched and the application is prompting the user to exit. Well, In my case the file name was found and the video got downloaded too without any changes in the code. Please check the attached image. Let me know if I'm missing anything
4. docker run -e DISPLAY=$DISPLAY --net=host -v /tmp/.X11-unix:/tmp/.X11-unix ghcr.io/saptarshisarkar12/drifty-gui:master
The same issue. Please check the attached image
@SaptarshiSarkar12 As per the issue description, using CLI the filename isn't getting fetched and the application is prompting the user to exit. Well, In my case the file name was found and the video got downloaded too without any changes in the code. Please check the attached image. Let me know if I'm missing anything
@Tarunkumarkanakam The filename is the name of the playlist and not of the video. So, sometimes it shows filename detection failed if yt-dlp reported error else it uses playlist name if available. Hence, it is better to use watch?v=
instead of list=
when both are present to prevent confusion of yt-dlp :laughing:.
The same issue. Please check the attached image
Interesting. It used to work some months back and without even a small change, it stopped working now :cry:. How inconsistent can these systems be :upside_down_face:!
Tell me what xhost
shows when you run in wsl
@Tarunkumarkanakam Can you change the last part of the docker compose file content to this? Replace this in the section where gui volumes are defined.
volumes:
- ${DOWNLOAD_FOLDER:-.}:/root/Downloads
- /run/desktop/mnt/host/wslg/.X11-unix:/tmp/.X11-unix
- /run/desktop/mnt/host/wslg:/mnt/wslg
- ./.drifty:/root/.drifty
Rebuild and retry. Then, let me know if it worked.
@SaptarshiSarkar12 As per the issue description, using CLI the filename isn't getting fetched and the application is prompting the user to exit. Well, In my case the file name was found and the video got downloaded too without any changes in the code. Please check the attached image. Let me know if I'm missing anything
@Tarunkumarkanakam The filename is the name of the playlist and not of the video. So, sometimes it shows filename detection failed if yt-dlp reported error else it uses playlist name if available. Hence, it is better to use
watch?v=
instead oflist=
when both are present to prevent confusion of yt-dlp š.
Got it
Tell me what
xhost
shows when you run in wsl
It is there in the image I attached. The first line
root@DESKTOP-GCCO37B:~# xhost +
access control disabled, clients can connect from any host
Or check this out for additional info
root@DESKTOP-GCCO37B:~# xhost
access control disabled, clients can connect from any host
INET:172.29.208.1 (no nameserver response within 5 seconds)
LOCAL:
INET:0.0.0.0
INET6:ip6-localhost
4. docker run -e DISPLAY=$DISPLAY --net=host -v /tmp/.X11-unix:/tmp/.X11-unix ghcr.io/saptarshisarkar12/drifty-gui:master
š Same issue
Unable to open DISPLAY
Tell me what
xhost
shows when you run in wslIt is there in the image I attached. The first line
root@DESKTOP-GCCO37B:~# xhost + access control disabled, clients can connect from any host
Or check this out for additional info
root@DESKTOP-GCCO37B:~# xhost access control disabled, clients can connect from any host INET:172.29.208.1 (no nameserver response within 5 seconds) LOCAL: INET:0.0.0.0 INET6:ip6-localhost
Okay. So, everything there seems to be alright.
- docker run -e DISPLAY=$DISPLAY --net=host -v /tmp/.X11-unix:/tmp/.X11-unix ghcr.io/saptarshisarkar12/drifty-gui:master
š Same issue
Unable to open DISPLAY
@Tarunkumarkanakam Have you used the modified docker compose content that I posted earlier? What is the outcome?
- docker run -e DISPLAY=$DISPLAY --net=host -v /tmp/.X11-unix:/tmp/.X11-unix ghcr.io/saptarshisarkar12/drifty-gui:master
š Same issue
Unable to open DISPLAY
@Tarunkumarkanakam Have you used the modified docker compose content that I posted earlier? What is the outcome?
Yes, I used it but the outcome is the same, pasted the given data, and rebuilt the whole project.
Okay. Need to see why is that problem occurring. @Tarunkumarkanakam Can you create one issue for this? You need to use Bug report for Application issue template and choose Docker under Environment. We'll deal this in a separate issue :smile:.
@SaptarshiSarkar12 Please check the attached image. Solved the problem for CLI
My approach is when a URL is passed extract the video Id out of it such the the URL will be the simplified URL with only video id (https://www.youtube.com/watch?v={id}
). I used the same Github playlist URL given in the issue description.
Also, check the code. And let me know what tests I need to do on this to ensure everything is correct. This code I wrote in the file Drifty_CLI.java
else {
if (isInstagram(link)) {
link = formatInstagramLink(link);
} else if (isYoutube(link)) {
Map<String, String> queryParams = extractQueryParams(link, "v"); // Wrote a function extractQueryParams in Utility.java, which takes link, varargs of query params and returns a Map of quert params
String videoId = queryParams.get("v");
if (videoId != null) {
try {
URI uri = new URI("https", "www.youtube.com", "/watch", "v=" + videoId, null); // Created a simplified link with only video id param
link = uri.toString();
} catch (URISyntaxException e) {
messageBroker.msgLinkError("Failed to construct YouTube URL with video ID: " + videoId);
}
} else {
messageBroker.msgLinkError("YouTube video ID ('v' parameter) not found in the link.");
}
}
messageBroker.msgFilenameInfo("Retrieving filename from link...");
fileName = findFilenameInLink(link);
if (!Objects.requireNonNull(fileName).isEmpty()) {
verifyJobAndDownload(true);
} else {
messageBroker.msgFilenameError("Failed to retrieve filename from link!");
}
}
If you feel everything is fine I'll commit with proper documentation and create a PR.
@Tarunkumarkanakam I think there's a better solution. There are two types of YouTube links:
youtu.be/<videoID>
youtube.com/watch?v=<videoID>
The first link never includes the playlist id, so, that won't cause any problem ever :smile:. But, the second link might contain a list
parameter. So, we need to add a check for the domain of the url (youtu.be
or youtube.com
).Secondly, we will check if it has playlist
or watch
.
playlist
: then it is a playlist, so nothing to do about thiswatch
: we will need to extract the url upto videoID. We can use String videoId = link.substring(link.indexOf("v=") + 2, link.indexOf("v=") + 13);
because YouTube videoIDs are of 11 characters. You can also use String sanitizedLink = link.split("&")[0];
and I would recommend the second one (using split
) because it would be more reliable as different parameters in a URL are always joined with &
.So, I think creating that function is not needed. Let me know what you think :smiley:.
Moreover, the split("&")
function works even if &
is not present in the link like https://www.youtube.com/watch?v=EG45lEhSURY
. So, you don't need to add any check for the presence of &
in the link.
@Tarunkumarkanakam I think there's a better solution. There are two types of YouTube links:
youtu.be/<videoID>
youtube.com/watch?v=<videoID>
The first link never includes the playlist id, so, that won't cause any problem ever š. But, the second link might contain alist
parameter. So, we need to add a check for the domain of the url (youtu.be
oryoutube.com
).Secondly, we will check if it has
playlist
orwatch
.
playlist
: then it is a playlist, so nothing to do about thiswatch
: we will need to extract the url upto videoID. We can useString videoId = link.substring(link.indexOf("v=") + 2, link.indexOf("v=") + 13);
because YouTube videoIDs are of 11 characters. You can also useString sanitizedLink = link.split("&")[0];
and I would recommend the second one (usingsplit
) because it would be more reliable as different parameters in a URL are always joined with&
.So, I think creating that function is not needed. Let me know what you think š.
Well, I think If we have a simple function that can identify each part would be better and future-proofed (for example, if in the future we require the application to deal with a playlist just use this function to extract the list param or any required param put the param in a URL, it works without any issue).
To address the problem, here is the workaround as follows.
youtu.be/<videoID>
is a link that doesn't require any query param right. So first we will validate the YouTube link, then we will check if the link is a .com domain or a .be domain. If it is a .be we process it for download. Else it would be a link with .com and has watch?v={videoID}
right. From this, we will extract the video ID and create a new URL like youtu.be\{videoID}
to be processed by our backend.youtu.be/<videoID>
youtube.com/watch?v=<videoID>
). It's future-proofed to extract the query params using a library and those params can be used in future functionalityWell I think this would be a better choice since whatever YouTube link we receive we handle it with a unified URL (youtu.be/{videoID}
) format by converting it in the backend, let me know your opinion on this
@Tarunkumarkanakam
Well, I think If we have a simple function that can identify each part would be better and future-proofed (for example, if in the future we require the application to deal with a playlist just use this function to extract the list param or any required param put the param in a URL, it works without any issue).
You are right. Moreover, the current version of drifty doesn't detect duplicate links if they have the tracking parameter different although the ID of video or playlist is same. So, your method would also help in that case.
As per your information
youtu.be/<videoID>
is a link that doesn't require any query param right. So first we will validate the YouTube link, then we will check if the link is a .com domain or a .be domain. If it is a .be we process it for download. Else it would be a link with .com and haswatch?v={videoID}
right. From this, we will extract the video ID and create a new URL likeyoutu.be\{videoID}
to be processed by our backend.
Right :+1:. There should be a /
instead of \
in youtu.be\{videoID}
.
By this we have a unified approach and every video will be handled based on one format under the hood even if the user gives any type of link (youtu.be/
youtube.com/watch?v= ). It's future-proofed to extract the query params using a library and those params can be used in future functionality
Right. Agreed :100:.
Well I think this would be a better choice since whatever YouTube link we receive we handle it with a unified URL (
youtu.be/{videoID}
) format by converting it in the backend
Please don't use youtu.be/{videoID}
. Instead use youtube.com/watch?v={videoID}
or youtube.com/playlist?list={listID}
because you can have both playlists
and videos
under the same domain as youtu.be
doesn't support playlists. What do you think?
@SaptarshiSarkar12 Please check the below code. It is as per our discussions
else if (isYoutube(link)) {
String videoId = null;
try {
URI uri = URI.create(link);
String domain = uri.getHost();
// checking if the domain is youtu.be
if ("youtu.be".equals(domain)) {
String path = uri.getPath();
if (path != null && path.length() > 1) {
videoId = path.substring(1); // removing the leading "/"
}
}
// checking if the domain is youtube.com
else if ("www.youtube.com".equals(domain) || "youtube.com".equals(domain)) {
Map<String, String> queryParams = extractQueryParams(link, "v");
videoId = queryParams.get("v");
}
if (videoId != null) {
// constructing link in youtube.com/watch?v={videoID}
uri = new URI("https", "www.youtube.com", "/watch", "v=" + videoId, null);
link = uri.toString();
} else {
messageBroker.msgLinkError("YouTube video ID not found in the link.");
}
} catch (IllegalArgumentException | URISyntaxException e) {
messageBroker.msgLinkError("Failed to process the YouTube link: " + e.getMessage());
}
}
I think it's pretty straightforward with the if condition and some logic. By the way thanks for the positive response š Let me know what you think and any further changes.
@Tarunkumarkanakam I have some questions on your code.
https://www.youtube.com/playlist?list=PL0lo9MOBetEFGPccyxyfex8BYF_PQUQWn
"youtu.be".equals(domain)
and "www.youtube.com".equals(domain)
seems to be controversial as you considered the presence or absence of www
in youtube.com
domain but not in youtu.be
domain. Why is that so?@Tarunkumarkanakam I have some questions on your code.
- You haven't handled the playlist url like
https://www.youtube.com/playlist?list=PL0lo9MOBetEFGPccyxyfex8BYF_PQUQWn
@SaptarshiSarkar12 You told me that only the video needs to be downloaded not the entire playlist right (if a playlist url contains video id)? Do we need to download the full playlist if the link is of type playlist?
Could you please describe more on this.
"youtu.be".equals(domain)
and"www.youtube.com".equals(domain)
seems to be controversial as you considered the presence or absence ofwww
inyoutube.com
domain but not inyoutu.be
domain. Why is that so?
Well, As per my knowledge www.youtube.com/watch?v={videoID}
is the url that we get in the address bar when we open a youtube video. And the youtu.be
(is a shortened url given by youtube when we want to share a yt video), this won't work with www
since it doesn't come in www
subdomain. You can check by explicitly putting www
in front of a youtu.be
url. Due to that www
doesn't make sense to youtu.be
Let me know what you think and correct me if I'm wrong š
@SaptarshiSarkar12 You told me that only the video needs to be downloaded not the entire playlist right (if a playlist url contains video id)? Do we need to download the full playlist if the link is of type playlist?
Could you please describe more on this.
@Tarunkumarkanakam See, there are three scenarios for a YouTube link (considering www.youtube.com
domain) :point_down:
/playlist?list=
end point and parameter. I don't see you have handled this case in the code line else if ("www.youtube.com".equals(domain) || "youtube.com".equals(domain))
. You should have added a check that it does not contain the keyword playlist
which is unique for direct playlist URLs only.&list=
parameter. You seem to have handled this case pretty well.extractQueryParams()
method. Let me know if you have handled this and in which segment of code.Well, As per my knowledge
www.youtube.com/watch?v={videoID}
is the url that we get in the address bar when we open a youtube video. And theyoutu.be
(is a shortened url given by youtube when we want to share a yt video), this won't work withwww
since it doesn't come inwww
subdomain. You can check by explicitly puttingwww
in front of ayoutu.be
url. Due to thatwww
doesn't make sense toyoutu.be
Yeah, I saw. It shows DNS_PROBE_FINISHED_NXDOMAIN
meaning DNS fails to resolve such name. But I think it would be better to programmatically remove www
if it exists in the link because some users might trick Drifty to crash :rofl:.
@SaptarshiSarkar12 You told me that only the video needs to be downloaded not the entire playlist right (if a playlist url contains video id)? Do we need to download the full playlist if the link is of type playlist? Could you please describe more on this.
@Tarunkumarkanakam See, there are three scenarios for a YouTube link (considering
www.youtube.com
domain) š
- The link is purely a playlist URL and has
/playlist?list=
end point and parameter. I don't see you have handled this case in the code lineelse if ("www.youtube.com".equals(domain) || "youtube.com".equals(domain))
. You should have added a check that it does not contain the keywordplaylist
which is unique for direct playlist URLs only.
yep, got it. But what we need to do with the playlist-only URLs? Do you mean, we need to give the functionality to download the whole playlist (I mean for the future if it is not there?), or warn the user that its a playlist?
- The link points to a video but has a
&list=
parameter. You seem to have handled this case pretty well.
yes
- The link is purely a video URL, and I suppose you might have handled that in
extractQueryParams()
method. Let me know if you have handled this and in which segment of code.
Yep It works, I checked with a normal video URL, a Video from a playlist, and a video with the URL youtu.be
Well, As per my knowledge
www.youtube.com/watch?v={videoID}
is the url that we get in the address bar when we open a youtube video. And theyoutu.be
(is a shortened url given by youtube when we want to share a yt video), this won't work withwww
since it doesn't come inwww
subdomain. You can check by explicitly puttingwww
in front of ayoutu.be
url. Due to thatwww
doesn't make sense toyoutu.be
Yeah, I saw. It shows
DNS_PROBE_FINISHED_NXDOMAIN
meaning DNS fails to resolve such name. But I think it would be better to programmatically removewww
if it exists in the link because some users might trick Drifty to crash š¤£.
Sure, this should be done while validating the link right. While validation if the Application says it is valid and after that, while fetching the filename if it says invalid link due to the presence of www
, that's not good. So, shall I tweak in link validation step?
Yeah, they aren't handling it with www
. Well, in my opinion www
doesn't add any technical benefit to youtube, it defy the purpose of URL shortening lol
yep, got it. But what we need to do with the playlist-only URLs? Do you mean, we need to give the functionality to download the whole playlist (I mean for the future if it is not there?), or warn the user that its a playlist?
@Tarunkumarkanakam So, I checked and found that YouTube playlists are handled only in Drifty GUI and not in CLI as of now. So, do you want to add that functionality? Let me know.
Sure, this should be done while validating the link right. While validation if the Application says it is valid and after that, while fetching the filename if it says invalid link due to the presence of www, that's not good. So, shall I tweak in link validation step?
Yeah, if it has www
with youtu.be
, you're going to remove that word. I think that code needs to be added just before Utility.isLinkValid(link)
is called from both Drifty GUI and CLI.
Here's a fragment of the code for
You must create a new method called sanitizeLink()
in Utility
class of Core
module which will remove the unwanted www
word accordingly.
Yeah, they aren't handling it with www. Well, in my opinion www doesn't add any technical benefit to youtube, it defy the purpose of URL shortening lol
Yeah, right :laughing:.
@Tarunkumarkanakam So, I checked and found that YouTube playlists are handled only in Drifty GUI and not in CLI as of now. So, do you want to add that functionality? Let me know.
Sure I'll give it a try š
Yeah, if it has
www
withyoutu.be
, you're going to remove that word. I think that code needs to be added just beforeUtility.isLinkValid(link)
is called from both Drifty GUI and CLI. Here's a fragment of the code forYou must create a new method called
sanitizeLink()
inUtility
class ofCore
module which will remove the unwantedwww
word accordingly.
Sure I'll update the code and I can test on CLI, but GUI isn't working in my machine right what shall we do on this?
Sure I'll give it a try š
Thank you :heart:.
Sure I'll update the code and I can test on CLI, but GUI isn't working in my machine right what shall we do on this?
@Tarunkumarkanakam Great :+1:. Do you have IntelliJ installed on your machine? You can run the Drifty_GUI.java
class.
Or, you can just run these few commands :point_down:
mvn clean install
cd GUI/
mvn javafx:run
The Drifty GUI window will open up with your changes.
Remember to run mvn clean install
every time you make changes to Core
module. It's an important step to always reflect your latest changes.
Can you help me with this issue. I added DISPLAY variable in my WSL bash, and I also downloaded VcXSrv, Tested it with xclock working without any issue. But still this error is still there in my console
@Tarunkumarkanakam Which version of WSL were you using? It needs to be WSL version 2.
@Tarunkumarkanakam Please reply
@Tarunkumarkanakam Please reply
I'm extremely sorry for not responding, I caught up in placement work at university. The weather in our region is very worse though.
Remember to run
mvn clean install
every time you make changes toCore
module. It's an important step to always reflect your latest changes.
I'll try this today and let you know about the output.
Can you help me with this issue. I added DISPLAY variable in my WSL bash, and I also downloaded VcXSrv, Tested it with xclock working without any issue. But still this error is still there in my console
@Tarunkumarkanakam Which version of WSL were you using? It needs to be WSL version 2.
Yes its WSL 2
Describe the bug
A person clicked on a video from a YouTube playlist and when the link to the video is pasted in Drifty,
Steps To Reproduce
watch?v=
andlist=
parameters), for example, this link - https://www.youtube.com/watch?v=uy_PEGgUF4U&list=PL0lo9MOBetEFGPccyxyfex8BYF_PQUQWn&index=1Expected Behavior
Drifty must recognize that the URL is of a YouTube video and not of any playlists.
Screenshots
Drifty CLI
Drifty GUI
https://github.com/user-attachments/assets/db281ab2-10d2-4a25-811f-da45f78b4cad
Additional information
A regex match to identify if the provided link is a YouTube playlist or a specific YouTube video link, must be added. Yt-dlp fails to identify if a specific video is asked rather than the full playlist as seen when this code fragment is executed. https://github.com/SaptarshiSarkar12/Drifty/blob/3fff0e5e05a4fd871b50129aefff458ac246d59d/GUI/src/main/java/ui/GetFilename.java#L172-L192 A possible and reliable soluyion is to truncate the url upto the specific video id (the resultant url will be of the form
https://www.youtube.com/watch?v=uy_PEGgUF4U
).If anyone wants to work on this issue, please comment below.