Skip to content
Closed
Show file tree
Hide file tree
Changes from 11 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
113 changes: 104 additions & 9 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ if(NOT PORTAUDIO_FOUND)
"${CMAKE_SOURCE_DIR}/external/portaudio/lib")
endif()

# Ensure PORTAUDIO_LIBRARIES is a full path when possible (helps macOS Homebrew installs)
if(PORTAUDIO_LIBRARIES)
if(NOT IS_ABSOLUTE "${PORTAUDIO_LIBRARIES}")
find_library(PORTAUDIO_LIB_FULL NAMES portaudio PATHS /opt/homebrew/lib /usr/local/lib /usr/lib)
if(PORTAUDIO_LIB_FULL)
set(PORTAUDIO_LIBRARIES ${PORTAUDIO_LIB_FULL} CACHE STRING "PortAudio library full path" FORCE)
endif()
endif()
endif()

# SDL2
if(PkgConfig_FOUND)
pkg_check_modules(SDL2 sdl2)
Expand All @@ -91,6 +101,21 @@ if(NOT SDL2_FOUND)
endif()
endif()

# pkg_check_modules can yield a bare library name (e.g. "SDL2") and a separate
# library directory. Add that directory explicitly on Homebrew/macOS builds and
# prefer a full path when available so the linker does not need to guess.
if(SDL2_LIBRARY_DIRS)
link_directories(${SDL2_LIBRARY_DIRS})
endif()
if(SDL2_LIBRARIES)
if(NOT IS_ABSOLUTE "${SDL2_LIBRARIES}")
find_library(SDL2_LIB_FULL NAMES SDL2 PATHS /opt/homebrew/lib /usr/local/lib /usr/lib)
if(SDL2_LIB_FULL)
set(SDL2_LIBRARIES ${SDL2_LIB_FULL} CACHE STRING "SDL2 library full path" FORCE)
endif()
endif()
endif()

# RtMidi (MIDI input)
# Skip auto-detection when paths are provided explicitly (e.g. CI -D flags).
if(NOT RTMIDI_LIBRARIES)
Expand All @@ -108,6 +133,17 @@ if(NOT RTMIDI_LIBRARIES)
"${CMAKE_SOURCE_DIR}/external/rtmidi")
endif()
endif()
if(RTMIDI_LIBRARY_DIRS)
link_directories(${RTMIDI_LIBRARY_DIRS})
endif()
if(RTMIDI_LIBRARIES)
if(NOT IS_ABSOLUTE "${RTMIDI_LIBRARIES}")
find_library(RTMIDI_LIB_FULL NAMES rtmidi PATHS /opt/homebrew/lib /usr/local/lib /usr/lib)
if(RTMIDI_LIB_FULL)
set(RTMIDI_LIBRARIES ${RTMIDI_LIB_FULL} CACHE STRING "RtMidi library full path" FORCE)
endif()
endif()
endif()
# pkg_check_modules sets RTMIDI_LIBRARIES to just the name ("rtmidi") and
# RTMIDI_LIBRARY_DIRS to the directory (/opt/homebrew/lib). Without
# link_directories the linker never searches that path, so we add it
Expand Down Expand Up @@ -213,18 +249,54 @@ endif()
# --- Platform-specific sources (selected at configure time) ---
# Mobile (Android/iOS) and Web use SDL audio backend + stub file dialog
# Android 8+ uses Oboe (AAudio exclusive mode) for low-latency audio.
if(UNIX AND NOT APPLE)
set(DEFAULT_WITH_JACK ON)
else()
set(DEFAULT_WITH_JACK OFF)
endif()
option(WITH_JACK "Build with JACK audio backend support" ${DEFAULT_WITH_JACK})

if(EMSCRIPTEN OR IOS)
set(BACKEND_SOURCES src/audio/audio_backend_sdl.cpp)
set(DIALOG_SOURCES src/gui/file_dialog_web.cpp)
elseif(ANDROID)
set(BACKEND_SOURCES src/audio/audio_backend_oboe.cpp)
set(DIALOG_SOURCES src/gui/file_dialog_web.cpp)
else()
set(BACKEND_SOURCES
src/audio/audio_backend_portaudio.cpp
src/audio/audio_backend_portaudio_lifecycle.cpp
src/audio/audio_backend_portaudio_devices.cpp
)
if(WITH_JACK)
if(NOT JACK_INCLUDE_DIRS OR NOT JACK_LIBRARIES)
find_package(PkgConfig QUIET)
if(PkgConfig_FOUND)
pkg_check_modules(JACK QUIET jack)
endif()
if(NOT JACK_FOUND)
find_path(JACK_INCLUDE_DIRS jack/jack.h
PATHS /usr/include /usr/local/include /opt/homebrew/include
"C:/Program Files/JACK/include"
"${CMAKE_SOURCE_DIR}/external/jack")
find_library(JACK_LIBRARIES NAMES jack
PATHS /usr/lib /usr/local/lib /opt/homebrew/lib
"C:/Program Files/JACK/lib"
"${CMAKE_SOURCE_DIR}/external/jack")
endif()
endif()
if(NOT JACK_INCLUDE_DIRS OR NOT JACK_LIBRARIES)
message(FATAL_ERROR "WITH_JACK=ON but JACK development files were not found")
endif()
if(JACK_LIBRARY_DIRS)
link_directories(${JACK_LIBRARY_DIRS})
endif()
set(BACKEND_SOURCES
src/audio/audio_backend_jack.cpp
src/audio/audio_backend_jack_lifecycle.cpp
)
else()
set(BACKEND_SOURCES
src/audio/audio_backend_portaudio.cpp
src/audio/audio_backend_portaudio_lifecycle.cpp
src/audio/audio_backend_portaudio_devices.cpp
)
endif()
Comment on lines +265 to +298

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and examine the CMakeLists.txt file
git ls-files | grep -i cmake

Repository: sudip-mondal-2002/Amplitron

Length of output: 86


🏁 Script executed:

# Read the CMakeLists.txt to understand the context around lines 265-295
wc -l CMakeLists.txt

Repository: sudip-mondal-2002/Amplitron

Length of output: 90


🏁 Script executed:

# Read the relevant sections mentioned in the review
sed -n '260,300p' CMakeLists.txt

Repository: sudip-mondal-2002/Amplitron

Length of output: 1663


🏁 Script executed:

# Also check the other locations mentioned: 517-518, 529-530, 535-536, 642-643, 652-654, 659-660
sed -n '510,540p' CMakeLists.txt

Repository: sudip-mondal-2002/Amplitron

Length of output: 1081


🏁 Script executed:

# Check the target_include_directories and target_link_libraries usage
sed -n '630,670p' CMakeLists.txt

Repository: sudip-mondal-2002/Amplitron

Length of output: 1419


🏁 Script executed:

# Check all the "Also applies to" locations mentioned
echo "=== Lines 517-518 ==="
sed -n '515,520p' CMakeLists.txt
echo -e "\n=== Lines 529-530 ==="
sed -n '527,532p' CMakeLists.txt
echo -e "\n=== Lines 535-536 ==="
sed -n '533,538p' CMakeLists.txt
echo -e "\n=== Lines 642-643 ==="
sed -n '640,645p' CMakeLists.txt
echo -e "\n=== Lines 652-654 ==="
sed -n '650,656p' CMakeLists.txt
echo -e "\n=== Lines 659-660 ==="
sed -n '657,662p' CMakeLists.txt

Repository: sudip-mondal-2002/Amplitron

Length of output: 1486


🏁 Script executed:

# Search for all WITH_JACK usages throughout the file
rg -n "WITH_JACK" CMakeLists.txt

Repository: sudip-mondal-2002/Amplitron

Length of output: 655


🏁 Script executed:

# Check if JACK libraries are actually used in compilation targets to understand impact
grep -n "add_executable\|add_library\|target_sources" CMakeLists.txt | head -20

Repository: sudip-mondal-2002/Amplitron

Length of output: 556


🏁 Script executed:

# Check PORTAUDIO handling for comparison
sed -n '200,265p' CMakeLists.txt

Repository: sudip-mondal-2002/Amplitron

Length of output: 2196


🏁 Script executed:

# Check if there are validation checks after find_package/pkg_check_modules for other libraries
rg -A 10 "find_package|pkg_check_modules" CMakeLists.txt | head -100

Repository: sudip-mondal-2002/Amplitron

Length of output: 4174


🏁 Script executed:

# Search for any checks of PORTAUDIO_FOUND or similar validation after find operations
rg -B 5 -A 5 "if\(NOT.*FOUND" CMakeLists.txt

Repository: sudip-mondal-2002/Amplitron

Length of output: 1807


🏁 Script executed:

# Final check: verify that JACK source files are included unconditionally when WITH_JACK is true
sed -n '265,295p' CMakeLists.txt | grep -A 5 "if(WITH_JACK)"

Repository: sudip-mondal-2002/Amplitron

Length of output: 290


🏁 Script executed:

# Verify PORTAUDIO is always required (never disabled like JACK should be)
rg -B 3 -A 3 "PORTAUDIO_FOUND\|if.*PORTAUDIO" CMakeLists.txt | head -30

Repository: sudip-mondal-2002/Amplitron

Length of output: 53


Add fallback logic when JACK is requested but not found to prevent compilation failure.

When WITH_JACK=ON but the JACK libraries are not found on the system, the code still sets BACKEND_SOURCES to compile JACK audio backend sources without verifying that JACK_INCLUDE_DIRS and JACK_LIBRARIES were successfully resolved. This causes compilation to fail when the jack/jack.h header is missing, rather than gracefully falling back to the PortAudio backend.

The current code checks if(NOT JACK_INCLUDE_DIRS OR NOT JACK_LIBRARIES) to conditionally search for JACK, but lacks validation after the find operations to confirm they succeeded. Add a check after the pkg-config and find_path/find_library attempts: if JACK is still not found, disable WITH_JACK with a warning message and re-evaluate BACKEND_SOURCES accordingly.

Suggested CMake fix
 if(WITH_JACK)
     if(NOT JACK_INCLUDE_DIRS OR NOT JACK_LIBRARIES)
         find_package(PkgConfig QUIET)
         if(PkgConfig_FOUND)
             pkg_check_modules(JACK QUIET jack)
         endif()
         if(NOT JACK_FOUND)
             find_path(JACK_INCLUDE_DIRS jack/jack.h
                 PATHS /usr/include /usr/local/include /opt/homebrew/include
                       "C:/Program Files/JACK/include"
                       "${CMAKE_SOURCE_DIR}/external/jack")
             find_library(JACK_LIBRARIES NAMES jack
                 PATHS /usr/lib /usr/local/lib /opt/homebrew/lib
                       "C:/Program Files/JACK/lib"
                       "${CMAKE_SOURCE_DIR}/external/jack")
         endif()
     endif()
+    if(NOT JACK_INCLUDE_DIRS OR NOT JACK_LIBRARIES)
+        message(WARNING "WITH_JACK=ON but JACK not found; falling back to PortAudio backend.")
+        set(WITH_JACK OFF CACHE BOOL "Build with JACK audio backend support" FORCE)
+    endif()
+endif()
+
+if(WITH_JACK)
+    if(JACK_LIBRARY_DIRS)
+        link_directories(${JACK_LIBRARY_DIRS})
+    endif()
+    set(BACKEND_SOURCES
+        src/audio/audio_backend_jack.cpp
+        src/audio/audio_backend_jack_lifecycle.cpp
+    )
+else()
+    set(BACKEND_SOURCES
+        src/audio/audio_backend_portaudio.cpp
+        src/audio/audio_backend_portaudio_lifecycle.cpp
+        src/audio/audio_backend_portaudio_devices.cpp
+    )
+endif()
-
-    if(JACK_LIBRARY_DIRS)
-        link_directories(${JACK_LIBRARY_DIRS})
-    endif()
-    set(BACKEND_SOURCES
-        src/audio/audio_backend_jack.cpp
-        src/audio/audio_backend_jack_lifecycle.cpp
-    )
-else()
-    set(BACKEND_SOURCES
-        src/audio/audio_backend_portaudio.cpp
-        src/audio/audio_backend_portaudio_lifecycle.cpp
-        src/audio/audio_backend_portaudio_devices.cpp
-    )
-endif()

Per coding guidelines, external libraries should validate successful resolution and fallback when not available—this change ensures WITH_JACK complies with that pattern.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if(WITH_JACK)
if(NOT JACK_INCLUDE_DIRS OR NOT JACK_LIBRARIES)
find_package(PkgConfig QUIET)
if(PkgConfig_FOUND)
pkg_check_modules(JACK QUIET jack)
endif()
if(NOT JACK_FOUND)
find_path(JACK_INCLUDE_DIRS jack/jack.h
PATHS /usr/include /usr/local/include /opt/homebrew/include
"C:/Program Files/JACK/include"
"${CMAKE_SOURCE_DIR}/external/jack")
find_library(JACK_LIBRARIES NAMES jack
PATHS /usr/lib /usr/local/lib /opt/homebrew/lib
"C:/Program Files/JACK/lib"
"${CMAKE_SOURCE_DIR}/external/jack")
endif()
endif()
if(JACK_LIBRARY_DIRS)
link_directories(${JACK_LIBRARY_DIRS})
endif()
set(BACKEND_SOURCES
src/audio/audio_backend_jack.cpp
src/audio/audio_backend_jack_lifecycle.cpp
)
else()
set(BACKEND_SOURCES
src/audio/audio_backend_portaudio.cpp
src/audio/audio_backend_portaudio_lifecycle.cpp
src/audio/audio_backend_portaudio_devices.cpp
)
endif()
if(WITH_JACK)
if(NOT JACK_INCLUDE_DIRS OR NOT JACK_LIBRARIES)
find_package(PkgConfig QUIET)
if(PkgConfig_FOUND)
pkg_check_modules(JACK QUIET jack)
endif()
if(NOT JACK_FOUND)
find_path(JACK_INCLUDE_DIRS jack/jack.h
PATHS /usr/include /usr/local/include /opt/homebrew/include
"C:/Program Files/JACK/include"
"${CMAKE_SOURCE_DIR}/external/jack")
find_library(JACK_LIBRARIES NAMES jack
PATHS /usr/lib /usr/local/lib /opt/homebrew/lib
"C:/Program Files/JACK/lib"
"${CMAKE_SOURCE_DIR}/external/jack")
endif()
endif()
if(NOT JACK_INCLUDE_DIRS OR NOT JACK_LIBRARIES)
message(WARNING "WITH_JACK=ON but JACK not found; falling back to PortAudio backend.")
set(WITH_JACK OFF CACHE BOOL "Build with JACK audio backend support" FORCE)
endif()
endif()
if(WITH_JACK)
if(JACK_LIBRARY_DIRS)
link_directories(${JACK_LIBRARY_DIRS})
endif()
set(BACKEND_SOURCES
src/audio/audio_backend_jack.cpp
src/audio/audio_backend_jack_lifecycle.cpp
)
else()
set(BACKEND_SOURCES
src/audio/audio_backend_portaudio.cpp
src/audio/audio_backend_portaudio_lifecycle.cpp
src/audio/audio_backend_portaudio_devices.cpp
)
endif()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CMakeLists.txt` around lines 265 - 295, The WITH_JACK branch sets
BACKEND_SOURCES to JACK files before confirming JACK was actually found; after
the pkg_check_modules / find_path / find_library attempts check whether
JACK_INCLUDE_DIRS and JACK_LIBRARIES (or JACK_FOUND) are set, and if they are
still missing, emit a warning via message(WARNING ...) and set WITH_JACK to OFF
(or clear the JACK-specific vars), then fall back to the PortAudio sources by
re-evaluating BACKEND_SOURCES; ensure you reference and update the existing
symbols (WITH_JACK, JACK_INCLUDE_DIRS, JACK_LIBRARIES, JACK_FOUND,
BACKEND_SOURCES) so compilation will use portaudio if JACK resolution fails.

set(DIALOG_SOURCES
src/gui/file_dialog_native.cpp
src/gui/file_dialog_native_open.cpp
Expand Down Expand Up @@ -446,6 +518,7 @@ else() # NOT EMSCRIPTEN, ANDROID, or IOS
${IMGUI_DIR}
${IMGUI_DIR}/backends
${PORTAUDIO_INCLUDE_DIRS}
$<$<BOOL:${WITH_JACK}>:${JACK_INCLUDE_DIRS}>
${SDL2_INCLUDE_DIRS}
${OPENGL_INCLUDE_DIR}
${RTMIDI_INCLUDE_DIRS}
Expand All @@ -457,11 +530,13 @@ else() # NOT EMSCRIPTEN, ANDROID, or IOS
target_compile_definitions(Amplitron PRIVATE
AMPLITRON_VERSION="${AMPLITRON_VERSION}"
AMPLITRON_HAS_MIDI
$<$<BOOL:${WITH_JACK}>:WITH_JACK>
)

target_link_libraries(Amplitron PRIVATE
nlohmann_json::nlohmann_json
${PORTAUDIO_LIBRARIES}
$<$<BOOL:${WITH_JACK}>:${JACK_LIBRARIES}>
${SDL2_LIBRARIES}
${OPENGL_LIBRARIES}
${RTMIDI_LIBRARIES}
Expand Down Expand Up @@ -500,6 +575,10 @@ else() # NOT EMSCRIPTEN, ANDROID, or IOS
tests/test_gui_manager.cpp
)

if(WITH_JACK)
list(APPEND TEST_SOURCES tests/test_jack_backend.cpp)
endif()

# Effect + core sources needed by tests (no GUI, no main)
set(CORE_SOURCES
src/preset_manager.cpp
Expand All @@ -510,10 +589,21 @@ else() # NOT EMSCRIPTEN, ANDROID, or IOS
src/audio/audio_engine_chain.cpp
src/audio/audio_engine_process.cpp
src/audio/audio_engine_api.cpp
src/audio/audio_backend_portaudio.cpp
src/audio/audio_backend_portaudio_lifecycle.cpp
src/audio/audio_backend_portaudio_devices.cpp
src/audio/recorder.cpp
)

if(WITH_JACK)
list(APPEND CORE_SOURCES
src/audio/audio_backend_jack.cpp
src/audio/audio_backend_jack_lifecycle.cpp
)
else()
list(APPEND CORE_SOURCES
src/audio/audio_backend_portaudio.cpp
src/audio/audio_backend_portaudio_lifecycle.cpp
src/audio/audio_backend_portaudio_devices.cpp
)
endif()
list(APPEND CORE_SOURCES src/audio/recorder.cpp
src/audio/recorder_session.cpp
src/audio/recorder_io.cpp
src/audio/recorder_wav.cpp
Expand Down Expand Up @@ -557,6 +647,7 @@ else() # NOT EMSCRIPTEN, ANDROID, or IOS
${IMGUI_DIR}
${IMGUI_DIR}/backends
${PORTAUDIO_INCLUDE_DIRS}
$<$<BOOL:${WITH_JACK}>:${JACK_INCLUDE_DIRS}>
${SDL2_INCLUDE_DIRS}
${OPENGL_INCLUDE_DIR}
${RTMIDI_INCLUDE_DIRS}
Expand All @@ -566,10 +657,14 @@ else() # NOT EMSCRIPTEN, ANDROID, or IOS
# DEFINE AMPLITRON_HAS_MIDI FOR TEST BUILDS
# ============================================================
target_compile_definitions(amplitron-tests PRIVATE AMPLITRON_HAS_MIDI)
if(WITH_JACK)
target_compile_definitions(amplitron-tests PRIVATE WITH_JACK=1)
endif()

target_link_libraries(amplitron-tests PRIVATE
nlohmann_json::nlohmann_json
${PORTAUDIO_LIBRARIES}
$<$<BOOL:${WITH_JACK}>:${JACK_LIBRARIES}>
${SDL2_LIBRARIES}
${OPENGL_LIBRARIES}
${RTMIDI_LIBRARIES}
Expand Down
6 changes: 4 additions & 2 deletions scripts/setup_dependencies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ install_deps() {
build-essential cmake pkg-config \
libportaudio2 portaudio19-dev \
libsdl2-dev \
libgl1-mesa-dev
libgl1-mesa-dev \
libjack-jackd2-dev
elif command -v dnf &> /dev/null; then
echo "Detected Fedora/RHEL. Installing dependencies..."
sudo dnf install -y \
Expand All @@ -92,7 +93,8 @@ install_deps() {
mesa
elif command -v brew &> /dev/null; then
echo "Detected macOS with Homebrew. Installing dependencies..."
brew install cmake portaudio sdl2
brew update
brew install cmake portaudio rtmidi sdl2 pkg-config
else
echo "WARNING: Could not detect package manager."
echo "Please install manually: cmake, portaudio, sdl2, opengl dev headers"
Expand Down
19 changes: 19 additions & 0 deletions src/audio/audio_backend_jack.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Minimal JACK backend factory and helpers
#include "audio/audio_backend.h"
#include "audio/audio_backend_jack_internal.h"
#include <iostream>

namespace Amplitron
{

AudioBackendState *create_audio_backend()
{
return new AudioBackendState();
}

void destroy_audio_backend(AudioBackendState *state)
{
delete state;
}

} // namespace Amplitron
18 changes: 18 additions & 0 deletions src/audio/audio_backend_jack_internal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#pragma once

#ifdef WITH_JACK
#include <jack/jack.h>

namespace Amplitron
{

struct AudioBackendState
{
jack_client_t *client = nullptr;
jack_port_t *input_port = nullptr;
jack_port_t *output_port = nullptr;
int measured_latency_ms = 0;
};

} // namespace Amplitron
#endif // WITH_JACK
Loading
Loading