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

Create FX Functions #10

Merged
merged 79 commits into from
Aug 16, 2021
Merged

Create FX Functions #10

merged 79 commits into from
Aug 16, 2021

Conversation

Watermelanie
Copy link
Contributor

@Watermelanie Watermelanie commented Aug 12, 2021

This PR creates filter functions to modify input sound waves for greater flexibility in the audio that is produced from user input. In particular, some functions that this PR will create include:

  • apply_filter: This is the base function where we call other filter functions for each note.
  • get_notes: This separates the audio into note slices and identifies the root note being played.
  • change_volume: This changes the amplitude of audio samples.
  • stretch_audio: This changes the duration of the audio.
  • change_pitch: This changes the pitch of the notes in the audio.
  • add_chords: This builds major or minor triads upon each note in the audio..

joshfeli and others added 30 commits August 2, 2021 16:42
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Concerns: `audio` is an int16 array, so this may lead to rounding and quantization errors.
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Focused on `change_speed` and making sure we return copies of mutable objects

Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Thanks to @irinazoccolini !

Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
to reflect the extra `score` value included in each tuple in the `notes` list

Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
(It doesn't exactly represent the period of the sine wave but the amount of time for which it would play)

Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
The `change_speed` function works find for speeding up audio, but we need a solution for slowing down audio (we need more samples than we're given!)

Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Note added to reveal the very narrow assumptions on which these functions operate; still working out the `overlap_notes` filter

Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
All filters (except `overlap_notes`) will act on a note at a time and will be applied via the `apply_filter` function.

Order of priorities: `_get_notes`, `_get_frequency`, `change_pitch`, `overlap_notes`, and (if time permits), a second look at `change_volume`. All other functions should have a stable implementation, but we likely cannot test anything until the first three functions listed above are fully implemented.

Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
(All located in a new `assets` directory)
farooqashar and others added 6 commits August 12, 2021 17:19
Co-Authored-By: Watermelanie <[email protected]>
Added constant dictionary to `common.py` to map note names to frequencies.

Future TODO: Design a more robust detection function that can detect note onsets for repeated notes (i.e. notes with the same/similar frequencies)

Co-Authored-By: irinazoccolini <[email protected]>
@joshfeli joshfeli requested a review from MBJean August 13, 2021 01:33
@joshfeli joshfeli marked this pull request as ready for review August 13, 2021 01:33
audio = audio_metadata['audio_samples']
sample_rate = audio_metadata['sample_rate']
encoded_audio = wav_to_base64(audio, sample_rate)
# audio_data = filters.change_pitch(audio_data, 3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment meant to demonstrate how one might implement one of your new filters?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was from us testing out the function by playing sound from the webpage. This can be deleted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be! We were using that line to test out some of the filters using the frontend. We were planning on removing the line (perhaps after the hack session), but we could leave it there for future reference!

…r invalid arguments

(With some more debugging)
Copy link
Contributor

@MBJean MBJean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is seriously impressive work, and I'll have to admit that the specifics of some of the filter implementations are beyond my ken. Since I won't be of significant use to you in terms of line-by-line code review, I'll leave a few meta comments:

  1. The docstring comments and cited research are exceptional. Thanks for considering the needs of the team that'll be coming after you.
  2. I'm having trouble running most of the test cases locally. Which of them do we expect to be fully functional at this point?
  3. Is stretch_audio complete, or still a WIP? It seems like it duplicates audio input in sequence rather than 'stretching' it, which seems like a partial implementation that isn't fully complete yet, but I'm willing to admit I might not grok the bigger picture.

Overall the direction is very sound, the code is relatively self-documenting with excellent supplementary docstrings. I'd like to feel confident that the tests support the filter functions, at which point I'll be comfortable approving this. Thanks!

@joshfeli
Copy link
Contributor

The stretch_audio is more of a work in progress. While repeating the audio input works decently for pure sinusoids, this would not work well with acoustic audio and would probably throw off other filters (e.g., with note onset detection). We can add some ideas for more robust approaches to Issue #9.

joshfeli and others added 2 commits August 13, 2021 15:09
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Copy link
Contributor

@MBJean MBJean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for showing off this work today in the hack session. Just a note for posterity: this PR will contain some code that is still a work in progress. Code comments will point to these places. Otherwise, this work is well tested and very well documented, so I leave my approval. Thanks for all your hard work!

Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
@farooqashar
Copy link
Contributor

This is seriously impressive work, and I'll have to admit that the specifics of some of the filter implementations are beyond my ken. Since I won't be of significant use to you in terms of line-by-line code review, I'll leave a few meta comments:

  1. The docstring comments and cited research are exceptional. Thanks for considering the needs of the team that'll be coming after you.
  2. I'm having trouble running most of the test cases locally. Which of them do we expect to be fully functional at this point?
  3. Is stretch_audio complete, or still a WIP? It seems like it duplicates audio input in sequence rather than 'stretching' it, which seems like a partial implementation that isn't fully complete yet, but I'm willing to admit I might not grok the bigger picture.

Overall the direction is very sound, the code is relatively self-documenting with excellent supplementary docstrings. I'd like to feel confident that the tests support the filter functions, at which point I'll be comfortable approving this. Thanks!

The current tests should all be passing. The commented failing tests have some comments that can be used in conjunction with some of the upgrades to the filter functions to make them pass in the future.

joshfeli and others added 3 commits August 13, 2021 16:46
(This makes it easier to filter the audio before sending it to the frontend)

Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
Co-Authored-By: irinazoccolini <[email protected]>
Co-Authored-By: Ashar Farooq <[email protected]>
Co-Authored-By: Watermelanie <[email protected]>
@joshfeli joshfeli merged commit 7c82d01 into master Aug 16, 2021
@joshfeli joshfeli linked an issue Aug 16, 2021 that may be closed by this pull request
4 tasks
@joshfeli joshfeli mentioned this pull request Aug 16, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build a sound toolkit with FX modules
5 participants