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.

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

Update Region.cpp to support per-voice CC's #1173

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 8 additions & 28 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,8 @@ env:
# shell: pwsh on windows, bash all the rest
# working-directory: ${{ github.workspace }}

# FIXME:
# Tests disabled (again); they were working one day and broken the day after
# without changes: ./build/tests/sfizz_tests: No such file or directory
# It didn't work either by specifying the full path (that was OK in previous builds)
jobs:
clang_tidy:
# if: ${{ false }}
name: Clang Tidy
runs-on: ubuntu-20.04
steps:
Expand All @@ -40,7 +35,6 @@ jobs:
run: ./scripts/run_clang_tidy.sh

build_for_linux:
# if: ${{ false }}
name: Linux Ubuntu 22.04
runs-on: ubuntu-22.04 # abseil (libabsl) is not available in 20.04 repository
env:
Expand Down Expand Up @@ -83,7 +77,6 @@ jobs:
echo "-- github.workspace: ${{ github.workspace }}"
echo "-- runner.workspace: ${{ runner.workspace }}"
- name: Build tests
if: ${{ false }}
run: |
options=(
--build build
Expand All @@ -94,13 +87,9 @@ jobs:
)
cmake "${options[@]}"
- name: Run tests
if: ${{ false }}
run: |
./build/tests/sfizz_tests
options=(
--build-config ${{ env.build_type }}
--debug
--extra-verbose
--output-on-failure
--parallel 2
--test-dir build
Expand Down Expand Up @@ -129,7 +118,6 @@ jobs:
path: "${{ github.workspace }}/${{ env.install_name }}.tar.gz"

build_for_macos:
# if: ${{ false }}
name: macOS 11
runs-on: macos-11
env:
Expand Down Expand Up @@ -161,7 +149,6 @@ jobs:
echo "-- runner.workspace: ${{ runner.workspace }}"
echo "-- github.workspace: ${{ github.workspace }}"
- name: Build tests
if: ${{ false }}
run: |
options=(
--build build
Expand All @@ -172,13 +159,9 @@ jobs:
)
cmake "${options[@]}"
- name: Run tests
if: ${{ false }}
run: |
./build/tests/sfizz_tests
options=(
--build-config ${{ env.build_type }}
--debug
--extra-verbose
--output-on-failure
--parallel 2
--test-dir build
Expand Down Expand Up @@ -207,7 +190,6 @@ jobs:
path: "${{ github.workspace }}/${{ env.install_name }}.tar.gz"

build_for_windows:
# if: ${{ false }}
name: Windows 2019
runs-on: windows-2019
strategy:
Expand Down Expand Up @@ -240,7 +222,6 @@ jobs:
echo "-- runner.workspace: ${{ runner.workspace }}"
echo "-- github.workspace: ${{ github.workspace }}"
- name: Build tests
if: ${{ false }}
run: |
cmake `
--build build `
Expand All @@ -249,13 +230,9 @@ jobs:
--target sfizz_tests `
--verbose `
- name: Run tests
if: ${{ false }}
run: |
.\build\tests\${{ env.build_type }}\sfizz_tests.exe
ctest `
--build-config ${{ env.build_type }} `
--debug `
--extra-verbose `
--output-on-failure `
--parallel 2 `
--test-dir build `
Expand Down Expand Up @@ -304,7 +281,6 @@ jobs:
path: "${{ github.workspace }}/${{ env.install_name }}.tar.gz"

build_with_libsndfile:
# if: ${{ false }}
name: Linux libsndfile
runs-on: ubuntu-20.04
steps:
Expand Down Expand Up @@ -332,7 +308,6 @@ jobs:
)
cmake "${options[@]}"
- name: Build tests
if: ${{ false }}
run: |
options=(
--build build
Expand All @@ -343,11 +318,16 @@ jobs:
)
cmake "${options[@]}"
- name: Run tests
if: ${{ false }}
run: ./build/tests/sfizz_tests
run: |
options=(
--build-config ${{ env.build_type }}
--output-on-failure
--parallel 2
--test-dir build
)
ctest "${options[@]}"

build_with_makefile:
# if: ${{ false }}
name: Linux makefile
runs-on: ubuntu-20.04
steps:
Expand Down
6 changes: 3 additions & 3 deletions cmake/BuildType.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
message(STATUS "Setting build type to '${default_build_type}' as none was specified.")
set(CMAKE_BUILD_TYPE "${default_build_type}" CACHE
STRING "Choose the type of build." FORCE)
endif()

# Set the possible values of build type for cmake-gui
set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release"
# Set the possible values of build type for cmake-gui
set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release"
"MinSizeRel" "RelWithDebInfo")
endif()
4 changes: 2 additions & 2 deletions cmake/SfizzConfig.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ endif()

# Process sources as UTF-8
if(MSVC)
add_compile_options("/utf-8")
add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:/utf-8>)
endif()

# Define the math constants everywhere
Expand Down Expand Up @@ -104,7 +104,7 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
endif()
endif()
elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
add_compile_options(/Zc:__cplusplus)
add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:/Zc:__cplusplus>)
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
endif()

Expand Down
15 changes: 13 additions & 2 deletions external/st_audiofile/src/st_audiofile.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
#include "st_audiofile_libs.h"
#include <stdlib.h>

#ifdef _WIN32
#include <windows.h>
#endif
#define WAVPACK_MEMORY_ASSUMED_VERSION 5

struct st_audio_file {
Expand Down Expand Up @@ -179,10 +182,18 @@ static st_audio_file* st_generic_open_file(const void* filename, int widepath)

// Try WV
{
af->wv =
#if defined(_WIN32)
WavpackOpenFileInput((const char*)filename, NULL, OPEN_FILE_UTF8, 0);
// WavPack expects an UTF8 input and has no widechar api, so we convert the filename back...
unsigned wsize = wcslen(filename);
unsigned size = WideCharToMultiByte(CP_UTF8, 0, filename, wsize, NULL, 0, NULL, NULL);
char *buffer = (char*)malloc((size+1) * sizeof(char));
WideCharToMultiByte(CP_UTF8, 0, filename, wsize, buffer, size, NULL, NULL);
buffer[size] = '\0';
af->wv =
WavpackOpenFileInput(buffer, NULL, OPEN_FILE_UTF8, 0);
free(buffer);
#else
af->wv =
WavpackOpenFileInput((const char*)filename, NULL, 0, 0);
#endif
if (af->wv) {
Expand Down
15 changes: 0 additions & 15 deletions src/Config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,6 @@

namespace sfz {

enum ExtendedCCs {
pitchBend = 128,
channelAftertouch = 129,
polyphonicAftertouch = 130,
noteOnVelocity = 131,
noteOffVelocity = 132,
keyboardNoteNumber = 133,
keyboardNoteGate = 134,
unipolarRandom = 135,
bipolarRandom = 136,
alternate = 137,
keydelta = 140,
absoluteKeydelta = 141,
};

namespace config {
constexpr float defaultSampleRate { 48000 };
constexpr float maxSampleRate { 192000 };
Expand Down
15 changes: 0 additions & 15 deletions src/sfizz/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,6 @@

namespace sfz {

enum ExtendedCCs {
pitchBend = 128,
channelAftertouch = 129,
polyphonicAftertouch = 130,
noteOnVelocity = 131,
noteOffVelocity = 132,
keyboardNoteNumber = 133,
keyboardNoteGate = 134,
unipolarRandom = 135,
bipolarRandom = 136,
alternate = 137,
keydelta = 140,
absoluteKeydelta = 141,
};

namespace config {
constexpr float defaultSampleRate { 48000 };
constexpr float maxSampleRate { 192000 };
Expand Down
2 changes: 1 addition & 1 deletion src/sfizz/FilePool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ void sfz::FilePool::loadingJob(const QueuedFileData& data) noexcept
AudioReaderPtr reader = createAudioReader(file, id->isReverse(), &readError);

if (readError) {
DBG("[sfizz] libsndfile errored for " << *id << " with message " << readError.message());
DBG("[sfizz] reading the file errored for " << *id << " with code " << readError << ": " << readError.message());
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/sfizz/MidiState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ void sfz::MidiState::noteOnEvent(int delay, int noteNumber, float velocity) noex
ccEvent(delay, ExtendedCCs::unipolarRandom, unipolarDist(Random::randomGenerator));
ccEvent(delay, ExtendedCCs::bipolarRandom, bipolarDist(Random::randomGenerator));
ccEvent(delay, ExtendedCCs::keyboardNoteGate, activeNotes > 0 ? 1.0f : 0.0f);
ccEvent(delay, ExtendedCCs::keydelta, keydelta);
ccEvent(delay, ExtendedCCs::absoluteKeydelta, std::abs(keydelta));
ccEvent(delay, AriaExtendedCCs::keydelta, keydelta);
ccEvent(delay, AriaExtendedCCs::absoluteKeydelta, std::abs(keydelta));
activeNotes++;

ccEvent(delay, ExtendedCCs::alternate, alternate);
Expand Down
44 changes: 24 additions & 20 deletions src/sfizz/Region.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1690,8 +1690,13 @@ bool sfz::Region::processGenericCc(const Opcode& opcode, OpcodeSpec<float> spec,
// search an existing connection of same CC number and target
// if it exists, modify, otherwise create
auto it = std::find_if(connections.begin(), connections.end(),
[ccNumber, &target](const Connection& x) -> bool
[ccNumber, &target, this](const Connection& x) -> bool
{
if (ccModulationIsPerVoice(ccNumber))
return x.source.id() == ModId::PerVoiceController &&
x.source.region() == id &&
x.source.parameters().cc == ccNumber &&
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we check the region id here too in this case? If not, I'd suggest just modifying the initial check to x.source.id() == ModId::PerVoiceController || x.source.id() == ModId::Controller.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the critiques, Paul. Insofar as this code and the one with the corresponding suggestion, good thought about the region checking, I suppose. Beyond that technicality with this particular one, I'll let you be the judge over how to format the syntax exactly: I fully admit code optimization is not my forte.

Sorry for my delayed response, btw.

x.target == target;
return x.source.id() == ModId::Controller &&
x.source.parameters().cc == ccNumber &&
x.target == target;
Expand Down Expand Up @@ -1730,22 +1735,11 @@ bool sfz::Region::processGenericCc(const Opcode& opcode, OpcodeSpec<float> spec,
break;
}

switch (p.cc) {
case ExtendedCCs::noteOnVelocity: // fallthrough
case ExtendedCCs::noteOffVelocity: // fallthrough
case ExtendedCCs::keyboardNoteNumber: // fallthrough
case ExtendedCCs::keyboardNoteGate: // fallthrough
case ExtendedCCs::unipolarRandom: // fallthrough
case ExtendedCCs::bipolarRandom: // fallthrough
case ExtendedCCs::alternate:
case ExtendedCCs::keydelta:
case ExtendedCCs::absoluteKeydelta:
if (ccModulationIsPerVoice(p.cc)) {
conn->source = ModKey(ModId::PerVoiceController, id, p);
break;
default:
} else {
conn->source = ModKey(ModId::Controller, {}, p);
break;
}
}
}

return true;
Expand Down Expand Up @@ -1850,11 +1844,21 @@ sfz::Region::Connection& sfz::Region::getOrCreateConnection(const ModKey& source

sfz::Region::Connection* sfz::Region::getConnectionFromCC(int sourceCC, const ModKey& target)
{
for (sfz::Region::Connection& conn : connections) {
if (conn.source.id() == sfz::ModId::Controller && conn.target == target) {
auto p = conn.source.parameters();
if (p.cc == sourceCC)
return &conn;
if (ccModulationIsPerVoice(sourceCC)) {
for (sfz::Region::Connection& conn : connections) {
if (conn.source.id() == sfz::ModId::PerVoiceController && conn.target == target && conn.source.region() == id) {
const auto& p = conn.source.parameters();
if (p.cc == sourceCC)
return &conn;
}
}
} else {
for (sfz::Region::Connection& conn : connections) {
if (conn.source.id() == sfz::ModId::Controller && conn.target == target) {
const auto& p = conn.source.parameters();
if (p.cc == sourceCC)
return &conn;
}
}
}
return nullptr;
Expand Down
Loading