Skip to content
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: 36 additions & 0 deletions AestraAudio/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,42 @@ if(WIN32)
# Define "AestraAudio" as an alias to the platform library for main app linking
add_library(AestraAudio ALIAS AestraAudioWin)

elseif(APPLE)
# macOS - CoreAudio via RtAudio
set(AESTRA_AUDIO_MACOS_SOURCES
src/macOS/RtAudioDriver.cpp
)

set(AESTRA_AUDIO_MACOS_HEADERS
src/macOS/RtAudioDriver.h
include/Drivers/AudioPlatformRegistry.h
${RTAUDIO_HEADERS}
)

add_library(AestraAudioMacOS STATIC
${AESTRA_AUDIO_MACOS_SOURCES}
${AESTRA_AUDIO_MACOS_HEADERS}
${RTAUDIO_SOURCES}
)

target_include_directories(AestraAudioMacOS
PUBLIC
# Nothing public
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/include
${CMAKE_CURRENT_SOURCE_DIR}/src/macOS
${RTAUDIO_DIR}
)

target_link_libraries(AestraAudioMacOS
PUBLIC
AestraAudioCore
PRIVATE
${RTAUDIO_LIBS}
)

add_library(AestraAudio ALIAS AestraAudioMacOS)

Comment on lines +494 to +529
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.

Action required

1. No macos driver registry 🐞 Bug ✓ Correctness

On APPLE, AestraAudioMacOS builds only src/macOS/RtAudioDriver.cpp and does not compile any macOS
RegisterPlatformDrivers implementation, so AudioDeviceManager ends up calling the no-op stub and
registers zero real drivers on macOS (no device enumeration/streaming).
Agent Prompt
## Issue description
On macOS, `AudioDeviceManager::initialize()` calls `RegisterPlatformDrivers(*this)`, but the APPLE build of `AestraAudioMacOS` does not compile any macOS implementation of `RegisterPlatformDrivers`. The only available definition is the no-op stub, so zero real audio drivers are registered.

## Issue Context
Linux and Windows both compile a platform registry translation unit (e.g. `src/Linux/LinuxDriverRegistry.cpp`, `src/Win32/WindowsDriverRegistry.cpp`) that registers at least one real driver.

## Fix Focus Areas
- Add a macOS registry TU implementing `Aestra::Audio::RegisterPlatformDrivers(AudioDeviceManager&)` and (optionally) `GetPlatformAudioInfo()`.
- Ensure the macOS registry registers a driver that implements `IAudioDriver` (either a macOS RtAudio-based `IAudioDriver` implementation or reuse/adapt the existing Linux `RtAudioDriver` pattern).
- Add the new registry source file to the APPLE `AESTRA_AUDIO_MACOS_SOURCES` list.

### Fix Focus Areas (exact locations)
- AestraAudio/CMakeLists.txt[494-529]
- AestraAudio/src/Core/AudioDeviceManager.cpp[31-60]
- AestraAudio/src/Core/DriverRegistryStub.cpp[1-14]
- AestraAudio/src/Linux/LinuxDriverRegistry.cpp[14-24]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

elseif(UNIX)
# Non-Windows (Linux)
if(ALSA_FOUND)
Expand Down
5 changes: 3 additions & 2 deletions AestraAudio/include/AestraUUID.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <atomic>
#include <chrono>
#include <cstdint>
#include <cstdio>
#include <functional>
#include <random>
#include <string>
Expand Down Expand Up @@ -46,14 +47,14 @@ struct AestraUUID {
bool operator<=(const AestraUUID& other) const { return !(other < *this); }
bool operator>(const AestraUUID& other) const { return other < *this; }
bool operator>=(const AestraUUID& other) const { return !(*this < other); }

/**
* @brief Convert to string representation
*/
std::string toString() const {
// Simple hex representation
char buf[64];
snprintf(buf, sizeof(buf), "%016lx%016lx", high, low);
snprintf(buf, sizeof(buf), "%016llx%016llx", static_cast<unsigned long long>(high),
static_cast<unsigned long long>(low));
return std::string(buf);
}
};
Expand Down
11 changes: 6 additions & 5 deletions AestraAudio/include/Core/AutomationCurve.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#pragma once

#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <string>
#include <vector>
Expand All @@ -19,7 +20,7 @@ struct AutomationPoint {
float value{0.0f};
double beat{0.0}; // For serialization
float curve{0.0f}; // For serialization (curve tension)
bool selected{false}; // Selection state for UI
bool selected{false};
};

struct AutomationCurve {
Expand All @@ -30,6 +31,7 @@ struct AutomationCurve {
std::string name;
AutomationTarget target{AutomationTarget::Custom};
float defaultValue{0.0f};
bool visible{true};

AutomationCurve() = default;
AutomationCurve(const std::string& n, AutomationTarget t) : name(n), target(t) {}
Expand All @@ -52,9 +54,8 @@ struct AutomationCurve {
/**
* @brief Get all points
*/
const std::vector<AutomationPoint>& getPoints() const { return points; }

std::vector<AutomationPoint>& getPoints() { return points; }
const std::vector<AutomationPoint>& getPoints() const { return points; }

/**
* @brief Get interpolated value at a given beat position
Expand Down Expand Up @@ -131,7 +132,7 @@ struct AutomationCurve {

void removePoint(size_t index) {
if (index < points.size()) {
points.erase(points.begin() + index);
points.erase(points.begin() + static_cast<std::ptrdiff_t>(index));
}
}

Expand All @@ -141,7 +142,7 @@ struct AutomationCurve {
});
}

bool isVisible() const { return true; }
bool isVisible() const { return visible; }
};

} // namespace Audio
Expand Down
6 changes: 3 additions & 3 deletions AestraAudio/include/DSP/SampleRateConverter.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,9 @@ class SampleRateConverter {
SampleRateConverter(const SampleRateConverter&) = delete;
SampleRateConverter& operator=(const SampleRateConverter&) = delete;

// Move is allowed
SampleRateConverter(SampleRateConverter&&) = default;
SampleRateConverter& operator=(SampleRateConverter&&) = default;
// Atomic state prevents implicit moves.
SampleRateConverter(SampleRateConverter&&) = delete;
SampleRateConverter& operator=(SampleRateConverter&&) = delete;

// =========================================================================
// Configuration
Expand Down
6 changes: 3 additions & 3 deletions AestraAudio/include/IO/WaveformCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,11 @@ class WaveformCache {
WaveformCache();
~WaveformCache();

// Non-copyable, movable
// Non-copyable and non-movable because atomic state prevents implicit moves.
WaveformCache(const WaveformCache&) = delete;
WaveformCache& operator=(const WaveformCache&) = delete;
WaveformCache(WaveformCache&&) = default;
WaveformCache& operator=(WaveformCache&&) = default;
WaveformCache(WaveformCache&&) = delete;
WaveformCache& operator=(WaveformCache&&) = delete;

/**
* @brief Build cache from audio buffer
Expand Down
3 changes: 3 additions & 0 deletions AestraAudio/include/Models/ClipInstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ struct ClipEdits {
double sourceStart = 0.0;
};

using LocalEdits = ClipEdits;

Comment on lines +63 to +64
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.

🧹 Nitpick | 🔵 Trivial

Consider adding a comment explaining LocalEdits alias purpose.

If LocalEdits is intended for semantic distinction or future expansion, a brief comment would help maintainability. Otherwise, direct usage of ClipEdits may be clearer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@AestraAudio/include/Models/ClipInstance.h` around lines 63 - 64, Add a short
clarifying comment above the using alias LocalEdits = ClipEdits; explaining why
the alias exists (e.g., to convey semantic intent for per-instance/local edit
semantics or to reserve the name for future specialized behavior) and when to
prefer LocalEdits versus ClipEdits; reference the alias symbol LocalEdits and
the underlying type ClipEdits so reviewers can easily find and understand the
intention in ClipInstance.h.

/**
* @brief Represents a single clip instance on the playlist
*/
Expand All @@ -71,6 +73,7 @@ struct ClipInstance {
double startBeat = 0.0;
double durationBeats = 4.0;
double sourceOffset = 0.0; // Offset into source material
bool muted = false;

ClipEdits edits;
Comment on lines +76 to 78
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how both muted fields are used to understand intended semantics
rg -n '\.muted\b' --type=cpp -C 2 | head -80

# Check for any logic combining the two muted states
rg -n 'edits\.muted|instance.*muted' --type=cpp -C 3

Repository: currentsuspect/Aestra

Length of output: 5985


🏁 Script executed:

#!/bin/bash
# Check the ClipInstance.h header structure
cat -n AestraAudio/include/Models/ClipInstance.h | grep -A 10 -B 10 "muted\|ClipEdits"

Repository: currentsuspect/Aestra

Length of output: 1757


🏁 Script executed:

#!/bin/bash
# More thorough search for any usage of muted as a member of ClipInstance (not edits)
# Search patterns: obj.muted, instance.muted, clip.muted (not clip.edits.muted)
rg -n '\bmuted\b' --type=cpp -A 2 -B 2 | grep -v 'edits\.muted\|channel->muted\|info\.muted\|existing->muted' | head -100

Repository: currentsuspect/Aestra

Length of output: 6943


🏁 Script executed:

#!/bin/bash
# Search for any direct assignment or read of ClipInstance muted field
# Look for patterns like: instance.muted, clip.muted (not clip.edits.muted), ->muted (for pointers)
rg -n 'clip\s*\.\s*muted(?!\s*\.)|instance\s*\.\s*muted|->muted' --type=cpp | grep -v 'edits\|channel\|lane\|mixer\|info\|existing'

Repository: currentsuspect/Aestra

Length of output: 341


🏁 Script executed:

#!/bin/bash
# Search for ClipInstance constructor usage or initialization
ast-grep --pattern 'ClipInstance $$ = $$$' | head -50

Repository: currentsuspect/Aestra

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Simple search for clip.muted or instance.muted patterns
rg -n '\bclip\b.*\bmuted\b' --type=cpp | head -50

Repository: currentsuspect/Aestra

Length of output: 424


🏁 Script executed:

#!/bin/bash
# Alternative: search for assignment to muted field on ClipInstance context
rg -n '\bmuted\s*=' --type=cpp -B 3 | grep -A 3 -B 3 'ClipInstance\|clip\.' | head -80

Repository: currentsuspect/Aestra

Length of output: 1788


Remove unused instance-level muted field.

The instance-level muted field on line 76 is dead code—it is never read or written anywhere in the codebase. All actual mute state is managed through ClipInstance::edits.muted, which is used in serialization (ProjectSerializer.cpp), UI rendering (TrackUIComponent.cpp), and tests.

Remove the unused field to eliminate confusion and keep the codebase clean.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@AestraAudio/include/Models/ClipInstance.h` around lines 76 - 78, Remove the
dead instance-level field `muted` from the ClipInstance class: this field is
never read or written—mute state is stored in `ClipInstance::edits.muted` (type
`ClipEdits`). Delete the `bool muted = false;` declaration in ClipInstance and
ensure no remaining references exist (the serializer and UI already use
`edits.muted` as seen in ProjectSerializer and TrackUIComponent).


Expand Down
3 changes: 1 addition & 2 deletions AestraAudio/include/Models/ClipSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class ClipSource {
const std::string& getFilePath() const { return m_filePath; }

ClipSourceID getID() const { return m_id; }
std::shared_ptr<WaveformCache> getWaveformCache() const { return m_waveformCache; }

bool isValid() const { return m_buffer && m_buffer->isValid(); }

Expand All @@ -74,8 +75,6 @@ class ClipSource {
void setFilePath(const std::string& path) { m_filePath = path; }

void setBuffer(std::shared_ptr<AudioBufferData> buffer) { m_buffer = std::move(buffer); }

std::shared_ptr<WaveformCache> getWaveformCache() const { return m_waveformCache; }
void setWaveformCache(std::shared_ptr<WaveformCache> cache) { m_waveformCache = std::move(cache); }

private:
Expand Down
1 change: 0 additions & 1 deletion AestraAudio/include/Models/MeterSnapshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ struct MeterSnapshotBuffer {

// STUB: readMeter — Phase 2 will return actual metering data
MeterReadout readMeter(int slot) const { return {}; }

MeterReadout readSnapshot(int slot) const { return readMeter(slot); }
};

Expand Down
49 changes: 27 additions & 22 deletions AestraAudio/include/Models/PatternManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class PatternManager {
/**
* @brief Create a MIDI pattern
*/
PatternID createMidiPattern(const std::string& name, double lengthBeats, const MidiPayload& payload) {
PatternID createMidiPattern(const std::string& name, double lengthBeats, const MidiPayload& payload = MidiPayload{}) {
PatternID id{nextId++};
auto pattern = std::make_unique<PatternSource>();
pattern->id = id;
Expand All @@ -74,26 +74,6 @@ class PatternManager {
return id;
}

/**
* @brief Clone an existing pattern and return the new ID
*/
PatternID clonePattern(PatternID sourceId) {
auto* src = getPattern(sourceId);
if (!src) return PatternID{};
PatternID id{nextId++};
auto pattern = std::make_unique<PatternSource>(*src);
pattern->id = id;
m_patterns[id.value] = std::move(pattern);
return id;
}

/**
* @brief Remove a pattern by ID
*/
void removePattern(PatternID id) {
m_patterns.erase(id.value);
}

/**
* @brief Get or create a pattern
*/
Expand Down Expand Up @@ -126,10 +106,35 @@ class PatternManager {
fn(*pattern);
}

/**
* @brief Clone an existing pattern
*/
PatternID clonePattern(PatternID sourceId) {
auto* source = getPattern(sourceId);
if (!source) return PatternID{};

PatternID newId{nextId++};
auto pattern = std::make_unique<PatternSource>();
pattern->id = newId;
pattern->name = source->name + " (Copy)";
pattern->lengthBeats = source->lengthBeats;
pattern->type = source->type;
pattern->payload = source->payload;
m_patterns[newId.value] = std::move(pattern);
Comment on lines +112 to +123
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="$(fd '^PatternSource\.h$' | head -n1)"
if [[ -z "${file}" ]]; then
  echo "PatternSource.h not found" >&2
  exit 1
fi

echo "Inspecting ${file}"
sed -n '1,240p' "${file}"

Repository: currentsuspect/Aestra

Length of output: 2782


Clone silently drops the mixer channel assignment.

The clonePattern method does not copy m_mixerChannel, causing cloned patterns to lose their mixer routing. Add pattern->m_mixerChannel = source->m_mixerChannel; to preserve this state, or explicitly document why clones should start with a reset channel assignment.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@AestraAudio/include/Models/PatternManager.h` around lines 112 - 123, In
clonePattern (function PatternID clonePattern) the cloned PatternSource
currently doesn't preserve the source's mixer routing; update the clone creation
to copy source->m_mixerChannel into the new pattern (pattern->m_mixerChannel =
source->m_mixerChannel) so the cloned pattern retains its mixer assignment, or
alternatively add a clear comment in PatternSource/clonePattern explaining why
m_mixerChannel is intentionally reset.

return newId;
}

/**
* @brief Remove a pattern
*/
void removePattern(PatternID id) {
m_patterns.erase(id.value);
}
Comment on lines +130 to +132
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 | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "PatternManager.h" -type f

Repository: currentsuspect/Aestra

Length of output: 110


🏁 Script executed:

cat -n "AestraAudio/include/Models/PatternManager.h"

Repository: currentsuspect/Aestra

Length of output: 5040


🏁 Script executed:

rg "getAllPatterns" --type cpp --type h -B 2 -A 5

Repository: currentsuspect/Aestra

Length of output: 4009


🏁 Script executed:

rg "removePattern" --type cpp --type h -B 2 -A 5

Repository: currentsuspect/Aestra

Length of output: 1102


removePattern() introduces a use-after-free vulnerability in getAllPatterns().

getAllPatterns() returns std::shared_ptr<PatternSource> objects with no-op deleters, containing raw pointers to objects owned by the unique_ptr in m_patterns. When removePattern() erases the map entry, it deletes the underlying PatternSource object, but any retained shared_ptr instances still reference the freed memory. This is a critical memory safety issue.

The code comment at line 96 acknowledges this is "not ideal but works for shim," indicating the developers were aware of the design flaw.

Resolve by either:

  1. Backing m_patterns with shared_ptr instead of unique_ptr to share ownership, or
  2. Refactoring getAllPatterns() to return non-owning views (IDs or raw pointers) instead of shared_ptr.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@AestraAudio/include/Models/PatternManager.h` around lines 130 - 132,
removePattern(PatternID id) erases the unique_ptr-held PatternSource in
m_patterns causing shared_ptrs returned by getAllPatterns() (which use no-op
deleters and raw pointers) to dangle; fix by either converting m_patterns to
hold std::shared_ptr<PatternSource> so getAllPatterns() can return shared_ptrs
that correctly extend object lifetime, or change getAllPatterns() to return
non-owning references (e.g., PatternID list or raw/non-owning pointers) instead
of shared_ptrs; update removePattern, m_patterns' type, and
getAllPatterns()/PatternSource handling consistently so ownership semantics are
clear and no use-after-free can occur.


private:
uint64_t nextId{1};
std::unordered_map<uint64_t, std::unique_ptr<PatternSource>> m_patterns;
};

} // namespace Audio
} // namespace Aestra
} // namespace Aestra
3 changes: 1 addition & 2 deletions AestraAudio/include/Models/PatternSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,8 @@ class PatternSource {

// Convenience access to MIDI notes
std::vector<MidiNote>& getMidiNotes() { return std::get<MidiPayload>(payload).notes; }

const std::vector<MidiNote>& getMidiNotes() const { return std::get<MidiPayload>(payload).notes; }
};

} // namespace Audio
} // namespace Aestra
} // namespace Aestra
6 changes: 4 additions & 2 deletions AestraAudio/include/Models/PlaylistMixer.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@

namespace Aestra {
namespace Audio {

class PlaylistMixer {
public:
static void setResamplingQuality(ClipResamplingQuality quality) {
s_resamplingQuality = quality;
}

static void setResamplingQuality(int quality) {
setResamplingQuality(static_cast<ClipResamplingQuality>(quality));
}
Comment on lines +12 to +14
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 | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check ClipResamplingQuality enum definition to determine valid range
ast-grep --pattern $'enum class ClipResamplingQuality {
  $$$
}'

# Fallback: search for enum definition
rg -n 'enum.*ClipResamplingQuality' -A 10

Repository: currentsuspect/Aestra

Length of output: 18416


🏁 Script executed:

# Find all calls to setResamplingQuality with int argument
rg -n 'setResamplingQuality\s*\(\s*\d+\s*\)|setResamplingQuality\s*\(\s*\w+\s*\)' -A 2 -B 2

# Also search for the implementation in PlaylistMixer
rg -n 'setResamplingQuality' -A 3 -B 1 AestraAudio/include/Models/PlaylistMixer.h

# Check if there's a definition in PlaylistMixer.h
cat -n AestraAudio/include/Models/PlaylistMixer.h | head -50

Repository: currentsuspect/Aestra

Length of output: 1191


🏁 Script executed:

# Search for calls to setResamplingQuality(int) - look for numeric literals or int variables
rg -n 'setResamplingQuality\(' --type cpp --type h | grep -E '\(.*\)' | head -30

# More specific: search for setResamplingQuality calls with int or numeric values
rg -n 'PlaylistMixer.*setResamplingQuality|\.setResamplingQuality' -B 2 -A 2

# Check AudioSettingsPage.cpp mentioned in original review
fd 'AudioSettingsPage' -type f | head -5

Repository: currentsuspect/Aestra

Length of output: 2198


🏁 Script executed:

# Examine AudioSettingsPage.cpp around the call to setResamplingQuality
sed -n '370,400p' Source/Settings/AudioSettingsPage.cpp

Repository: currentsuspect/Aestra

Length of output: 1288


🏁 Script executed:

# Check what determines qIdx and its range
sed -n '360,390p' Source/Settings/AudioSettingsPage.cpp

Repository: currentsuspect/Aestra

Length of output: 1219


Int overload lacks validation — consider removing or adding bounds checks.

The setResamplingQuality(int) overload casts any int directly to ClipResamplingQuality without validating it's in the valid range [0–3]. While the current call site in AudioSettingsPage.cpp validates before calling, the function signature itself permits invalid values and offers no protection against future misuse.

Consider either:

  • Removing the int overload to force explicit enum usage
  • Adding bounds validation (return early or log if out of range)
  • Documenting the valid range and expected usage pattern
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@AestraAudio/include/Models/PlaylistMixer.h` around lines 12 - 14, The int
overload setResamplingQuality(int) casts blindly to ClipResamplingQuality;
either remove this overload or add validation to ensure the incoming integer is
within the valid enum range (0–3) before casting—e.g., check if quality >= 0 &&
quality <= 3, log or return early on invalid values, otherwise call
setResamplingQuality(static_cast<ClipResamplingQuality>(quality)); reference the
setResamplingQuality(int) and ClipResamplingQuality symbols and update callers
if you choose to remove the overload to force enum usage.


static ClipResamplingQuality getResamplingQuality() {
return s_resamplingQuality;
}

private:
static inline ClipResamplingQuality s_resamplingQuality = ClipResamplingQuality::Standard;
};

} // namespace Audio
} // namespace Aestra
Loading
Loading