omeryusufyagci / fast-music-remover

A C++ based, lightweight music and noise remover for YouTube and other internet media, using DeepFilterNet for audio enhancement.
MIT License
208 stars 22 forks source link

feat: add windows support (MIDWAY) #22

Closed IbrahimHamshari closed 1 month ago

IbrahimHamshari commented 1 month ago

This is just a pull request to see why this error occurs when processing the video on Windows. #18

Error: Failed to process chunk with DeepFilterNet: C:\Users\acer\Desktop\Issues\fast-music-remover\uploads\chunks\chunk_4.wav
Command failed with return code 1:
'MediaProcessor' is not recognized as an internal or external command,
operable program or batch file.
omeryusufyagci commented 1 month ago

Thanks for the start on this! I took a look and was able to reproduce the problem you shared in #18, but it turned out to be a config issue. The MediaProcessor didn't compile for me as is, I had to adjust the CMakeLists to include a fetch for the json library, and handle pthreads for unix only. After these the filtering worked fine.

Then I ran into an issue with the mergeChunks(). It's a Windows-specific problem related to the quotes on the command itself causing it to not get parsed correctly. The same seems to work from the terminal.

I also ended up refactoring the entire core to eliminate strings for file paths, and strictly use std::filesystem::path everywhere.

What I propose to do is to limit the scope of this PR to the work you've done for the ConfigManager, including the corrected config.json and cleaned version of the app.py. Let's merge that and I'll push my refactor along with the CMakeChanges and the CommandBuilder I wanted to add. Then, you can then tackle the remaining issues if you're up for it.

For this to work though, it can't break the Unix/Linux version. So please test there, and only add non-breaking changes. How does this sound?

IbrahimHamshari commented 1 month ago

So for some reason, it works now on both Linux (using docker) and Windows right now. Here are the steps that worked for me, First I installed GCC using MSYS2 with this command:

pacman -S mingw-w64-x86_64-toolchain base-devel

Then installed Cmake using this command :

pacman -S mingw-w64-x86_64-cmake

Then Installed the nlohmann/json.hpp by using this command:

pacman -S mingw-w64-x86_64-nlohmann-json

if the ffmpeg is properly installed in the C:\ffmpeg\bin it should work.

omeryusufyagci commented 1 month ago

So for some reason, it works now on both Linux (using docker) and Windows right now. Here are the steps that worked for me, First I installed GCC using MSYS2 with this command:

pacman -S mingw-w64-x86_64-toolchain base-devel

Then installed Cmake using this command :

pacman -S mingw-w64-x86_64-cmake

Then Installed the nlohmann/json.hpp by using this command:

pacman -S mingw-w64-x86_64-nlohmann-json

if the ffmpeg is properly installed in the C:\ffmpeg\bin it should work.

The video playback works on the UI? I had an issue with the ffmpeg command for mergeChunks yesterday. I'll take a look

omeryusufyagci commented 1 month ago

Hi @IbrahimHamshari, I refactored the core to strictly use std::filesystem::path everywhere, and introduced a command building interface. It was painful to work with the hardcoded, in-place commands.

Could you please update this to conform to these changes as well as to target the PR for the new branch windows-support?

Thanks a lot!

IbrahimHamshari commented 1 month ago

I'll close this pull request and then open another one which has the complete thing.