diff --git a/.github/workflows/acceptance.yml b/.github/workflows/acceptance.yml index c7d4b36..353fc77 100644 --- a/.github/workflows/acceptance.yml +++ b/.github/workflows/acceptance.yml @@ -156,7 +156,7 @@ jobs: shell: bash run: | check() { echo "$1: got '$2', expected '$3'"; [ "$2" = "$3" ] || exit 1; } - check "validate passed" "${{ steps.validate.outputs.passed }}" 3 + check "validate passed" "${{ steps.validate.outputs.passed }}" 4 check "validate failed" "${{ steps.validate.outputs.failed }}" 0 check "validate-missing outcome" "${{ steps.validate-missing.outcome }}" failure check "validate-missing passed" "${{ steps.validate-missing.outputs.passed }}" 1 diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 48bdff5..e7857b6 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -128,7 +128,9 @@ jobs: shell: bash run: | # Paths must be single quoted for bash not to escape the Windows backslash character \ used in absolute paths - ${{ env.APP_DIR }}/${{ matrix.test-binary }} validate --strictness-level 10 '${{ env.PLUGIN_CACHE_PATH }}/DSPModulePluginDemo_artefacts/Release/VST3/DSPModulePluginDemo.vst3' + # "Discrete parameter state restoration" is disabled for DSPModulePluginDemo until juce::AudioParameterBool::setValue + # snaps to 0 or 1: https://forum.juce.com/t/pluginval-can-set-an-audioparameterbools-to-odd-values/50971 + ${{ env.APP_DIR }}/${{ matrix.test-binary }} validate --strictness-level 10 --disabled-tests "Discrete parameter state restoration" '${{ env.PLUGIN_CACHE_PATH }}/DSPModulePluginDemo_artefacts/Release/VST3/DSPModulePluginDemo.vst3' ${{ env.APP_DIR }}/${{ matrix.test-binary }} validate --strictness-level 10 '${{ env.PLUGIN_CACHE_PATH }}/MultiOutSynthPlugin_artefacts/Release/VST3/MultiOutSynthPlugin.vst3' - name: Validate JUCE Plugin examples (AU) @@ -143,7 +145,7 @@ jobs: killall -9 AudioComponentRegistrar # kick the AU registrar # Intentionally uses the deprecated "--validate" flat flag to keep the # backwards-compatible alias covered (this step is continue-on-error). - ${{ env.APP_DIR }}/${{ matrix.test-binary }} --strictness-level 10 --validate ~/Library/Audio/Plug-Ins/Components/DSPModulePluginDemo.component + ${{ env.APP_DIR }}/${{ matrix.test-binary }} --strictness-level 10 --disabled-tests "Discrete parameter state restoration" --validate ~/Library/Audio/Plug-Ins/Components/DSPModulePluginDemo.component - name: Codesign (macOS) if: ${{ matrix.name == 'macOS' }} diff --git a/CHANGELIST.md b/CHANGELIST.md index f816eab..5a3bd3d 100644 --- a/CHANGELIST.md +++ b/CHANGELIST.md @@ -17,6 +17,10 @@ - **Breaking:** invalid option values now error instead of silently falling back (e.g. `--rtcheck banana`, `--strictness-level abc`) - **Breaking:** falsey flag environment variables now mean off (e.g. `SKIP_GUI_TESTS=0` no longer enables skipping) - `--help` output is now generated by CLI11 +- Added a "Discrete parameter state restoration" test (strictness 8) which checks a discrete parameter has the same value after restoring a state, whatever value it held beforehand. This fails for parameters that keep the raw normalised value from the host instead of mapping it to a step (e.g. `juce::AudioParameterBool`) +- Fixed false failures in the "Plugin state restoration" test where the first parameter was compared against the last value sent by a previous test rather than the plugin's own value +- Below strictness level 8, two-step (on/off) parameters are now only sent 0 or 1 when tests randomise parameter values. Arbitrary values are still sent to them at level 8 and above, and to all other parameters at every level +- Fixed the "Plugin state restoration" test's exact binary state comparison never running. It was documented as running at strictness level 8 and above but checked the test's own level instead of the run's - Enabled the editor stress and extreme (real-time allocation / oversized block) tests that were present in the source tree but had not been compiled into the build ### 1.0.5 diff --git a/CLAUDE.md b/CLAUDE.md index c3f2759..dd52b43 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -117,7 +117,8 @@ pluginval/ │ ├── test_plugins/ # Test plugin files │ │ ├── tone_generator/ # Deterministic dogfood generator (PLUGINVAL_BUILD_TEST_PLUGINS) │ │ ├── gain/ # Deterministic dogfood gain effect (for the input.audio path) -│ │ └── playhead_probe/ # Writes the host transport to output (for the playhead path) +│ │ ├── playhead_probe/ # Writes the host transport to output (for the playhead path) +│ │ └── discrete_params/ # Bool/choice-only plugin (for the discrete parameter state test) │ ├── acceptance/ # Acceptance self-tests: configs (*.json.in), inputs/, checked-in refs/ WAVs │ ├── mac_tests/ # macOS-specific tests │ └── windows_tests.bat # Windows test scripts @@ -379,7 +380,7 @@ struct Requirements { | File | Tests Included | |------|----------------| -| `BasicTests.cpp` | PluginInfo, Programs, Editor, AudioProcessing, PluginState, Automation, auval, VST3validator | +| `BasicTests.cpp` | PluginInfo, Programs, Editor, AudioProcessing, PluginState, DiscreteParameterState, Automation, auval, VST3validator | | `BusTests.cpp` | Bus layout, channel configuration | | `EditorTests.cpp` | Editor creation, resizing | | `ParameterFuzzTests.cpp` | Random parameter value testing | @@ -443,6 +444,7 @@ static MyNewTest myNewTest; Located in `TestUtilities.h`: - `getNonBypassAutomatableParameters()` - Get automatable params +- `getRandomParameterValue()` - Random value to set on a param (only 0 or 1 for two-step params below strictness 8) - `fillNoise()` - Fill buffer with random audio - `countNaNs()`, `countInfs()`, `countSubnormals()` - Audio validation - `ScopedEditorShower` - RAII editor creation/destruction diff --git a/CMakeLists.txt b/CMakeLists.txt index d5cd63b..ba9d07d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -289,5 +289,6 @@ if (PLUGINVAL_BUILD_TEST_PLUGINS) add_subdirectory(tests/test_plugins/tone_generator) add_subdirectory(tests/test_plugins/gain) add_subdirectory(tests/test_plugins/playhead_probe) + add_subdirectory(tests/test_plugins/discrete_params) add_subdirectory(tests/acceptance) endif() diff --git a/source/TestUtilities.h b/source/TestUtilities.h index e7a304a..148760c 100644 --- a/source/TestUtilities.h +++ b/source/TestUtilities.h @@ -53,6 +53,23 @@ static inline juce::Array getNonBypassAutomatabl return parameters; } +/** Returns a random normalised value to set on a parameter. + + Below strictness level 8, two-step (on/off) parameters are only sent 0 or 1. + Any value is valid, but some plugins keep the values in between rather than + mapping them to a step, which the state restoration tests then pick up. +*/ +static inline float getRandomParameterValue (juce::Random& r, const juce::AudioProcessorParameter& parameter, int strictnessLevel) +{ + // N.B. hosted VST3 parameters don't report isBoolean so also check the number of steps + const bool isTwoStep = parameter.isBoolean() || parameter.getNumSteps() == 2; + + if (isTwoStep && strictnessLevel < 8) + return r.nextBool() ? 1.0f : 0.0f; + + return r.nextFloat(); +} + //============================================================================== template void iterateAudioBuffer (juce::AudioBuffer& ab, UnaryFunction fn) diff --git a/source/tests/BasicTests.cpp b/source/tests/BasicTests.cpp index 3e21baa..83416a8 100644 --- a/source/tests/BasicTests.cpp +++ b/source/tests/BasicTests.cpp @@ -347,7 +347,7 @@ struct PluginStateTest : public PluginTest // Set random parameter values for (auto parameter : getNonBypassAutomatableParameters (instance)) - parameter->setValue (r.nextFloat()); + parameter->setValue (getRandomParameterValue (r, *parameter, ut.getOptions().strictnessLevel)); // Restore original state callSetStateInformationOnMessageThreadIfVST3 (instance, originalState); @@ -378,13 +378,17 @@ struct PluginStateTestRestoration : public PluginTest // Read state auto originalState = callGetStateInformationOnMessageThreadIfVST3 (instance); + // Hosted parameters report the last value sent to them until a state is + // restored, so restore once here to start from the plugin's own values + callSetStateInformationOnMessageThreadIfVST3 (instance, originalState); + const auto tolaratedDiff = 0.1f; // Set random parameter values for (auto parameter : getNonBypassAutomatableParameters(instance)) { const auto originalValue = parameter->getValue(); - parameter->setValue(r.nextFloat()); + parameter->setValue (getRandomParameterValue (r, *parameter, ut.getOptions().strictnessLevel)); // Restore original state callSetStateInformationOnMessageThreadIfVST3(instance, originalState); @@ -394,7 +398,7 @@ struct PluginStateTestRestoration : public PluginTest parameter->getName(1024) + juce::String(" not restored on setStateInformation")); } - if (strictnessLevel >= 8) + if (ut.getOptions().strictnessLevel >= 8) { // Read state again and compare to what we set auto duplicateState = callGetStateInformationOnMessageThreadIfVST3 (instance); @@ -417,6 +421,67 @@ struct PluginStateTestRestoration : public PluginTest static PluginStateTestRestoration pluginStateTestRestoration; +//============================================================================== +/** + Tests that a discrete parameter reports the same value after a state restore + regardless of the value it held beforehand. + + A host can send any normalised value to a discrete parameter and only the + plugin knows which step that maps to. If the plugin keeps the raw value for + a single parameter change but saves/restores the step, the value it reports + depends on how it got there. + See https://forum.juce.com/t/pluginval-can-set-an-audioparameterbools-to-odd-values/50971 +*/ +struct DiscreteParameterStateTest : public PluginTest +{ + DiscreteParameterStateTest() + : PluginTest ("Discrete parameter state restoration", 8) + { + } + + void runTest (PluginTests& ut, juce::AudioPluginInstance& instance) override + { + auto r = ut.getRandom(); + + for (auto parameter : getNonBypassAutomatableParameters (instance)) + { + if (! parameter->isDiscrete()) + continue; + + // Any normalised value is valid so use ones that are unlikely to be on a step + const auto value = 0.1f + 0.8f * r.nextFloat(); + const auto otherValue = value < 0.5f ? value + 0.4f : value - 0.4f; + + parameter->setValue (value); + const auto state = callGetStateInformationOnMessageThreadIfVST3 (instance); + + // Restore the state with the parameter still at the value it was saved with + callSetStateInformationOnMessageThreadIfVST3 (instance, state); + const auto restoredFromValue = parameter->getValue(); + + // Then restore it again from a different value + parameter->setValue (otherValue); + callSetStateInformationOnMessageThreadIfVST3 (instance, state); + const auto restoredFromOtherValue = parameter->getValue(); + + ut.logVerboseMessage (parameter->getName (1024) + ": set " + juce::String (value) + ", restored as " + juce::String (restoredFromValue) + + "; set " + juce::String (otherValue) + ", restored as " + juce::String (restoredFromOtherValue)); + + ut.expectWithinAbsoluteError (restoredFromOtherValue, restoredFromValue, 0.0001f, + parameter->getName (1024) + " has a different value after restoring the same state"); + } + } + + std::vector getDescription (int) const override + { + return { { name, "For each discrete param: sets a random value, saves state, then restores that state twice - " + "once unchanged and once after setting a different value - and expects the same value both times" } }; + } +}; + +static DiscreteParameterStateTest discreteParameterStateTest; + + //============================================================================== struct AutomationTest : public PluginTest { @@ -474,7 +539,7 @@ struct AutomationTest : public PluginTest for (int i = 0; i < juce::jmin (10, parameters.size()); ++i) { const int paramIndex = r.nextInt (parameters.size()); - parameters[paramIndex]->setValue (r.nextFloat()); + parameters[paramIndex]->setValue (getRandomParameterValue (r, *parameters[paramIndex], ut.getOptions().strictnessLevel)); } } @@ -552,7 +617,7 @@ struct EditorAutomationTest : public PluginTest while (--numBlocks >= 0) { for (auto parameter : parameters) - parameter->setValue (r.nextFloat()); + parameter->setValue (getRandomParameterValue (r, *parameter, ut.getOptions().strictnessLevel)); ut.resetTimeout(); juce::Thread::sleep (10); @@ -702,7 +767,7 @@ struct BackgroundThreadStateTest : public PluginTest // Set random parameter values for (auto parameter : parameters) - parameter->setValue (r.nextFloat()); + parameter->setValue (getRandomParameterValue (r, *parameter, ut.getOptions().strictnessLevel)); // Restore original state callSetStateInformationOnMessageThreadIfVST3 (instance, originalState); @@ -739,6 +804,7 @@ struct ParameterThreadSafetyTest : public PluginTest auto parameters = getNonBypassAutomatableParameters (instance); const bool isPluginInstrument = instance.getPluginDescription().isInstrument; const int numBlocks = 500; + const int runStrictnessLevel = ut.getOptions().strictnessLevel; // This emulates the plugin itself setting a value for example from a slider within its UI juce::MessageManager::callAsync ([&, threadRandom = r]() mutable @@ -747,7 +813,7 @@ struct ParameterThreadSafetyTest : public PluginTest for (int i = 0; i < numBlocks; ++i) for (auto param : parameters) - param->setValueNotifyingHost (threadRandom.nextFloat()); + param->setValueNotifyingHost (getRandomParameterValue (threadRandom, *param, runStrictnessLevel)); endWaiter.signal(); }); @@ -777,7 +843,7 @@ struct ParameterThreadSafetyTest : public PluginTest addNoteOff (mb, noteChannel, noteNumber, 0); for (auto param : parameters) - param->setValue (r.nextFloat()); + param->setValue (getRandomParameterValue (r, *param, runStrictnessLevel)); fillNoise (ab); diff --git a/source/tests/ParameterFuzzTests.cpp b/source/tests/ParameterFuzzTests.cpp index c2a3c86..3abee71 100644 --- a/source/tests/ParameterFuzzTests.cpp +++ b/source/tests/ParameterFuzzTests.cpp @@ -44,7 +44,7 @@ struct FuzzParametersTest : public PluginTest for (int i = 0; i < 5; ++i) { - const float value = r.nextFloat(); + const float value = getRandomParameterValue (r, parameter, ut.getOptions().strictnessLevel); parameter.setValue (value); const float v = parameter.getValue(); diff --git a/tests/test_plugins/discrete_params/CMakeLists.txt b/tests/test_plugins/discrete_params/CMakeLists.txt new file mode 100644 index 0000000..fae0ca1 --- /dev/null +++ b/tests/test_plugins/discrete_params/CMakeLists.txt @@ -0,0 +1,37 @@ +# A minimal plugin with only discrete (bool / choice) parameters, used to +# dogfood the "Discrete parameter state restoration" test. +# Built only when PLUGINVAL_BUILD_TEST_PLUGINS is ON. + +set(discrete_params_formats VST3) + +if (APPLE) + list(APPEND discrete_params_formats AU) +endif() + +juce_add_plugin(pluginval_discrete_params + PRODUCT_NAME "pluginval Discrete Params" + COMPANY_NAME Tracktion + BUNDLE_ID com.Tracktion.pluginvalDiscreteParams + PLUGIN_MANUFACTURER_CODE Trkt + PLUGIN_CODE Pdp1 + FORMATS ${discrete_params_formats} + IS_SYNTH FALSE + NEEDS_MIDI_INPUT FALSE + COPY_PLUGIN_AFTER_BUILD FALSE) + +target_sources(pluginval_discrete_params PRIVATE + DiscreteParamsPlugin.cpp + DiscreteParamsPlugin.h) + +target_compile_features(pluginval_discrete_params PRIVATE cxx_std_20) + +target_compile_definitions(pluginval_discrete_params PRIVATE + JUCE_WEB_BROWSER=0 + JUCE_USE_CURL=0 + JUCE_VST3_CAN_REPLACE_VST2=0) + +target_link_libraries(pluginval_discrete_params PRIVATE + juce::juce_audio_utils + juce::juce_audio_plugin_client + juce::juce_recommended_config_flags + juce::juce_recommended_warning_flags) diff --git a/tests/test_plugins/discrete_params/DiscreteParamsPlugin.cpp b/tests/test_plugins/discrete_params/DiscreteParamsPlugin.cpp new file mode 100644 index 0000000..04bcb00 --- /dev/null +++ b/tests/test_plugins/discrete_params/DiscreteParamsPlugin.cpp @@ -0,0 +1,65 @@ +/*============================================================================== + + Copyright 2018 by Tracktion Corporation. + For more information visit www.tracktion.com + + You may also use this code under the terms of the GPL v3 (see + www.gnu.org/licenses). + + pluginval IS PROVIDED "AS IS" WITHOUT ANY WARRANTY, AND ALL WARRANTIES, WHETHER + EXPRESSED OR IMPLIED, INCLUDING MERCHANTABILITY AND FITNESS FOR PURPOSE, ARE + DISCLAIMED. + + ==============================================================================*/ + +#include "DiscreteParamsPlugin.h" + +//============================================================================== +DiscreteParamsProcessor::DiscreteParamsProcessor() + : juce::AudioProcessor (BusesProperties().withInput ("Input", juce::AudioChannelSet::stereo(), true) + .withOutput ("Output", juce::AudioChannelSet::stereo(), true)) +{ + for (int i = 1; i <= 3; ++i) + { + bools.push_back (new juce::AudioParameterBool (juce::ParameterID ("bool" + juce::String (i), 1), + "Bool " + juce::String (i), false)); + addParameter (bools.back()); + } + + for (int i = 1; i <= 2; ++i) + { + choices.push_back (new juce::AudioParameterChoice (juce::ParameterID ("choice" + juce::String (i), 1), + "Choice " + juce::String (i), + { "One", "Two", "Three", "Four" }, 0)); + addParameter (choices.back()); + } +} + +//============================================================================== +void DiscreteParamsProcessor::getStateInformation (juce::MemoryBlock& destData) +{ + juce::MemoryOutputStream mos (destData, false); + + for (auto p : bools) + mos.writeBool (p->get()); + + for (auto p : choices) + mos.writeInt (p->getIndex()); +} + +void DiscreteParamsProcessor::setStateInformation (const void* data, int sizeInBytes) +{ + juce::MemoryInputStream mis (data, (size_t) sizeInBytes, false); + + for (auto p : bools) + *p = mis.readBool(); + + for (auto p : choices) + *p = mis.readInt(); +} + +//============================================================================== +juce::AudioProcessor* JUCE_CALLTYPE createPluginFilter() +{ + return new DiscreteParamsProcessor(); +} diff --git a/tests/test_plugins/discrete_params/DiscreteParamsPlugin.h b/tests/test_plugins/discrete_params/DiscreteParamsPlugin.h new file mode 100644 index 0000000..a296f9e --- /dev/null +++ b/tests/test_plugins/discrete_params/DiscreteParamsPlugin.h @@ -0,0 +1,68 @@ +/*============================================================================== + + Copyright 2018 by Tracktion Corporation. + For more information visit www.tracktion.com + + You may also use this code under the terms of the GPL v3 (see + www.gnu.org/licenses). + + pluginval IS PROVIDED "AS IS" WITHOUT ANY WARRANTY, AND ALL WARRANTIES, WHETHER + EXPRESSED OR IMPLIED, INCLUDING MERCHANTABILITY AND FITNESS FOR PURPOSE, ARE + DISCLAIMED. + + ==============================================================================*/ + +#pragma once + +#include + +//============================================================================== +/** + A minimal pass-through plugin with only discrete parameters (three bools + and two choices), used to dogfood the "Discrete parameter state restoration" + test. + + It saves and restores its state the way most plugins do: as the discrete + values (bool / index), not the normalised floats the host sent. +*/ +class DiscreteParamsProcessor : public juce::AudioProcessor +{ +public: + //============================================================================== + DiscreteParamsProcessor(); + ~DiscreteParamsProcessor() override = default; + + //============================================================================== + void prepareToPlay (double, int) override {} + void releaseResources() override {} + void processBlock (juce::AudioBuffer&, juce::MidiBuffer&) override {} + + //============================================================================== + juce::AudioProcessorEditor* createEditor() override { return nullptr; } + bool hasEditor() const override { return false; } + + //============================================================================== + const juce::String getName() const override { return "pluginval Discrete Params"; } + bool acceptsMidi() const override { return false; } + bool producesMidi() const override { return false; } + bool isMidiEffect() const override { return false; } + double getTailLengthSeconds() const override { return 0.0; } + + //============================================================================== + int getNumPrograms() override { return 1; } + int getCurrentProgram() override { return 0; } + void setCurrentProgram (int) override {} + const juce::String getProgramName (int) override { return "Default"; } + void changeProgramName (int, const juce::String&) override {} + + //============================================================================== + void getStateInformation (juce::MemoryBlock&) override; + void setStateInformation (const void*, int) override; + +private: + //============================================================================== + std::vector bools; + std::vector choices; + + JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (DiscreteParamsProcessor) +};