search

Thalhammer / snowman

Snowboy reimplementation
https://thalhammer.github.io/snowman
Other
74 stars 21 forks source link

Samples devided but never multiplied in gain-control-stream.cpp #14

Closed leso-kn closed 2 years ago

leso-kn commented 2 years ago

Hi! First of all, thanks for making snowman! It's really great to see snowboy living on thanks to the open source community :)

I've noticed a small issue: Calling detector->SetAudioGain(), with any other value than 1.0f seems to be causing the detection to break.

Issue Source

Browsing the code, I think I've found the issue: In gain-control-stream.cpp:39 we devide the audio samples by m_maxAudioAmplitude (set to 32767.0 in line 29), in order to normalize the values for applying the gain modifier.

I've noticed, that we never multiply by m_maxAudioAmplitude afterwards, resulting in the samples becoming very quiet. Thus the detection stops working.

This explains that the detection stops working when setting a gain different from 1.0f since the arithmetics are only applied when a different value than 1.0 is passed.

Solution

A simple fix is to add a multiplication in line 47:

- ptr[i] = v;
+ ptr[i] = v * m_maxAudioAmplitude;
leso-kn commented 2 years ago

I can confirm the proposed solution fixes the issue. Let me know if I should submit a PR for this.

> I've not done it yet, since this is really just a dirty fix and there might be at least a more elegant way of changing the arithmetics

Thalhammer commented 2 years ago

This is indeed an oversight on my side. I fixed it in commit 24e025d.

since this is really just a dirty fix

No its not, it is actually the way the original snowboy library did it (in fact almost all of the code is). I am not saying it good and we will definitly need to rewrite large parts of it at some point, but I'd like to have some people actively using it first, to make it worthwhile doing so, as well has having someone to spot bugs (like this one).

Thanks for spoting this (I think we might in fact have hit that bug during working at wasm support but did not realize it) and let me know if you find other ones / have improvements.