med1844 / 2022-MUSI6106

Template project for assignments and exercises for the class MUSI6106
GNU General Public License v3.0
0 stars 0 forks source link

Assignment1 combfilter peer review #2

Open med1844 opened 2 years ago

med1844 commented 2 years ago

Feel free to criticize code design.

thiago-roque07 commented 2 years ago

I really liked the way you structured the main function with auxiliary functions, this makes the code much easier to understand. Your test structure is quite neat. I liked it very much! Honestly, I couldn't fully understand the way you are parsing the input arguments, but the code structure seems good. Something that I got a bit worried is the fact that you are reading the whole file then processing the whole file. On my first implementation I was doing something similar, and the code was crashing. It might be a good idea to do it by blocks, just like the original MUSI6106Exec.cpp was working for the simple read and write file.

For some reason I couldn't run your code with the sweep.wav file... The terminal printed several times the message "can't open fake_id.wav: No such file or directory". I'm not sure if it was something I was doing wrong, but it was a bit weird. The tests run well, though.

(Ps: Yes, I did a stupid confusion with Rose's github profile.... lol)

med1844 commented 2 years ago

I really liked the way you structured the main function with auxiliary functions, this makes the code much easier to understand. Your test structure is quite neat. I liked it very much! Honestly, I couldn't fully understand the way you are parsing the input arguments, but the code structure seems good. Something that I got a bit worried is the fact that you are reading the whole file then processing the whole file. On my first implementation I was doing something similar, and the code was crashing. It might be a good idea to do it by blocks, just like the original MUSI6106Exec.cpp was working for the simple read and write file.

For some reason I couldn't run your code with the sweep.wav file... The terminal printed several times the message "can't open fake_id.wav: No such file or directory". I'm not sure if it was something I was doing wrong, but it was a bit weird. The tests run well, though.

(Ps: Yes, I did a stupid confusion with Rose's github profile.... lol)

Hi! I'm pretty sure I'm reading block by block, as the kllBlockSize suggests:

    for (long long pos = 0; pos < audioLengthInFrame; phInputAudioFile->getPosition(pos)) {
        phInputAudioFile->readData(ppfInputAudioData, kllBlockSize);
        combFilterInterface->process(ppfInputAudioData, ppfOutputAudioData, kllBlockSize);
        phOutputAudioFile->writeData(ppfOutputAudioData, kllBlockSize);
    }

And the argument parser is simply iterating over the argv array and compare if the current argument matches any option. This makes random order option possible.

To run sweep.wav, you need to use the following command: MUSI6106Exec.exe -t FIR -d 0.001 -g 0.9 -i sweep.wav -o output.wav, where 0.001 means the delay time is 0.001 second, and 0.9 means the gain parameter is 0.9.

I will add an additional argument -h, that is help, to print the options available for the program.

med1844 commented 2 years ago

@thiago-roque07 I have added a usage printer function and you should be able to run the executable! Here's the printed usage help:

Usage: MUSI6106Exec.exe [option]

Options and arguments:
-t: The type of comb filter to use. There are two choices available: FIR or IIR.
    -t FIR: use FIR comb filter.
    -t IIR: use IIR comb filter.
-d: The length of delay in second.
    -d <delay>: set the delay time in second of the comb filter. For good comb effect this
                value should be around 0.001-0.01. The MAXIMUM possible delay length is 1s
-g: The gain parameter for the filter.
    -g <gain>: set the gain of the comb filter, which is similar to the amount of 'feedback'.
               The higher this value the 'sharper' the result is.
-i: The path to input wave file.
    -i <path>: load the wave file of <path>. Notice you need to include filename.
-o: The path to output wave file.
    -o <path>: write the filtered signal into <path>. Notice you need to include filename
-h --help: print this help message.
Examples:
- Read 'fake_id.wav' and filter it using a FIR comb filter, with delay time 0.001s and gain 0.9,
  write the filtered signal into 'out.wav':
  `MUSI6106Exec.exe -t FIR -d 0.001 -g 0.9 -i fake_id.wav -o out.wav`
- Run test:
  `MUSI6106Exec.exe`
- Display this help:
  `MUSI6106Exec.exe -h`
  or
  `MUSI6106Exec.exe --help`

Feel free to let me know if you still can't run the code!