Small bug fix to the StereoEnhancer effect and Gain knob

[IanCaio]

Labeling it as after-refactor, though it has a small bug fix and should be a really quick one to review. Only affects the StereoEnhancer plugin.

• Fixes a small bug where the buffer was never cleared because the conditional to clearing it was never reached.

• Improves the clearing conditional so it only clears the buffer if it was actually affected in the previous process cycle.

• Adds a Gain knob to be able to adjust the level of the output signal so a more appropriate comparison of the before/after sound is done.

[LmmsBot]

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Windows

• Windows 32-bit: lmms-1.3.0-alpha.1.88%2Bg928d3faec-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha.1.88%2Bg928d3faec-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

Linux

• Linux (AppImage): lmms-1.3.0-alpha.1.88%2Bg928d3fa-linux-x86_64.AppImage (build link)

macOS

• macOS : lmms-1.3.0-alpha.1.88%2Bg928d3faec-mac10.14.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://13199-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.88%2Bg928d3faec-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13199?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://13197-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.88%2Bg928d3faec-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13197?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/e4nwgrp2m2buiugb/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38409388"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/7rggc6klkmj61wxr/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38409388"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://13198-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.88%2Bg928d3fa-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13198?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://13196-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.88%2Bg928d3faec-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13196?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "0c968637dff9c917e52c6b6f85d67df5d990fe27"}

[Spekular]

Since clear is both a verb and an adjective, the variable name is a bit confusing. Can we think of something unambiguous instead?

bufferClear, bufferCleared, bufferIsClear
shouldClearBuffer, mustClearBuffer
filledBuffer, fillBuffer

[Veratil]

Since clear is both a verb and an adjective, the variable name is a bit confusing. Can we think of something unambiguous instead?

bufferClear, bufferCleared, bufferIsClear
shouldClearBuffer, mustClearBuffer
filledBuffer, fillBuffer

Would empty be clearer? (no pun intended lol)

[IanCaio]

Sure, now that I read it again name is a bit ambiguous. Everyone fine with m_emptyBuffer?

[Spekular]

I skipped that one intentionally because empty is another example of a verb and adjective, so it's no different from clear.

[IanCaio]

On second thought, personally I think since the prefix and lack of parenthesis makes clear it's a variable it isn't that confusing. But I went and changed it to m_isBufferClear, which looks good too IMO. I also forgot to set it to true on the clear method, it's fixed now.

[Veratil]

On second thought, personally I think since the prefix and lack of parenthesis makes clear it's a variable it isn't that confusing. But I went and changed it to m_isBufferClear, which looks good too IMO. I also forgot to set it to true on the clear method, it's fixed now.

I think bufferIsClear reads better than isBufferClear. if (m_isBufferClear) reads as if is buffer clear while if (m_bufferIsClear) reads as if buffer is clear.

[Spekular]

the prefix and lack of parenthesis makes clear it's a variable

I didn't mean that it would be confused for a method, I meant that the purpose of the variable could be misunderstood (inverted)

• clearBuffer = true, as in "the buffer is clear"
• clearBuffer = true, as in "we should/will clear the buffer (because it isn't clear right now)"

+1 for Veratil's point about the naming in if-statements.

[IanCaio]

I didn't mean that it would be confused for a method, I meant that the purpose of the variable could be misunderstood (inverted)

* clearBuffer = true, as in "the buffer is clear"

* clearBuffer = true, as in "we should/will clear the buffer (because it isn't clear right now)"

Oh, I see it now! You're right, it can be very misleading.

I changed it again to Veratil's suggestion!

[Veratil]

To the same point of readability, is buffer clear or is buffer empty a better read?

[IanCaio]

I think either is fine, but clear has the advantage of being consistent with the method name.

[PhysSong]

Why you didn't implement loading/saving m_outputGain?

[IanCaio]

Why you didn't implement loading/saving m_outputGain?

Oh, I think I forgot it 😅

I'll add it and push it later today

[IanCaio]

Now outputGain is loaded and saved on the project file!

[zonkmachine]

Looks like it's pretty much done from judging by the comments. A quick rebase/merge?
@IanCaio Are you still around? 🐳

[zonkmachine]

I resolved a merge conflict via the web-editor. It seem to have failed to merge correctly anyway and the last manual change should fix that.