Skip to content

Feature/mixer - #1

Open
infoniten wants to merge 15 commits into
masterfrom
feature/mixer
Open

Feature/mixer#1
infoniten wants to merge 15 commits into
masterfrom
feature/mixer

Conversation

@infoniten

Copy link
Copy Markdown
Collaborator

add voice mixer and voice compressor

@infoniten infoniten added the enhancement New feature or request label Feb 12, 2020
Comment thread mixer/mixer.go
}

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

Copy link
Copy Markdown
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.

Comment thread mixer/mixer.go
}
}

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

Copy link
Copy Markdown
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.

Comment thread mixer/mixer.go
return m.audioCompressor.Compress(bitVoice1 + bitVoice2)
}

func (m *Mixer) setRecivedVoice1(volume float64) {

Copy link
Copy Markdown
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.

Comment thread mixer/mixer_test.go
)

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

Copy link
Copy Markdown
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

Comment thread mixer/mixer_test.go
@@ -0,0 +1,183 @@
package mixer

Copy link
Copy Markdown
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.

Comment thread mixer/mixer_test.go

m := new(Mixer)
compressor := new(compressor.MockCompressorImpl)
m.audioCompressor = compressor

Copy link
Copy Markdown
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.

Comment thread mixer/mixer.go
m.setRecivedVoice1(bitChanVoice1)
m.waitAnotherChan(chanVoice2, &m.voice2)
} else {
chanVoice1 = nil

Copy link
Copy Markdown
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

Comment thread mixer/mixer_test.go

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

go saveChanToFile(output, exit2, buffer1, t)

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Owner

Choose a reason for hiding this comment

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

thresholdHasBeenExceeded

Comment thread mixer/mixer.go
)

//Mixer compare voice streams
type Mixer struct {

Copy link
Copy Markdown
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}
}

Comment thread compressor/Compressor.go
package compressor

//Compressor interface for sound compressor's
type Compressor interface {

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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