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

Feature/mixer #1

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Feature/mixer #1

wants to merge 15 commits into from

Conversation

infoniten
Copy link
Collaborator

add voice mixer and voice compressor

@infoniten infoniten added the enhancement New feature or request label Feb 12, 2020
}

//Compare two voices to one
func (m *Mixer) Compare(chanVoice1, chanVoice2 chan float64, joinedVoices chan<- float64) {
Copy link
Owner

Choose a reason for hiding this comment

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

Perhaps Mix would be a better name. Audio streams are joined here, not compared.

}
}

func (m *Mixer) waitAnotherChan(ch chan float64, voice *VoiceBit) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why not use a float64 pointer directly? It doesn't look like this new type is needed.

return m.audioCompressor.Compress(bitVoice1 + bitVoice2)
}

func (m *Mixer) setRecivedVoice1(volume float64) {
Copy link
Owner

Choose a reason for hiding this comment

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

No need for a private setter that is used only once.

)

func TestOneChanelInput(t *testing.T) {
ch1, ch2, exit2, output := chanelInitializer()
Copy link
Owner

Choose a reason for hiding this comment

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

It's easier to use Ginkgo's BeforeEach callbacks for setup IMO

@@ -0,0 +1,183 @@
package mixer
Copy link
Owner

Choose a reason for hiding this comment

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

Should be mixer_test. Don't include tests in the same package.


m := new(Mixer)
compressor := new(compressor.MockCompressorImpl)
m.audioCompressor = compressor
Copy link
Owner

Choose a reason for hiding this comment

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

Mixer does not allow this private field to be set by other packages. Tests shouldn't do it either.
This is another reason why it's better to put tests in a separate _test package.

m.setRecivedVoice1(bitChanVoice1)
m.waitAnotherChan(chanVoice2, &m.voice2)
} else {
chanVoice1 = nil
Copy link
Owner

Choose a reason for hiding this comment

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

Probably need to zero voice1


buffer1 := getBufferFromFile("../samples/Telephone prompt poss.wav", t)

go saveChanToFile(output, exit2, buffer1, t)
Copy link
Owner

Choose a reason for hiding this comment

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

This looks more like an example than a test since we have to check the file manually.
Maybe it should use a real compressor and be in ./examples.
Here should be a simple test with bare values like in Compressor.

threshold float64
attack int
release int
compretionRatio float64
Copy link
Owner

Choose a reason for hiding this comment

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

compressionRatio

releaseTimer int

isInitialize bool
thresholHasBeenExceeded bool
Copy link
Owner

Choose a reason for hiding this comment

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

thresholdHasBeenExceeded

)

//Mixer compare voice streams
type Mixer struct {
Copy link
Owner

Choose a reason for hiding this comment

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

There should be an initialization func:

func NewMixer(compressor compressor.Compressor) *Mixer {
    return &Mixer{audioCompressor: compressor}
}

package compressor

//Compressor interface for sound compressor's
type Compressor interface {
Copy link
Owner

Choose a reason for hiding this comment

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

In Go interfaces are usually defined in user packages. So Mixer should define what it accepts as a Compressor.

}

//NewDynamicRangeCompressor creates new DynamicRangeCompressor to process sounds
func NewDynamicRangeCompressor(threshold, compretionRatio float64, attack, release int) *DynamicRangeCompressor {
Copy link
Owner

Choose a reason for hiding this comment

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

It's convenient to use a struct instead of obscure parameter lists:

compressor.NewDynamicRangeCompressor(compressor.Options{
    Threshold: 0.8,
    CompressionRatio: 0.8,
    Attack: 200,
    Release: 1000,
})

compressedSound := notCompressedSound

if notCompressedSound > d.threshold {
d.releaseTimer = d.release
Copy link
Owner

Choose a reason for hiding this comment

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

Can attackTimer be set here as well?

d.thresholHasBeenExceeded = true
}

if d.attackTimer == 0 && d.releaseTimer == 0 && d.thresholHasBeenExceeded {
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like it's possible to do this without thresholHasBeenExceeded:
decrease attackTimer until it's zero,
then if attackTimer == 0 decrease releaseTimer (and apply compressFunction) until it's zero

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants