marsbroshok / VAD-python

Voice Activity Detector in Python
470 stars 133 forks source link

Odd label connection #2

Closed trikota closed 7 years ago

trikota commented 7 years ago

Hello! For-loop in this function bugs my brain.

def _connect_energy_with_frequencies(self, data_freq, data_energy):
    energy_freq = {}
    for (i, freq) in enumerate(data_freq):
        energy_freq[abs(freq)] = data_energy[i]
    return energy_freq

data_freq array is "symmetrical" such that if there is '-1.3' there also is '1.3'. We assigning different values to same dict key twice [abs(freq)] . Not only this is pointless from general performance perspective. Sign-opposite keys in dict represent amplitudes of same frequency wave in antiphase. Shouldn't we do this instead ?

def _connect_energy_with_frequencies(self, data_freq, data_energy):
    energy_freq = {}
    for (i, freq) in enumerate(data_freq):
        if abs(freq) in energy_freq:
            energy_freq[abs(freq)] += data_energy[i]
        else:
            energy_freq[abs(freq)] = data_energy[i]
    return energy_freq

Or perhaps '-=' ?

marsbroshok commented 7 years ago

Hello, good point, thank you! Because the frequencies are symmetrical, I think that should be the correct way:

def _connect_energy_with_frequencies(self, data_freq, data_energy):
    energy_freq = {}
    for (i, freq) in enumerate(data_freq):
        if abs(freq) not in energy_freq:
            energy_freq[abs(freq)] = data_energy[i] * 2
    return energy_freq

What do you think?

trikota commented 7 years ago

I think it definitely looks pretty now :+1: Closing the issue.