Sampletrack activity indicator

[serdnab]

My proposed implementation for the Sample Track activity indicator led.
When a sample track is playing samples it emits the corresponding signal to FadeButton that activates the button animation.
When It ceases playing samples it does the same with the signal notPlaying().
This is a work in progress, the behaviour is missing when the track is muted (maybe the same as InstrumentTrack, that is, the button fades in to another color), and the action when the led is pressed.
Suggestions for these?

[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! 🎩

Linux

• Linux (AppImage): lmms-1.2.1.577-linux-x86_64.AppImage (build link)

Windows

• Windows 32-bit: lmms-1.2.1.577-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.2.1.577-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.2.1-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.2.1-msvc2017-win64.exe (build link)

macOS

• macOS : lmms-1.2.1.577-mac10.13.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://5281-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.577-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5281?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://5280-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.577-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5280?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://5279-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.577-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5279?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/49gax8hkq5sbiw86/artifacts/build/lmms-1.2.1-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/28979390"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/2jbynyibx1xcrq01/artifacts/build/lmms-1.2.1-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/28979390"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://5283-15778896-gh.circle-artifacts.com/0/lmms-1.2.1.577-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/5283?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "df3e3883935fe71976f34ef97ce92f835985ccbf"}

[Spekular]

Here's the relevant change (mute color) for the instrument track indicators: https://github.com/LMMS/lmms/pull/1710/files

I think it'd be ideal if the mute behavior could be moved into a parent class (Track.cpp?). Maybe move the code up and wrap it in a check to see if the track has an indicator?

[serdnab]

Maybe move the code up and wrap it in a check to see if the track has an indicator?

I'm on it.

[serdnab]

Done. Moved the activity indicator mute code from InstrumentTrackView to the parent class (TrackView) and set the mute code for SampleTrackView.

[Spekular]

Sorry for the super delayed response. Testing results:

• Doesn't react to empty sample clips (good)
• Flashes fully lit when it enters a sample clip after not being on one, but overlapping clips don't re-trigger the indicator
• With longer clips, spends most of its time half lit (same intensity as muted tracks trigger color) since it only reacts to on/off & not volume
• Activates even on totally silent clips (not too surprising given previous findings)
• If a clip is extended, indicator stays lit even when entering the part with no audio. If the clip is cropped from the left so the audio is excluded, the empty segment doesn't trigger the indicator. If the clip is extended leftwards, the indicator only triggers once it reaches the actual audio.

Since reacting to volume would require more significant changes to FadeButton, I'm fine with this behavior as an improvement over having no indicator at all. The one thing I would want fixed, if possible, is that the indicator stays when entering an empty section of a clip (if you've extended the clip).

As for the code, my gut instinct is that it could be simplified a bit more. I'd like to take a closer look at it before making any suggestions though, and unfortunately that could take me a few days to get around to.

[Spekular]

I had a go at refactoring this, after fixing the merge issues. I tried to introduce fewer new members and prefer positive/present (isPlaying) over negative/past (wasPlaying, notPlaying). My version is here: master...Spekular:SampleIndicator

I'm not sure what the best way to handle this PR is, but just in case: @serdnab do you have anything against it if this is merged in a separate/new PR?

[Spekular]

I also did some additional testing, because I realized sample tracks in the BB editor were a special case. For a while, my version made the indicators stick on forever after one trigger, which is a problem that the original PR didn't have. However, I managed to fix it, so both PRs are now the same from a user perspective (as far as I can tell).

[Spekular]

Closing, replaced by #5477.

[serdnab]

@Spekular

do you have anything against it if this is merged in a separate/new PR?

ok, no problem