search

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
199 stars 21 forks source link

Feature: Add ffmpegcontroller to core (#60) #63

Open andcscott opened 1 month ago

andcscott commented 1 month ago

Hi @omeryusufyagci

I may have jumped the gun on this, but I'm eager for your feedback and I figure we'll go through a couple rounds of iteration to make sure it's right anyway.

I have three commits for you this time that will close #4 and #60.

  1. Update FFmpegCommandBuilder to prepare for FFmpegController
  2. Update FFmpegConfigManager for the same purpose
  3. Finally add FFmpegController :1st_place_medal:

I've attempted to be specific with the changes made in each commit message, please let me know if there are any concerns.

Still todo (not added yet as I assume they'd be revised based on any required changes):

andcscott commented 3 weeks ago

HI @omeryusufyagci just wanted to let you know I saw your comments, will be working on it this week, sorry things got a little crazy here too!

andcscott commented 3 weeks ago

In addition to the requested changes I also took a pass at adding a test for the public API. Nothing squashed yet to help separate the changes from the additions. Thanks!

andcscott commented 2 weeks ago

Hi @omeryusufyagci, I don't know about you, but it feels pretty good to have this section (almost) ready!

  1. Added the requested utility to the tests. I don't have a ton of experience with gtest. It seemed straightforward, but of course let me know if there's a problem.
  2. Added updateSettings. This implementation uses bitmasks as suggested in #60. While the hash map would have likely been less tedious to implement, I'm hopeful the added efficiency helps "future proof" the class.
  3. Added docstrings.

Nothing squashed or merged yet (except https://github.com/omeryusufyagci/fast-music-remover/pull/63/commits/c5139d60ddda67b54814c6d3278271b44e6b8088 where I had some redundancy), please let me know how you want to proceed with that.

Thanks!

andcscott commented 2 days ago

Hi @omeryusufyagci, first of all sorry this took me so long, these past couple weeks have been crazy! In addition to the requested changes I also cleaned up the commits a little bit, it was getting a little hard to keep track (at least on my end). This is a holiday weekend for us in the States so I at least wanted to make sure I pushed something before that. As always, please let me know if any additional changes are required. Thanks!