[Feature development] A native oscilloscope plugin

[bratpeki]

Hello!

I am currently developing an oscilloscope for LMMS.

Initially, it is meant to be similar to the oscilloscope available at the top of the main window toolbar, except that it will be applicable to any effect stack.

Then, I would like to be able to change the time window window of the oscilloscope, so that it can be used a time-variable waveform display, which is useful for mixing.

Then, I would implement pitch detection capabilities, so the oscilloscope can keep periodic signals steady.

This issue has been made as per the statement made in the LMMS README: "Before coding a new big feature, please always file an issue..."

Please do keep in mind that, despite having contributed and written LMMS code before, this is my first time making a plugin. I would still like to develop it myself, with suggestions and help from interested and experienced developers, since the experience would make contributing in the future easier.

Thoughts?

[bratpeki]

Currently, the scope is grabbing the same data from Engine as the main window scope. I'll work toward it grabbing the frames of the current track, not the entire song.

[michaelgregorius]

@bratpeki, work on an oscilloscope was started by @IanCaio some time ago. His branch is rather old but some months ago I merged it with `master´, fixed the build and pushed it into my repository so that does not get lost. You can find it here: https://github.com/michaelgregorius/lmms/tree/plugin/WaveAnalyzer

The current implementation has some performance problems. Perhaps you'd like to take a look at these. See also the discussion that started here: #7155 (comment).

If you are interested in this we could also push the branch to LMMS's main repository so that it can be worked on "officially" and collaboratively.

@IanCaio's original branch can be found here: https://github.com/IanCaio/lmms/tree/plugin/WaveAnalyzer

[michaelgregorius]

I have just merged my current branch with master again and fixed the build once more (sampleFrame vs. SampeFrame).

The current implementation looks like this:

It looks like I should have rather written that it has "serious performance problems". 😅

A general thing that might be worth working on is the efficient rendering of audio buffers as this might also be causing the slow updates in SlicerT which are described here: #7453 (comment)

[michaelgregorius]

Also linking #1358 here which is an old request for an oscilloscope and vector scope. The vector scope was implemented in #5328 so this issue would deal with the oscilloscope part of the request. If it leads to a pull request then this PR could close #1358.

[bratpeki]

Thank you so much for writing! I'll be checking this out, then! Since you mentioned so many different Git branches, could you link me to the one I should get and build in order to begin testing and code review?

[michaelgregorius]

You're welcome, @bratpeki!

The branch to check out for now is https://github.com/michaelgregorius/lmms/tree/plugin/WaveAnalyzer because it is based on the current master.

[michaelgregorius]

@bratpeki, I forgot to push the changes with regards to sampleFrame vs. SampleFrame. I have now pushed them so that the branch should build without problems.

[bratpeki]

I am immediately noticing this break in the waveform. Happens on all three modes. Will investigate further.

a.mp4

[bratpeki]

I'll be taking the plugin on. I have added it to my own fork and will be looking at the code and working on it! Also, official and collaborative work on this sounds great, since it'll ensure the quality of it! Should I close this issue and open a new issue/PR where we can communicate about the plugin?

[bratpeki]

Also, to note, since this thought came to me, I might not end up using the original codebase, since it's usually easier to implement something yourself than to work on someone else's code, and a waveform display is just a ring buffer and a linedraw call, but I will be taking inspiration from the plugin's UI design. This is, again, just a maybe!

[michaelgregorius]

[...] Should I close this issue and open a new issue/PR where we can communicate about the plugin?

You can leave this issue open and open a additional PR which you can configure such that it automatically closes this issue when it gets merged. The PR can then be used to discuss WIP code if necessary.

[bratpeki]

Alright, I'll do so!

[bratpeki]

I've decided I'll open a PR as soon as the issue stated above is resolved. I have found that the break happens every 256 samples, and am investigating the drawing and buffer fill code. Lost has brought it to my attention that a ring buffer class is now in LMMS, so that might be of use if the buffer handling code would be more readable.

Afterwards, I probably won't have more work other than making the window resizable and maybe adding pitch detection so that we can use the plugin as a proper oscilloscope. And testing, but I believe moving to a proper PR will yield a few testers, which is going to be great!

[bratpeki]

After adding this change to WaveAnalyzer.cpp:

@@ -98,6 +108,14 @@ bool WaveAnalyzerEffect::processAudioBuffer(SampleFrame *buffer, const fpp_t fra
                                m_controls.m_clippedRight.setAutomatedValue(true);
                        }
                }
+
+               printf("buffer\n==============================\n");
+               for ( int i = 0; i <= lastBufferIndex; i++ )
+               {
+                       printf("%f, ", m_controls.m_ampBufferL[i]);
+               }
+               printf("\n==============================\n");
+
                if (frameCount > 0)
                {
                        avgLeft = sqrt(avgLeft / frameCount);

I've managed to collect sample buffers which I could plot using Octave:

If you recall, the signal above was a pure sine. That's exactly what we're getting from the amplitude frame array. Meaning that this is definitely a drawing issue. I'll investigate the appropriate code ASAP.

[Rossmaxx]

Lost has brought it to my attention that a ring buffer class is now in LMMS, so that might be of use if the buffer handling code would be more readable.

Don't use that, I am thinking of removing it, so if you continue with that class, you might end up reworking on it.

Ideally, RingBuffer should just be a vector.

[bratpeki]

If I were to use it, it would be for populating the amplitude buffer.

Given the fact that the amplitude buffers are properly functioning with the current static array implementation (see above), there's no need for the ring buffer. 👍

[michaelgregorius]

Lost has brought it to my attention that a ring buffer class is now in LMMS, so that might be of use if the buffer handling code would be more readable.

Don't use that, I am thinking of removing it, so if you continue with that class, you might end up reworking on it.

Ideally, RingBuffer should just be a vector.

@Rossmaxx , aren't there several implementations of ring buffers in the code base and it was only one that was intended to be removed? I guess they won't get all removed so there should be one that can be used by @bratpeki.

[Rossmaxx]

aren't there several implementations of ring buffers in the code base and it was only one that was intended to be removed?

I meant the one used by multitap echo. The other ring buffer is not usable for this purpose either as that one has some weird functions which I don't understand. But if you want, you can use the other one, named LocklessRingBuffer

[bratpeki]

I'll keep the LocklessRingBuffer in mind then, although, as I stated, I think it will not be necessary. A ring buffer would be used for storing sample frames. However, sample frames are already being stored properly (see the plots above), so this is obviously a graphical issue.

[bratpeki]

I'll use this message as a board of information I collect about the problem:

• In void WaveAnalyzerWaveform::updatePoints(int count), count seems to always be 256, which is the "sample frame distance" at which the break happens
  • when the frame window is 512, the break happens at the half-point, which is 256 frames into the pixel drawing
  • when the same window is 1024, it happens every quarter of the way
  • count doesn't change, even when the buffer size setting is change, or the frame window in the plugin

Saker has informed me that "all of LMMS's audio buffers use a size of 256 sample frames. Larger buffer sizes are handled using a FIFO mechanism that the audio device reads from".

[bratpeki]

Got it!

It's in void WaveAnalyzerWaveform::updatePoints(int count). I made the following addition to the end:

@@ -336,6 +336,18 @@ void WaveAnalyzerWaveform::updatePoints(int count)
                        return;
        }

+       printf("====== 1 \n");
+       for (int i = 0; i < totalPixels; i++ ) {
+               printf("%d, ", m_pointsL[i].x());
+       }
+       printf("\n");
+       printf("====== 2 \n");
+       for (int i = 0; i < totalPixels; i++ ) {
+               printf("%d, ", m_pointsL[i].y());
+       }
+       printf("\n");
+       printf("\n");
+
        repaint();
 }

And plotted the lines in Octave again!

However, upon closer inspection of the first plot, the change happens between points 243 an 244, even though the specified with of the widget is 500:

Every single one of the plots experiences this issue.

I'll investigate further!

[bratpeki]

For such a diff:

@@ -258,6 +258,20 @@ void WaveAnalyzerWaveform::updatePoints(int count)
        int currentFrame = totalFrames - count;
        int currentPixel = totalPixels - (count / framesPerPixel);

+       printf("totalPixels: %d\n", totalPixels);
+       printf("totalFrames: %d\n", totalFrames);
+       printf("lastFrame: %d\n", lastFrame);
+       printf("framesPerPixel: %d\n", framesPerPixel);
+
+       printf("\n");
+
+       printf("baseY: %d\n", baseY);
+       printf("ySpace: %d\n", ySpace);
+       printf("currentFrame: %d\n", currentFrame);
+       printf("currentPixel: %d\n", currentPixel);
+
+       printf("\n");
+
        // Raw mode will get the value of the immediate frame
        // while Peaks/Troughs will get the peaks and troughs of the range
        switch(m_controls->m_drawingMode.value())

the output is:

totalPixels: 500
totalFrames: 512
lastFrame: 511
framesPerPixel: 1

baseY: 100
ySpace: 70
currentFrame: 256
currentPixel: 244

Ideally, currentPixel should be 250, seeing as how we have 256 frames being drawn at a time, and 512 in total. My guess is that framesPerPixel draws the specified number of frames per pixel, resulting in the drawing starting from 500-256/framesPerPixel=244.

[bratpeki]

I have found that the code does a lot of things I find unnecessary.

Namely, I would make the wave analyzer have one mode, and have the option of pausing the wave and a single int knob for the number of frames.

I would hide these options and have them appear with a double click, much like the compressor, or keep them to the side of the oscillator view.

I have started my own plugin development back, taking inspiration from the one by Ian. The development has been wonderfully hasteful with the help of @LostRobotMusic!

Will keep you updated.