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

[FEATURE] Changing Drifty GUI to a JavaFX interface #231

Closed
EasyG0ing1 opened this issue Jul 29, 2023 · 15 comments · Fixed by #233
Closed

[FEATURE] Changing Drifty GUI to a JavaFX interface #231

EasyG0ing1 opened this issue Jul 29, 2023 · 15 comments · Fixed by #233
Assignees
Labels
App 💻 Issues/Pull Requests which update Drifty Application Code feature ✨ New feature request or addition
Milestone

Comments

@EasyG0ing1
Copy link
Contributor

EasyG0ing1 commented Jul 29, 2023

Description

We talked briefly about the Drifty GUI interface in #225 so I wanted to open this one up specifically for this PR that I WANT to submit, but I've run into some issues that have been difficult to figure out. I've invested way too many hours into this to just toss it out the window so I'm hoping some collaboration will help me get this interface completed.

I have a lot of questions, so I'll try to stick to what's more relevant...

I'm new to yt-dlp or any of the forks from the original. It has enormous capability, but of course, leveraging all of that capability within a wrapper program can be very overwhelming. I noticed in the code, that you seem to have broken down capabilities offered to your users in generally three "categories" which are Youtube downloads, Instagram downloads, and "everything else"...

I've been focusing my efforts on just the youtube feature of the code. But when I thought it would be simple enough to just use the same method calls that you use in the Swing code which seems straightforward, but there are functions even in the Swing code that aren't quite working right, such as filename detection, etc. You also seem to rely heavily on the MessageBroker class for passing information around in the program which I think might also be connected to logging somehow. I haven't studied that area of the code in any depth because it looks a little intimidating at first glance.

I was able to get filename detection working using these command switches in yt-dlp

--write-info-json --skip-download -P /path urlLink

That writes a very thorough JSON file that I then use to grab the filename. And as it turns out, when your link is a list of youtube videos, it creates one JSON file for each video in the list which also includes the URL for each file, making it easy to create batch jobs from links like that.

Could you help me understand the messaging framework a little better? Whats the intent there, how should it be utilized etc.? Also what the primary goal has been for the project given the abilities of yt-dlp.... has your scope just been youtube and Instagram mainly?

Additional information

No response

@EasyG0ing1 EasyG0ing1 added the Other issue Issues other than feature/bug label Jul 29, 2023
@github-actions
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 feature ✨ New feature request or addition App 💻 Issues/Pull Requests which update Drifty Application Code and removed Other issue Issues other than feature/bug labels Jul 29, 2023
@SaptarshiSarkar12
Copy link
Owner

Yeah sure @EasyG0ing1!

Why MessageBroker class is required

The MessageBroker class is used to send messages from the Backend classes (Like FileDownloader, Drifty, etc.) to the frontend classes (like Drifty_CLI and Drifty_GUI). Now, the Drifty CLI has the console/terminal as the default output stream, but, the Drifty GUI has multiple output areas (Like text areas in the swing GUI interface). So, to reduce repetitions, the message broker helps to pass the message to the required areas along with logging them in the log file and setting the respective colors for INFO, WARN and ERROR message categories.

A look into the Code

For Drifty CLI, the terminal is the output area. So, the System.out stream is passed in the consoleOutput field in the below code. The applicationType is CLI.

public MessageBroker(String applicationType, PrintStream consoleOutput){
        appType = applicationType;
        output = consoleOutput;
        logger = Logger.getInstance("CLI");
    }

For Drifty GUI, there are different text areas for the messages to be shown based on their category (Like about the download process, or Link Validation process, or FileName related messages, etc.), so, the object of the JLabel classes are passed to this constructor (see the below code). The applicationType is GUI.

public MessageBroker(String applicationType, JLabel link, JLabel dir, JLabel download, JLabel fileName){
        appType = applicationType;
        this.link = link;
        this.dir = dir;
        this.download = download;
        this.fileName = fileName;
        logger = Logger.getInstance("GUI");
    }

How does the sendMessage method works

The sendMessage method, at first, checks if the application from which the message is coming is CLI or GUI (It gets the info from the object variable applicationType set at the object creation time). Then, it checks for the category of the message (Like only log [for only logging the message], download [for messages regarding downloading the file], link [for link validation related messages], directory [for directory validation related messages], filename [for fileName related messages]).
If the application type is GUI (When Drifty GUI initiates the message Broker class), then it checks for the message type (Like ERROR, WARN, and INFO) and sets the color of the message accordingly.

public void sendMessage(String message, String messageType, String messageCategory){
        if (appType.equals("CLI")){
            if (!messageCategory.equalsIgnoreCase("only log")) {
                output.println(message);
            }
            logger.log(messageType, message);
        } else if (appType.equals("GUI")){
            Color color = Color.BLACK;
            switch (messageType) {
                case DriftyConstants.LOGGER_INFO -> color = Color.GREEN;
                case DriftyConstants.LOGGER_ERROR -> color = Color.RED;
                case DriftyConstants.LOGGER_WARN -> color = Color.YELLOW;
                default -> logger.log(DriftyConstants.LOGGER_ERROR, "Invalid message type provided to message broker!");
            }
            switch (messageCategory) {
                case "link" -> {
                    link.setText(message);
                    link.setForeground(color);
                    logger.log(messageType, message);
                }
                case "directory" -> {
                    dir.setText(message);
                    dir.setForeground(color);
                    logger.log(messageType, message);
                }
                case "download" -> {
                    download.setText(message);
                    download.setForeground(color);
                    logger.log(messageType, message);
                }
                case "Filename" -> {
                    fileName.setText(message);
                    fileName.setForeground(color);
                    logger.log(messageType, message);
                }
                case "only log" -> logger.log(messageType, message);
                default ->
                        logger.log(DriftyConstants.LOGGER_ERROR, "Invalid message category provided to message broker!");
            }
        } else {
            logger.log(DriftyConstants.LOGGER_ERROR, "Invalid application type provided to message broker!");
        }
    }

@SaptarshiSarkar12 SaptarshiSarkar12 changed the title JavaFX Interface Discussion [FEATURE] Changing Drifty GUI to a JavaFX interface Jul 29, 2023
@SaptarshiSarkar12
Copy link
Owner

@EasyG0ing1 yt-dlp provides support for downloading YouTube and Instagram videos, so, it has been kept as a part of the Drifty program.

@EasyG0ing1
Copy link
Contributor Author

When I replaced Swing objects with FX objects, I did see the hooks into the message class for the AWT objects and I provided the StringProperty of the Label class for that purpose.

I also thought it a bit strange that every time a download happens, it checks for an update. I used the Preferences class to store (among other things) the current system time stamp so if the copy method finds that the file already exists in the destination, it doesn't re-copy it and the timestamp gets checked when the code runs to do the update task and If it has not yet been 24 hours, then the update code doesn't run. So in effect, it only runs once every day.

How much liberty do I have to deviate from the code in the download class specifically? Would you be OK relegating that code to the CLI and using a different method for the GUI? Or maybe even a different class?

When I was trying to get the CLI working at all on MacOS, which, by the way, the code as it is now in the repository doesn't work on MacOS "out of the box" ... first issue is the command wont copy out of the resources folder but then kept erroring out when I tried a download. I had to change this:

"-f", "[height<=" + height + "][width<=" + width + "]"

to this

 "-f", "mp4"

To get it to work which begs the question ... does it work in Windows and Linux? When I tried running the command in BASH using that -f option with the resolution dimensions it just threw an error... those really puzzle me cause I don't see anything in the yt-dlp documentation about using that nomenclature for that switch.

This code is puzzling too ...

        if (outputFileName.equals("%(title)s.%(ext)s")) {
            fileDownloadMessagePart = "the YouTube Video";
        }
        else {
            fileDownloadMessagePart = outputFileName;
        }

When would the outputFileName ever be equal to %(title)s.%(ext)s ?

@SaptarshiSarkar12
Copy link
Owner

When I replaced Swing objects with FX objects, I did see the hooks into the message class for the AWT objects and I provided the StringProperty of the Label class for that purpose.
I also thought it a bit strange that every time a download happens, it checks for an update. I used the Preferences class to store (among other things) the current system time stamp so if the copy method finds that the file already exists in the destination, it doesn't re-copy it and the timestamp gets checked when the code runs to do the update task and If it has not yet been 24 hours, then the update code doesn't run. So in effect, it only runs once every day.

Yeah @EasyG0ing1, it would be great if Drifty checks for yt-dlp update once in a day.

How much liberty do I have to deviate from the code in the download class specifically? Would you be OK relegating that code to the CLI and using a different method for the GUI? Or maybe even a different class?

Let me know how you are going to change the download class, and how would the GUI handle the download process. I need to think on this based on your ideas 💡.

When I was trying to get the CLI working at all on MacOS, which, by the way, the code as it is now in the repository doesn't work on MacOS "out of the box" ... first issue is the command wont copy out of the resources folder but then kept erroring out when I tried a download. I had to change this:

"-f", "[height<=" + height + "][width<=" + width + "]"

to this

"-f", "mp4"

To get it to work which begs the question ... does it work in Windows and Linux? When I tried running the command in BASH using that -f option with the resolution dimensions it just threw an error... those really puzzle me cause I don't see anything in the yt-dlp documentation about using that nomenclature for that switch.

Please remove the -f option from the commands passed to yt-dlp as it is no more required. Let me know if -f option has any use cases for MacOS. Also, for windows and linux OS, yt-dlp runs well with / without -f "mp4".

This code is puzzling too ...

if (outputFileName.equals("%(title)s.%(ext)s")) {
    fileDownloadMessagePart = "the YouTube Video";
}
else {
   fileDownloadMessagePart = outputFileName;
}

When would the outputFileName ever be equal to %(title)s.%(ext)s ?

@EasyG0ing1 The outputFileName will be equal to this yt-dlp specific regex pattern %(title)s.%(ext)s when the user wants to keep the video title as the file name.
If you are successful in extracting the name of the YouTube / Instagram video, then you need to replace "the YouTube Video" with the video title.

@SaptarshiSarkar12
Copy link
Owner

@EasyG0ing1 Have you understood? Let me know if you have faced any errors.

@EasyG0ing1
Copy link
Contributor Author

EasyG0ing1 commented Jul 29, 2023

@SaptarshiSarkar12 Yes, I understand ... with the -f option removed, this might make things simpler. The other problem Ive been seeing is that yt-dlp does not run the same in different Process classes. For example, in my efforts to capture the download progress, I tried using the library called jproc and what is interesting about that library is that it works fine when I run yt-dlp for name extraction but for actually downloading a video, it just hangs and will never download the video. It's as if yt-dlp switches to another process or program between resolving the URL and actually downloading the file where ProcessBuilder seems to handle the transition, but it loses connection to the output from the command. But with that whole program being written in Python, this doesn't surprise me - though I would think they could write it somehow so that it remains in a single thread or process but I don't know enough about it or Python to find out if my assumptions are correct.

I've never cared much for Python because it has always felt like a scripting language to me that has evolved into what is now classified as a standard oop language - yet it still lacks real structure and the code is hard to read because of that lack of structure. I've used it for different things in the past but I've never dove deep into Python.

I also figured out my earlier question about the filename string being assigned from this line:

outputFileName = Objects.requireNonNullElse(fileName, "%(title)s.%(ext)s");

I've only used requireNonNull in a different context but I see now that the command tests the filename object for null value then assigned it a value if it is null.

@EasyG0ing1
Copy link
Contributor Author

EasyG0ing1 commented Jul 29, 2023

@SaptarshiSarkar12 Talk to me about the class: Backend.ProgressBarThread - I haven't even looked at it since we can't get download progress from yt-dlp in the first place. Was this an attempt to gain that functionality for the GUI? Can I remove it from the program?

@EasyG0ing1
Copy link
Contributor Author

@SaptarshiSarkar12 - I wanted to mention this before I forget but the whole reason how I found this project in the first place was from an issue you opened on the JavaPackager project ... which I've been using for a while to package my projects, but after studying the POM file in this project, I learned a lot about how to use JavaPackager that I didn't previously know. Like how you can use dot notation in the XML references when referring to a file in a folder instead of slashes ... and also how to create Windows and Linux executables without needing to package the project in those operating systems ... that is a HUGE time saver!

@SaptarshiSarkar12
Copy link
Owner

Yeah @EasyG0ing1, I have also tried to get yt-dlp progress but all in vain. It does not work.
A better idea (if can be implemented) is that creating a console area in the GUI, where the yt-dlp will redirect its output.

@SaptarshiSarkar12
Copy link
Owner

@SaptarshiSarkar12 Talk to me about the class: Backend.ProgressBarThread - I haven't even looked at it since we can't get download progress from yt-dlp in the first place. Was this an attempt to gain that functionality for the GUI? Can I remove it from the program?

No, the ProgressBar class is used to generate a progress bar for downloading files from sources other than YouTube and Instagram. Removing it would cause problem in Drifty CLI.

@SaptarshiSarkar12
Copy link
Owner

@SaptarshiSarkar12 - I wanted to mention this before I forget but the whole reason how I found this project in the first place was from an issue you opened on the JavaPackager project ... which I've been using for a while to package my projects, but after studying the POM file in this project, I learned a lot about how to use JavaPackager that I didn't previously know. Like how you can use dot notation in the XML references when referring to a file in a folder instead of slashes ... and also how to create Windows and Linux executables without needing to package the project in those operating systems ... that is a HUGE time saver!

@EasyG0ing1 Thank you for checking out the project and bringing such good ideas! I appreciate your help and concern 😀!

@SaptarshiSarkar12
Copy link
Owner

SaptarshiSarkar12 commented Jul 30, 2023

@EasyG0ing1 I have successfully got the process ID of yt-dlp but I don't know how to get the progress using the process ID.
I have used the below code just after starting the process :

System.out.println(yt_dlp.pid());

and it prints the PID.

@EasyG0ing1
Copy link
Contributor Author

@SaptarshiSarkar12 In case you havent seen this yet on Discord

@SaptarshiSarkar12
Copy link
Owner

@SaptarshiSarkar12 In case you havent seen this yet on Discord

Yeah @EasyG0ing1 , I have seen that. It's pretty awesome UI. Great Work 👏 ! Please share a video for batch download also.

@SaptarshiSarkar12 SaptarshiSarkar12 added this to the Drifty v2.0.0 milestone Aug 10, 2023
SaptarshiSarkar12 added a commit that referenced this issue Sep 16, 2023
## Fixes issue
<!-- If your PR fixes an open issue, use `Fixes #ISSUE_NUMBER` to link
your PR with the issue. -->
Fixes #231 

## Changes proposed
<!-- List all the proposed changes in your PR -->

## Check List (Check all the applicable boxes) 
- [x] My code follows the code style of this project.
- [ ] My change requires changes to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] This PR does not contain plagiarized content.
- [x] The title of my pull request is a short description of the
requested changes.

## Screenshots
<!-- Add all the screenshots which support your changes -->

## Note to reviewers
The discussions about this PR has been continued in our [Discord
Server](https://discord.gg/DeT4jXPfkG).

--------
Co-authored-by: Saptarshi Sarkar <saptarshi.programmer@gmail.com>
@SaptarshiSarkar12 SaptarshiSarkar12 modified the milestone: Drifty v2.0.0 Sep 23, 2023
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 feature ✨ New feature request or addition
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants