Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove ValueBuffer #7170

Closed
wants to merge 7 commits into from
Closed

Remove ValueBuffer #7170

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
822d81e
Make ValueBuffer unsigned
Snowiiii Mar 27, 2024
95f142f
change to size_t
Snowiiii Mar 28, 2024
6cccf71
Remove ValueBuffer
Snowiiii Mar 29, 2024
193b4ef
Delete settings.json
Snowiiii Mar 29, 2024
bc15840
Merge branch 'master' into master
Snowiiii Mar 29, 2024
2157e93
Switch to clamp
Snowiiii Apr 1, 2024
0dd2559
Fix: macOS CI
Snowiiii Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions include/AutomatableModel.h
View file
Open in desktop
Expand Up @@ -33,7 +33,6 @@
#include "JournallingObject.h"
#include "Model.h"
#include "TimePos.h"
#include "ValueBuffer.h"
#include "ModelVisitor.h"


Expand Down Expand Up @@ -174,7 +173,7 @@ class LMMS_EXPORT AutomatableModel : public Model, public JournallingObject

//! @brief Function that returns sample-exact data as a ValueBuffer
//! @return pointer to model's valueBuffer when s.ex.data exists, NULL otherwise
ValueBuffer * valueBuffer();
std::vector<float> * valueBuffer();

template<class T>
T initValue() const
Expand Down Expand Up @@ -406,7 +405,7 @@ public slots:
ControllerConnection* m_controllerConnection;


ValueBuffer m_valueBuffer;
std::vector<float> m_valueBuffer;
long m_lastUpdatedPeriod;
static long s_periodCounter;

Expand Down
5 changes: 2 additions & 3 deletions include/Controller.h
View file
Open in desktop
Expand Up @@ -30,7 +30,6 @@
#include "Engine.h"
#include "Model.h"
#include "JournallingObject.h"
#include "ValueBuffer.h"

namespace lmms
{
Expand Down Expand Up @@ -70,7 +69,7 @@ class LMMS_EXPORT Controller : public Model, public JournallingObject

virtual float currentValue( int _offset );
// The per-controller get-value-in-buffers function
virtual ValueBuffer * valueBuffer();
virtual std::vector<float> * valueBuffer();

inline bool isSampleExact() const
{
Expand Down Expand Up @@ -155,7 +154,7 @@ public slots:
// buffer for storing sample-exact values in case there
// are more than one model wanting it, so we don't have to create it
// again every time
ValueBuffer m_valueBuffer;
std::vector<float> m_valueBuffer;
// when we last updated the valuebuffer - so we know if we have to update it
long m_bufferLastUpdated;

Expand Down
3 changes: 1 addition & 2 deletions include/ControllerConnection.h
View file
Open in desktop
Expand Up @@ -33,7 +33,6 @@

#include "Controller.h"
#include "JournallingObject.h"
#include "ValueBuffer.h"

#include <vector>

Expand Down Expand Up @@ -73,7 +72,7 @@ class LMMS_EXPORT ControllerConnection : public QObject, public JournallingObjec
return m_controller->currentValue( _offset );
}

ValueBuffer * valueBuffer()
std::vector<float> * valueBuffer()
{
return m_controller->valueBuffer();
}
Expand Down
3 changes: 1 addition & 2 deletions include/LadspaControl.h
View file
Open in desktop
Expand Up @@ -30,7 +30,6 @@

#include "AutomatableModel.h"
#include "TempoSyncKnobModel.h"
#include "ValueBuffer.h"

namespace lmms
{
Expand All @@ -55,7 +54,7 @@ class LMMS_EXPORT LadspaControl : public Model, public JournallingObject
~LadspaControl() override = default;

LADSPA_Data value();
ValueBuffer * valueBuffer();
std::vector<float> * valueBuffer();
void setValue( LADSPA_Data _value );
void setLink( bool _state );

Expand Down
11 changes: 6 additions & 5 deletions include/MixHelpers.h
View file
Open in desktop
Expand Up @@ -27,10 +27,11 @@

#include "lmms_basics.h"

#include <vector>

namespace lmms
{

class ValueBuffer;
namespace MixHelpers
{

Expand All @@ -55,19 +56,19 @@ void addMultiplied( sampleFrame* dst, const sampleFrame* src, float coeffSrc, in
void addSwappedMultiplied( sampleFrame* dst, const sampleFrame* src, float coeffSrc, int frames );

/*! \brief Add samples from src multiplied by coeffSrc and coeffSrcBuf to dst */
void addMultipliedByBuffer( sampleFrame* dst, const sampleFrame* src, float coeffSrc, ValueBuffer * coeffSrcBuf, int frames );
void addMultipliedByBuffer( sampleFrame* dst, const sampleFrame* src, float coeffSrc, std::vector<float> * coeffSrcBuf, int frames );

/*! \brief Add samples from src multiplied by coeffSrc and coeffSrcBuf to dst */
void addMultipliedByBuffers( sampleFrame* dst, const sampleFrame* src, ValueBuffer * coeffSrcBuf1, ValueBuffer * coeffSrcBuf2, int frames );
void addMultipliedByBuffers( sampleFrame* dst, const sampleFrame* src, std::vector<float> * coeffSrcBuf1, std::vector<float> * coeffSrcBuf2, int frames );

/*! \brief Same as addMultiplied, but sanitize output (strip out infs/nans) */
void addSanitizedMultiplied( sampleFrame* dst, const sampleFrame* src, float coeffSrc, int frames );

/*! \brief Add samples from src multiplied by coeffSrc and coeffSrcBuf to dst - sanitized version */
void addSanitizedMultipliedByBuffer( sampleFrame* dst, const sampleFrame* src, float coeffSrc, ValueBuffer * coeffSrcBuf, int frames );
void addSanitizedMultipliedByBuffer( sampleFrame* dst, const sampleFrame* src, float coeffSrc, std::vector<float> * coeffSrcBuf, int frames );

/*! \brief Add samples from src multiplied by coeffSrc and coeffSrcBuf to dst - sanitized version */
void addSanitizedMultipliedByBuffers( sampleFrame* dst, const sampleFrame* src, ValueBuffer * coeffSrcBuf1, ValueBuffer * coeffSrcBuf2, int frames );
void addSanitizedMultipliedByBuffers( sampleFrame* dst, const sampleFrame* src, std::vector<float> * coeffSrcBuf1, std::vector<float> * coeffSrcBuf2, int frames );

/*! \brief Add samples from src multiplied by coeffSrcLeft/coeffSrcRight to dst */
void addMultipliedStereo( sampleFrame* dst, const sampleFrame* src, float coeffSrcLeft, float coeffSrcRight, int frames );
Expand Down
58 changes: 0 additions & 58 deletions include/ValueBuffer.h
View file
Open in desktop

This file was deleted.

16 changes: 8 additions & 8 deletions plugins/Amplifier/Amplifier.cpp
View file
Open in desktop
Expand Up @@ -65,17 +65,17 @@ bool AmplifierEffect::processAudioBuffer(sampleFrame* buf, const fpp_t frames)
const float d = dryLevel();
const float w = wetLevel();

const ValueBuffer* volumeBuf = m_ampControls.m_volumeModel.valueBuffer();
const ValueBuffer* panBuf = m_ampControls.m_panModel.valueBuffer();
const ValueBuffer* leftBuf = m_ampControls.m_leftModel.valueBuffer();
const ValueBuffer* rightBuf = m_ampControls.m_rightModel.valueBuffer();
const auto* volumeBuf = m_ampControls.m_volumeModel.valueBuffer();
const auto* panBuf = m_ampControls.m_panModel.valueBuffer();
const auto* leftBuf = m_ampControls.m_leftModel.valueBuffer();
const auto* rightBuf = m_ampControls.m_rightModel.valueBuffer();

for (fpp_t f = 0; f < frames; ++f)
{
const float volume = (volumeBuf ? volumeBuf->value(f) : m_ampControls.m_volumeModel.value()) * 0.01f;
const float pan = (panBuf ? panBuf->value(f) : m_ampControls.m_panModel.value()) * 0.01f;
const float left = (leftBuf ? leftBuf->value(f) : m_ampControls.m_leftModel.value()) * 0.01f;
const float right = (rightBuf ? rightBuf->value(f) : m_ampControls.m_rightModel.value()) * 0.01f;
const float volume = (volumeBuf ? volumeBuf->at(f % volumeBuf->size()) : m_ampControls.m_volumeModel.value()) * 0.01f;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious if the modulo was covering for some bug in ValueBuffer before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which bug?

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is that this is hiding out of bounds array access. The question is why is this needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just implemented the Old behaviour from ValueBuffer which was:

float ValueBuffer::value(int offset) const
{
	return at(offset % length());
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, and the question is why was modulo being used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the notes not rendering was fixed. Can you load and play an intensive project like Krem Kaakkuja with no modulo/clamp? If so, then I think we can remove the modulo/clamp.

works without any problems.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

@DomClark I think we can remove the clamp, too, now. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

@PhysSong for additional opinion :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should use static code analysis to check every occurrence where the value function is used. In all these cases it must be assured that no out of bounds read is possible. If in doubt, I would suggest to add an assert before such calls and then make tests where this assert is being tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should leave it an runtime check

const float pan = (panBuf ? panBuf->at(f % panBuf->size()) : m_ampControls.m_panModel.value()) * 0.01f;
const float left = (leftBuf ? leftBuf->at(f % leftBuf->size()) : m_ampControls.m_leftModel.value()) * 0.01f;
const float right = (rightBuf ? rightBuf->at(f % rightBuf->size()) : m_ampControls.m_rightModel.value()) * 0.01f;

const float panLeft = std::min(1.0f, 1.0f - pan);
const float panRight = std::min(1.0f, 1.0f + pan);
Expand Down
4 changes: 2 additions & 2 deletions plugins/BassBooster/BassBooster.cpp
View file
Open in desktop
Expand Up @@ -85,7 +85,7 @@ bool BassBoosterEffect::processAudioBuffer( sampleFrame* buf, const fpp_t frames
if( m_bbControls.m_ratioModel.isValueChanged() ) { changeRatio(); }

const float const_gain = m_bbControls.m_gainModel.value();
const ValueBuffer *gainBuffer = m_bbControls.m_gainModel.valueBuffer();
const auto *gainBuffer = m_bbControls.m_gainModel.valueBuffer();

double outSum = 0.0;
const float d = dryLevel();
Expand All @@ -96,7 +96,7 @@ bool BassBoosterEffect::processAudioBuffer( sampleFrame* buf, const fpp_t frames
float gain = const_gain;
if (gainBuffer) {
//process period using sample exact data
gain = gainBuffer->value( f );
gain = gainBuffer->at( f % gainBuffer->size() );
}
//float gain = gainBuffer ? gainBuffer[f] : gain;
m_bbFX.leftFX().setGain( gain );
Expand Down
16 changes: 8 additions & 8 deletions plugins/Delay/DelayEffect.cpp
View file
Open in desktop
Expand Up @@ -98,18 +98,18 @@ bool DelayEffect::processAudioBuffer( sampleFrame* buf, const fpp_t frames )
float amplitude = m_delayControls.m_lfoAmountModel.value() * sr;
float lfoTime = 1.0 / m_delayControls.m_lfoTimeModel.value();
float feedback = m_delayControls.m_feedbackModel.value();
ValueBuffer *lengthBuffer = m_delayControls.m_delayTimeModel.valueBuffer();
ValueBuffer *feedbackBuffer = m_delayControls.m_feedbackModel.valueBuffer();
ValueBuffer *lfoTimeBuffer = m_delayControls.m_lfoTimeModel.valueBuffer();
ValueBuffer *lfoAmountBuffer = m_delayControls.m_lfoAmountModel.valueBuffer();
auto *lengthBuffer = m_delayControls.m_delayTimeModel.valueBuffer();
auto *feedbackBuffer = m_delayControls.m_feedbackModel.valueBuffer();
auto *lfoTimeBuffer = m_delayControls.m_lfoTimeModel.valueBuffer();
auto *lfoAmountBuffer = m_delayControls.m_lfoAmountModel.valueBuffer();
int lengthInc = lengthBuffer ? 1 : 0;
int amplitudeInc = lfoAmountBuffer ? 1 : 0;
int lfoTimeInc = lfoTimeBuffer ? 1 : 0;
int feedbackInc = feedbackBuffer ? 1 : 0;
float *lengthPtr = lengthBuffer ? &( lengthBuffer->values()[ 0 ] ) : &length;
float *amplitudePtr = lfoAmountBuffer ? &( lfoAmountBuffer->values()[ 0 ] ) : &amplitude;
float *lfoTimePtr = lfoTimeBuffer ? &( lfoTimeBuffer->values()[ 0 ] ) : &lfoTime;
float *feedbackPtr = feedbackBuffer ? &( feedbackBuffer->values()[ 0 ] ) : &feedback;
float *lengthPtr = lengthBuffer ? &( lengthBuffer->data()[ 0 ] ) : &length;
float *amplitudePtr = lfoAmountBuffer ? &( lfoAmountBuffer->data()[ 0 ] ) : &amplitude;
float *lfoTimePtr = lfoTimeBuffer ? &( lfoTimeBuffer->data()[ 0 ] ) : &lfoTime;
float *feedbackPtr = feedbackBuffer ? &( feedbackBuffer->data()[ 0 ] ) : &feedback;

if( m_delayControls.m_outGainModel.isValueChanged() )
{
Expand Down
28 changes: 14 additions & 14 deletions plugins/DualFilter/DualFilter.cpp
View file
Open in desktop
Expand Up @@ -107,13 +107,13 @@ bool DualFilterEffect::processAudioBuffer( sampleFrame* buf, const fpp_t frames
float gain2 = m_dfControls.m_gain2Model.value();
float mix = m_dfControls.m_mixModel.value();

ValueBuffer *cut1Buffer = m_dfControls.m_cut1Model.valueBuffer();
ValueBuffer *res1Buffer = m_dfControls.m_res1Model.valueBuffer();
ValueBuffer *gain1Buffer = m_dfControls.m_gain1Model.valueBuffer();
ValueBuffer *cut2Buffer = m_dfControls.m_cut2Model.valueBuffer();
ValueBuffer *res2Buffer = m_dfControls.m_res2Model.valueBuffer();
ValueBuffer *gain2Buffer = m_dfControls.m_gain2Model.valueBuffer();
ValueBuffer *mixBuffer = m_dfControls.m_mixModel.valueBuffer();
auto *cut1Buffer = m_dfControls.m_cut1Model.valueBuffer();
auto *res1Buffer = m_dfControls.m_res1Model.valueBuffer();
auto *gain1Buffer = m_dfControls.m_gain1Model.valueBuffer();
auto *cut2Buffer = m_dfControls.m_cut2Model.valueBuffer();
auto *res2Buffer = m_dfControls.m_res2Model.valueBuffer();
auto *gain2Buffer = m_dfControls.m_gain2Model.valueBuffer();
auto *mixBuffer = m_dfControls.m_mixModel.valueBuffer();

int cut1Inc = cut1Buffer ? 1 : 0;
int res1Inc = res1Buffer ? 1 : 0;
Expand All @@ -123,13 +123,13 @@ bool DualFilterEffect::processAudioBuffer( sampleFrame* buf, const fpp_t frames
int gain2Inc = gain2Buffer ? 1 : 0;
int mixInc = mixBuffer ? 1 : 0;

float *cut1Ptr = cut1Buffer ? &( cut1Buffer->values()[ 0 ] ) : &cut1;
float *res1Ptr = res1Buffer ? &( res1Buffer->values()[ 0 ] ) : &res1;
float *gain1Ptr = gain1Buffer ? &( gain1Buffer->values()[ 0 ] ) : &gain1;
float *cut2Ptr = cut2Buffer ? &( cut2Buffer->values()[ 0 ] ) : &cut2;
float *res2Ptr = res2Buffer ? &( res2Buffer->values()[ 0 ] ) : &res2;
float *gain2Ptr = gain2Buffer ? &( gain2Buffer->values()[ 0 ] ) : &gain2;
float *mixPtr = mixBuffer ? &( mixBuffer->values()[ 0 ] ) : &mix;
float *cut1Ptr = cut1Buffer ? &( cut1Buffer->data()[ 0 ] ) : &cut1;
float *res1Ptr = res1Buffer ? &( res1Buffer->data()[ 0 ] ) : &res1;
float *gain1Ptr = gain1Buffer ? &( gain1Buffer->data()[ 0 ] ) : &gain1;
float *cut2Ptr = cut2Buffer ? &( cut2Buffer->data()[ 0 ] ) : &cut2;
float *res2Ptr = res2Buffer ? &( res2Buffer->data()[ 0 ] ) : &res2;
float *gain2Ptr = gain2Buffer ? &( gain2Buffer->data()[ 0 ] ) : &gain2;
float *mixPtr = mixBuffer ? &( mixBuffer->data()[ 0 ] ) : &mix;

const bool enabled1 = m_dfControls.m_enabled1Model.value();
const bool enabled2 = m_dfControls.m_enabled2Model.value();
Expand Down
5 changes: 2 additions & 3 deletions plugins/LadspaEffect/LadspaEffect.cpp
View file
Open in desktop
Expand Up @@ -36,7 +36,6 @@
#include "LadspaControl.h"
#include "LadspaSubPluginFeatures.h"
#include "AutomationClip.h"
#include "ValueBuffer.h"
#include "Song.h"

#include "embed.h"
Expand Down Expand Up @@ -173,10 +172,10 @@ bool LadspaEffect::processAudioBuffer( sampleFrame * _buf,
break;
case BufferRate::AudioRateInput:
{
ValueBuffer * vb = pp->control->valueBuffer();
auto * vb = pp->control->valueBuffer();
if( vb )
{
memcpy( pp->buffer, vb->values(), frames * sizeof(float) );
memcpy( pp->buffer, vb->data(), frames * sizeof(float) );
}
else
{
Expand Down
16 changes: 8 additions & 8 deletions plugins/ReverbSC/ReverbSC.cpp
View file
Open in desktop
Expand Up @@ -89,28 +89,28 @@ bool ReverbSCEffect::processAudioBuffer( sampleFrame* buf, const fpp_t frames )
SPFLOAT tmpL, tmpR;
SPFLOAT dcblkL, dcblkR;

ValueBuffer * inGainBuf = m_reverbSCControls.m_inputGainModel.valueBuffer();
ValueBuffer * sizeBuf = m_reverbSCControls.m_sizeModel.valueBuffer();
ValueBuffer * colorBuf = m_reverbSCControls.m_colorModel.valueBuffer();
ValueBuffer * outGainBuf = m_reverbSCControls.m_outputGainModel.valueBuffer();
auto * inGainBuf = m_reverbSCControls.m_inputGainModel.valueBuffer();
auto * sizeBuf = m_reverbSCControls.m_sizeModel.valueBuffer();
auto * colorBuf = m_reverbSCControls.m_colorModel.valueBuffer();
auto * outGainBuf = m_reverbSCControls.m_outputGainModel.valueBuffer();

for( fpp_t f = 0; f < frames; ++f )
{
auto s = std::array{buf[f][0], buf[f][1]};

const auto inGain
= (SPFLOAT)DB2LIN((inGainBuf ? inGainBuf->values()[f] : m_reverbSCControls.m_inputGainModel.value()));
= (SPFLOAT)DB2LIN((inGainBuf ? inGainBuf->data()[f] : m_reverbSCControls.m_inputGainModel.value()));
const auto outGain
= (SPFLOAT)DB2LIN((outGainBuf ? outGainBuf->values()[f] : m_reverbSCControls.m_outputGainModel.value()));
= (SPFLOAT)DB2LIN((outGainBuf ? outGainBuf->data()[f] : m_reverbSCControls.m_outputGainModel.value()));

s[0] *= inGain;
s[1] *= inGain;
revsc->feedback = (SPFLOAT)(sizeBuf ?
sizeBuf->values()[f]
sizeBuf->data()[f]
: m_reverbSCControls.m_sizeModel.value());

revsc->lpfreq = (SPFLOAT)(colorBuf ?
colorBuf->values()[f]
colorBuf->data()[f]
: m_reverbSCControls.m_colorModel.value());


Expand Down
8 changes: 4 additions & 4 deletions plugins/WaveShaper/WaveShaper.cpp
View file
Open in desktop
Expand Up @@ -85,14 +85,14 @@ bool WaveShaperEffect::processAudioBuffer( sampleFrame * _buf,
const float * samples = m_wsControls.m_wavegraphModel.samples();
const bool clip = m_wsControls.m_clipModel.value();

ValueBuffer *inputBuffer = m_wsControls.m_inputModel.valueBuffer();
ValueBuffer *outputBufer = m_wsControls.m_outputModel.valueBuffer();
auto *inputBuffer = m_wsControls.m_inputModel.valueBuffer();
auto *outputBufer = m_wsControls.m_outputModel.valueBuffer();

int inputInc = inputBuffer ? 1 : 0;
int outputInc = outputBufer ? 1 : 0;

const float *inputPtr = inputBuffer ? &( inputBuffer->values()[ 0 ] ) : &input;
const float *outputPtr = outputBufer ? &( outputBufer->values()[ 0 ] ) : &output;
const float *inputPtr = inputBuffer ? &( inputBuffer->data()[ 0 ] ) : &input;
const float *outputPtr = outputBufer ? &( outputBufer->data()[ 0 ] ) : &output;

for( fpp_t f = 0; f < _frames; ++f )
{
Expand Down