-
Notifications
You must be signed in to change notification settings - Fork 4
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
Implement the use of background ambient noise recorded in the TAU rooms #53
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update @iranroman. I left some minor comments.
@@ -582,24 +582,21 @@ def traj_2_ir_idx(XYZs, trajectory): | |||
return indices | |||
|
|||
|
|||
def db2scale(db): | |||
def db2multiplier(db, x): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we leave x=1
as default? I am thinking it could simplify things for some users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x=1
as default assumes peak energy, or -0dB. Using this function without specifying x
makes little sense, I would say. x
is a handy way to pass the average absolute magnitude of an entire signal, for example.
offset=event.source_time, | ||
duration=event.event_duration, | ||
) | ||
else: # repeat ambient file until scape duration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell this considers background noise duration < scape duration. Maybe we should handle the case where the noise is longer, for which we should pick a random portion of the noise of duration len(scape)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the lines right above this else
statement do exactly that. Using librosa.load
with offset
and duration
, where offset is randomly and uniformly sampled from the entire duration of the ambient noise file, and duration is the scape duration.
@@ -507,7 +507,7 @@ def IR_normalizer(IRs): | |||
impulse_responses = np.array([[0.1, 0.2, 0.3], [0.4, 0.5, 0.6]]) | |||
normalized_IRs = IR_normalizer(impulse_responses) | |||
""" | |||
E = np.sqrt(np.sum(np.power(IRs, 2), axis=-1, keepdims=True)) | |||
E = np.sqrt(np.sum(np.power(np.abs(IRs), 2), axis=-1, keepdims=True)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe np.abs
is redundant here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it just in case this function is ever used with a complex-valued number, such as the fft of a real-valued signal.
Thanks for the comments @adrianSRoman, I've replied. |
Looks good! Will stamp and let others review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
Merging now, as this is an important missing piece, and we have two approving reviews. |
…ms (iranroman#53) Key changes are as follows: 1. **README Update**: The README file has been updated to accurately reflect the latest script used for generating examples. This ensures that users have up-to-date information on how to utilize the library. 2. **Script Enhancements**: The example generation script has been updated. 3. **Data Organization**: The dataset directory has been reorganized for clarity and better usability. Specifically, `datasets/rir_datasets/original_RIRs` has been renamed to `datasets/rir_datasets/source_data`. This change reflects that the directory contains more than just Room Impulse Responses (RIRs), including background ambient noise WAV files, thereby making the data organization more intuitive. 4. **Utility Function Implementation**: A new utility function, `db2multiplier`, has been defined in `spatialscaper/utils.py`. This function calculates a multiplier factor from a decibel (dB) value. When this factor is applied to a signal, it adjusts the signal's amplitude to reflect the specified dB level. This implementation is based on the formula `20 * log10(factor * x) ≈ db`, enhancing the library's capability in handling audio signals with precise amplitude adjustments based on dB values. 5. **Incorporation of Ambient Noise**: The library now utilizes background ambient noise recorded in TAU rooms. This addition aims to provide a more realistic spatial audio generation by incorporating ambient sounds into the audio processing and generation capabilities of SpatialScaper. 6. **Development Status Update**: The development status of SpatialScaper has been updated to beta, reflecting the library's broader public use and testing. 7. **Version Bump**: Accompanying these changes is a version update to 0.1.1. This version increment signifies the introduction of non-trivial changes that affect the library's output and functionality. 8. **Code Formatting and Commenting**: The PR includes updates to the codebase using `black` formatting for Python code.
This PR resolves issue #45
Changes required include:
db2multiplier
inspatialscaper/utils.py
, which calculates the multiplier factor from a decibel (dB) value that, when applied to x, adjusts its amplitude to reflect the specified dB. The relationship is based on the formula 20 * log10(factor * x) ≈ dbdatasets/rir_datasets/original_RIRs
todatasets/rir_datasets/source_data
, as these files include more than just RIRs (i.e. the background ambient noise wav files).@sivannavis @beasteers @ChrisIck @adrianSRoman please review, make suggestions and approve so that we can merge with main.