Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] YouTube video link with playlist parameter is mistaken as playlist link #601

Open
SaptarshiSarkar12 opened this issue Aug 15, 2024 · 67 comments · May be fixed by #630
Open

[BUG] YouTube video link with playlist parameter is mistaken as playlist link #601

SaptarshiSarkar12 opened this issue Aug 15, 2024 · 67 comments · May be fixed by #630
Assignees
Labels
App 💻 Issues/Pull Requests which update Drifty Application Code bug 🐛 Issues/Pull Requests reporting/fixing a bug EddieHub:good-first-issue Label for good-first-issue finder made by EddieHub Community good first issue Good for newcomers hacktoberfest Issues/Pull Requests for Hacktoberfest
Milestone

Comments

@SaptarshiSarkar12
Copy link
Owner

SaptarshiSarkar12 commented Aug 15, 2024

Describe the bug

A person clicked on a video from a YouTube playlist and when the link to the video is pasted in Drifty,

  • Drifty GUI tries to fetch the video name indefinitely. It also shows a popup window mentioning how many videos are there in the playlist but, it fails to download any. So, it keeps on detecting filename and computer heats up.
  • Drifty CLI shows an error that it fails to detect the filename and prompts the user to exit.

Steps To Reproduce

  1. Open Drifty GUI application.
  2. Paste a YouTube video link (containing both watch?v= and list= parameters), for example, this link - https://www.youtube.com/watch?v=uy_PEGgUF4U&list=PL0lo9MOBetEFGPccyxyfex8BYF_PQUQWn&index=1
  3. You can see that Drifty GUI tries to fetch the filename and the progress bar moves continuously indefinitely.
  4. Repeat the same in Drifty CLI to see that bug also.

Expected Behavior

Drifty must recognize that the URL is of a YouTube video and not of any playlists.

Screenshots

Drifty CLI

image

Drifty GUI

File.Name.detection.going.on.for.indefinite.time.mp4

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.

String command = Program.get(YT_DLP);
String[] args = new String[]{command, "--flat-playlist", "--skip-download", "-P", dir, link};
ProcessBuilder pb = new ProcessBuilder(args);
pb.redirectErrorStream(true);
Process process = pb.start();
StringJoiner joiner = new StringJoiner(lineFeed);
try {
try (
InputStream inputStream = process.getInputStream();
BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))
)
{
String line;
while ((line = reader.readLine()) != null) {
if (this.isCancelled()) {
break;
}
joiner.add(line);
feedback.setValue(joiner.toString());
}
}

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.

@SaptarshiSarkar12 SaptarshiSarkar12 added bug 🐛 Issues/Pull Requests reporting/fixing a bug App 💻 Issues/Pull Requests which update Drifty Application Code labels Aug 15, 2024
@SaptarshiSarkar12 SaptarshiSarkar12 added this to the Drifty v2.1.0 milestone Aug 15, 2024
@github-project-automation github-project-automation bot moved this to Todo in Drifty Aug 15, 2024
Copy link
Contributor

Hello 👋! Thank you very much for raising an issue 🙌! The maintainers will get back to you soon for discussion over the issue! 🚀

Meanwhile you can also discuss about the project in our Discord Server 😀

@SaptarshiSarkar12 SaptarshiSarkar12 added good first issue Good for newcomers EddieHub:good-first-issue Label for good-first-issue finder made by EddieHub Community labels Aug 15, 2024
@SaptarshiSarkar12 SaptarshiSarkar12 pinned this issue Aug 16, 2024
@SaptarshiSarkar12 SaptarshiSarkar12 added the help wanted Extra attention and support, and contributors are needed label Aug 16, 2024
@Tarunkumarkanakam
Copy link

Tarunkumarkanakam commented Aug 26, 2024

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?

@SaptarshiSarkar12
Copy link
Owner Author

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>

@Tarunkumarkanakam
Copy link

Tarunkumarkanakam commented Aug 26, 2024

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
Copy link

image

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
Copy link

Tarunkumarkanakam commented Aug 26, 2024

image

@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. 😞

@SaptarshiSarkar12
Copy link
Owner Author

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 😆.

@SaptarshiSarkar12
Copy link
Owner Author

image 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?

@SaptarshiSarkar12
Copy link
Owner Author

Sorry, My bad it's my firewall issue. 😞

Yeah, sometimes YouTube might block successive requests but not as harshly as Instagram does 😞.

@Tarunkumarkanakam
Copy link

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

@SaptarshiSarkar12
Copy link
Owner Author

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?

@Tarunkumarkanakam
Copy link

image 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?

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

@Tarunkumarkanakam
Copy link

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.

@SaptarshiSarkar12
Copy link
Owner Author

@Tarunkumarkanakam Can you try running the officially released docker images in wsl if it opens up the window?

@SaptarshiSarkar12
Copy link
Owner Author

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 👍

@Tarunkumarkanakam
Copy link

@Tarunkumarkanakam Can you try running the officially released docker images in wsl if it opens up the window?

Can you guide me

@SaptarshiSarkar12
Copy link
Owner Author

@Tarunkumarkanakam Yeah, sure. Follow the below steps. Ensure you have docker installed in wsl.

  1. Open WSL commandline
  2. Run xhost +
  3. Run docker pull ghcr.io/saptarshisarkar12/drifty-gui:master
  4. Run docker run -e DISPLAY=$DISPLAY --net=host -v /tmp/.X11-unix:/tmp/.X11-unix ghcr.io/saptarshisarkar12/drifty-gui:master

@Tarunkumarkanakam
Copy link

Tarunkumarkanakam commented Aug 26, 2024

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
Copy link

Tarunkumarkanakam commented Aug 26, 2024

4. docker run -e DISPLAY=$DISPLAY --net=host -v /tmp/.X11-unix:/tmp/.X11-unix ghcr.io/saptarshisarkar12/drifty-gui:master

image

The same issue. Please check the attached image

@SaptarshiSarkar12
Copy link
Owner Author

@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 😆.

@SaptarshiSarkar12
Copy link
Owner Author

@Tarunkumarkanakam Please reply

@Tarunkumarkanakam
Copy link

@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.

@Tarunkumarkanakam
Copy link

Tarunkumarkanakam commented Sep 2, 2024

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.

I'll try this today and let you know about the output.

@Tarunkumarkanakam
Copy link

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

@SaptarshiSarkar12
Copy link
Owner Author

I'm extremely sorry for not responding, I caught up in placement work at university.

It's okay 🙃.

The weather in our region is very worse though.

I see. @Tarunkumarkanakam Where do you live?

@SaptarshiSarkar12
Copy link
Owner Author

I'll try this today and let you know about the output.

Great 👍

@SaptarshiSarkar12
Copy link
Owner Author

@Tarunkumarkanakam Do you have IntelliJ installed on your machine? If not, which IDE are you using for development?

@Tarunkumarkanakam
Copy link

@Tarunkumarkanakam Do you have IntelliJ installed on your machine? If not, which IDE are you using for development?

yes, its intellij

@Tarunkumarkanakam
Copy link

I'm extremely sorry for not responding, I caught up in placement work at university.

It's okay 🙃.

The weather in our region is very worse though.

I see. @Tarunkumarkanakam Where do you live?

Vijayawada, Andhra Pradesh

@SaptarshiSarkar12
Copy link
Owner Author

yes, its intellij

It's a great IDE 😄 👍

@SaptarshiSarkar12
Copy link
Owner Author

Vijayawada, Andhra Pradesh

I see. Awesome place ❤️.

@Tarunkumarkanakam
Copy link

mvn javafx:run

Yep it worked, I'll work on it and let you know. If possible can you list out the things that need to be finished

Screenshot 2024-09-02 at 9 02 03 AM

@SaptarshiSarkar12
Copy link
Owner Author

Yep it worked, I'll work on it and let you know.

Awesome 👏.

If possible can you list out the things that need to be finished

So, here's what you need to finish 👇

  • Adding a sanitizeLink() method to remove leading www in youtu.be links. I think you can also move the logic for removing &list= and all such unwanted parameters and return a YouTube link in the form https://youtube.com/watch?v=<videoID> or https://www.youtube.com/playlist?list=<ListID>. Consider checking the LinkType.java class. It will help you to identify easily if it's a YouTube link and process it accordingly in the sanitizeLink() method.
  • Adding support for downloading YouTube playlist (you can refer how Drifty_GUI processes and downloads YouTube Playlists) for Drifty CLI

@Tarunkumarkanakam
Copy link

Tarunkumarkanakam commented Sep 2, 2024

Yep it worked, I'll work on it and let you know.

Awesome 👏.

If possible can you list out the things that need to be finished

So, here's what you need to finish 👇

  • Adding a sanitizeLink() method to remove leading www in youtu.be links. I think you can also move the logic for removing &list= and all such unwanted parameters and return a YouTube link in the form https://youtube.com/watch?v=<videoID> or https://www.youtube.com/playlist?list=<ListID>. Consider checking the LinkType.java class. It will help you to identify easily if it's a YouTube link and process it accordingly in the sanitizeLink() method.

The current changes (the code I sent in previous messages) are in Drifty_CLI.java
I observed that all the link validation is done in Utility.java in the Core Module. The same functions are being used in CLI and GUI. Shall I update the code in Utility.java?

Moreover, the Drifty CLI identifies www.youtu.be URL as an invalid URL, But minor tweaks are needed as it says invalid but still checks for the filename. So the invalid link www.youtu.be is already handled. If you mean to sanitize the www.youtu.be to youtu.be, I think it is not good to do this because the www.youtu.be is already an invalid URL and we are sanitizing it in such a way that it becomes valid. In my opinion, it isn't correct as we are giving support to an invalid URL. Let me know your opinion, If you want me to do that I can

@SaptarshiSarkar12
Copy link
Owner Author

The current changes (the code I sent in previous messages) are in Drifty_CLI.java
I observed that all the link validation is done in Utility.java in the Core Module. The same functions are being used in CLI and GUI. Shall I update the code in Utility.java?

@Tarunkumarkanakam If done, no need to update then. Where have you kept the sanitizeLink() method? Are you calling it from the link validation method?

Moreover, the Drifty CLI identifies www.youtu.be URL as an invalid URL, But minor tweaks are needed as it says invalid but still checks for the filename.

The most important step of URL validation is actually pinging the URL and it fails in this case. So, it is reported as Invalid.

....as it says invalid but still checks for the filename.

Interesting 💡. Can you show me a screenshot?

So the invalid link www.youtu.be is already handled.

Yeah, but the outputs are counter-intuitive, right? You won't process a link which is invalid 😆.

If you mean to sanitize the www.youtu.be to youtu.be, I think it is not good to do this because the www.youtu.be is already an invalid URL and we are sanitizing it in such a way that it becomes valid. In my opinion, it isn't correct as we are giving support to an invalid URL. Let me know your opinion, If you want me to do that I can

Leave this task then. I also don't think people would go too crazy to add leading www making the link a trash 🤣.
@Tarunkumarkanakam But is the link still processed in both CLI and GUI?

@Tarunkumarkanakam
Copy link

Tarunkumarkanakam commented Sep 2, 2024

The current changes (the code I sent in previous messages) are in Drifty_CLI.java
I observed that all the link validation is done in Utility.java in the Core Module. The same functions are being used in CLI and GUI. Shall I update the code in Utility.java?

@Tarunkumarkanakam If done, no need to update then. Where have you kept the sanitizeLink() method? Are you calling it from the link validation method?

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());
    }
}

This is the updated code in Drifty_CLI.java
This handles any type of YouTube link thrown by the user. This is right after the Instagram link handling in main method of Drifty_CLI.java

I'm preparing screenshots and short videos, I'll send once done

@SaptarshiSarkar12
Copy link
Owner Author

This is the updated code in Drifty_CLI.java

@Tarunkumarkanakam Looks good for YouTube videos.

This handles any type of YouTube link thrown by the user.

Can it also handle playlists? Or you will be implementing it later?

@Tarunkumarkanakam
Copy link

This handles any type of YouTube link thrown by the user.

Can it also handle playlists? Or you will be implementing it later?

I'm implementing it. Except that I guess everything works as expected.

@SaptarshiSarkar12
Copy link
Owner Author

I'm implementing it.

Great 👍. Would love to see your work on this ❤️.

Except that I guess everything works as expected.

Yeah, it should I suppose.

@Tarunkumarkanakam Can you share any of your socials (linkedIn preferably or twitter)? I would like to discuss something with you.

@Tarunkumarkanakam
Copy link

@Tarunkumarkanakam Can you share any of your socials (linkedIn preferably or twitter)? I would like to discuss something with you.

We can connect through LinkedIn, https://www.linkedin.com/in/tarun-kanakam

@SaptarshiSarkar12
Copy link
Owner Author

We can connect through LinkedIn, https://www.linkedin.com/in/tarun-kanakam

Invitation sent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App 💻 Issues/Pull Requests which update Drifty Application Code bug 🐛 Issues/Pull Requests reporting/fixing a bug EddieHub:good-first-issue Label for good-first-issue finder made by EddieHub Community good first issue Good for newcomers hacktoberfest Issues/Pull Requests for Hacktoberfest
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

2 participants