Remove 'NonPositiveEnergyError' exception when mix audio #922
Conversation
… if energy<=0.0 but just print warning.
|
You may need to modify the tests in accordance with the new behavior. |
lhotse/audio.py
Outdated
| @@ -1236,8 +1236,8 @@ def __init__( | |||
| self.reference_energy = reference_energy | |||
|
|
|||
| if self.reference_energy <= 0.0: | |||
| raise NonPositiveEnergyError( | |||
| f"To perform mix, energy must be non-zero and non-negative (got {self.reference_energy})" | |||
| logging.warning( | |||
There was a problem hiding this comment.
I would avoid the warning at all -- it will just annoy the users.
lhotse/audio.py
Outdated
| @@ -1323,6 +1323,9 @@ def add_to_mix( | |||
| if audio.size == 0: | |||
| return # do nothing for empty arrays | |||
|
|
|||
| if self.reference_energy <= 0.0: | |||
There was a problem hiding this comment.
hmmm in that case we have audio data with all zeroes, and want to mix something in -- the result should be mixed in audio rather than all zeros. I would remove this if here
lhotse/audio.py
Outdated
| @@ -1331,16 +1334,16 @@ def add_to_mix( | |||
| gain = 1.0 | |||
| if snr is not None: | |||
There was a problem hiding this comment.
I would change this to
| if snr is not None: | |
| if snr is not None and self.reference_energy > 0: |
lhotse/audio.py
Outdated
| # we need to take a square root of the energy ratio. | ||
| gain = sqrt(target_energy / added_audio_energy) | ||
| else: | ||
| logging.warning( |
There was a problem hiding this comment.
We don't need the warning, it will just annoy users.
lhotse/features/mixer.py
Outdated
| ), f"To perform mix, energy must be non-zero and non-negative (got {self.reference_energy})" | ||
| if self.reference_energy <= 0.0: | ||
| logging.warning( | ||
| f"To perform mix, energy must be non-zero and non-negative (got {self.reference_energy}), ignore mix operation." |
There was a problem hiding this comment.
Same as above, I'd just remove this.
There was a problem hiding this comment.
The rest of the comments I made above in audio.py are also relevant for this file
Done~ |
| with pytest.raises(NonPositiveEnergyError): | ||
| mixed.load_audio() | ||
| mix_cut_samples = mixed.load_audio() | ||
| assert mix_cut_samples.shape[1] == sr |
There was a problem hiding this comment.
can you additionally assert that np.testing.assert_equal(rand_cut.load_audio(), mix_cut_samples)?
| with pytest.raises(NonPositiveEnergyError): | ||
| mixed.load_audio() | ||
| mix_cut_samples = mixed.load_audio() | ||
| assert mix_cut_samples.shape[1] == sr | ||
|
|
||
| @pytest.mark.parametrize("snr", [None, 10]) | ||
| def test_fault_tolerant_loading_skips_cut(self, snr): |
There was a problem hiding this comment.
Can you remove this test? It does not do anything right now since mixing zero cuts cannot trigger failure in audio loading.
|
Thanks! |
Remove 'NonPositiveEnergyError' exception when mix audio, still check if energy<=0.0 but just print warning.
For more detail reason, please see this issue.