fix: review comments

Author: closetcaiman

packages/react-native-audio-api/common/cpp/audioapi/HostObjects/AudioParamHostObject.cpp

@@ -1,13 +1,11 @@
-#include <audioapi/HostObjects/AudioParamHostObject.h>
-
-#include <audioapi/core/AudioParam.h>
-#include <audioapi/core/utils/automation/AutomationEvent.hpp>
-#include <audioapi/utils/AudioArray.hpp>
-
+#include "audioapi/HostObjects/AudioParamHostObject.h"
 #include <memory>
 #include <string>
 #include <utility>
+#include "audioapi/core/AudioParam.h"
+#include "audioapi/core/utils/automation/AutomationEvent.h"
 #include "audioapi/jsi/JsiHostObject.h"
+#include "audioapi/utils/AudioArray.hpp"
 
 namespace audioapi {
 

packages/react-native-audio-api/common/cpp/audioapi/HostObjects/AudioParamHostObject.h

@@ -1,12 +1,11 @@
 #pragma once
 
-#include <audioapi/jsi/JsiHostObject.h>
-
 #include <jsi/jsi.h>
 #include <cstddef>
 #include <memory>
 #include <string>
 #include "audioapi/core/utils/automation/AutomationControlQueue.h"
+#include "audioapi/jsi/JsiHostObject.h"
 #include "audioapi/utils/Result.hpp"
 
 namespace audioapi {
@@ -32,7 +31,6 @@ class AudioParamHostObject : public JsiHostObject {
   JSI_HOST_FUNCTION_DECL(setValueCurveAtTime);
   JSI_HOST_FUNCTION_DECL(cancelScheduledValues);
   JSI_HOST_FUNCTION_DECL(cancelAndHoldAtTime);
-
   JSI_HOST_FUNCTION_DECL(checkCurveExclusion);
 
  private:

packages/react-native-audio-api/common/cpp/audioapi/core/types/AutomationEventType.h

@@ -1,9 +1,10 @@
 #pragma once
 
+#include <cstdint>
 #include <string_view>
 namespace audioapi {
 
-enum class AutomationEventType {
+enum class AutomationEventType : uint8_t {
   LINEAR_RAMP,
   EXPONENTIAL_RAMP,
   SET_VALUE,

packages/react-native-audio-api/common/cpp/audioapi/core/utils/Constants.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include <cmath>
-#include <cstddef>
 #include <limits>
 #include <new>
 #include <numbers>

packages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationControlQueue.cpp

@@ -1,77 +1,63 @@
 #include "audioapi/core/utils/automation/AutomationControlQueue.h"
 #include <cstddef>
 #include <sstream>
-#include <string>
 #include "audioapi/core/types/AutomationEventType.h"
-#include "audioapi/core/utils/automation/AutomationEvent.hpp"
+#include "audioapi/core/utils/automation/AutomationEvent.h"
 #include "audioapi/utils/Result.hpp"
 
 namespace audioapi {
 
-Result<NoneType, std::string> AutomationControlQueue::checkCurveExclusion(
-    const AutomationEvent &event) {
+EventConflictResult AutomationControlQueue::checkCurveExclusion(const AutomationEvent &event) {
   if (event.getType() == AutomationEventType::SET_VALUE_CURVE) {
     // For curve events, check for any event that occurs at or within the curve's time interval
-    const auto *conflict = findEventInInterval(event.getStartTime(), event.getEndTime());
-    if (conflict != nullptr) {
-      std::stringstream ss;
-      ss << "Cannot schedule curve event from time " << event.getStartTime() << " to "
-         << event.getEndTime() << " because it conflicts with an existing event of type "
-         << toString(conflict->getType()) << " at time " << conflict->getAutomationTime() << ".";
-      return Err(ss.str());
-    }
-  } else {
-    // For non-curve events check for curve events that conflict at the event's automationTime
-    const auto *conflict = findEventAtTime(event.getAutomationTime());
-    if ((conflict != nullptr) && conflict->getType() == AutomationEventType::SET_VALUE_CURVE) {
-      std::stringstream ss;
-      ss << "Cannot schedule event of type " << toString(event.getType()) << " at time "
-         << event.getAutomationTime()
-         << " because it conflicts with an existing curve event from time "
-         << conflict->getStartTime() << " to " << conflict->getEndTime() << ".";
-      return Err(ss.str());
-    }
+    return isConflictInInterval(event, event.getStartTime(), event.getEndTime());
   }
-  return Ok(None);
+  // For non-curve events check for curve events that conflict at the event's automationTime
+  return isConflictAtTime(event, event.getAutomationTime());
 }
 
 void AutomationControlQueue::purge(double currentTime) {
   eventQueue_.erase(eventQueue_.begin(), eventQueue_.lowerBound(currentTime));
 }
 
-const AutomationEvent *AutomationControlQueue::findEventAtTime(double automationTime) {
+EventConflictResult AutomationControlQueue::isConflictAtTime(
+    const AutomationEvent &newEvent,
+    double automationTime) {
   // Check if a SET_VALUE_CURVE that starts before automationTime extends into it
   auto it = eventQueue_.upperBound(automationTime);
   if (it != eventQueue_.begin()) {
     const auto &pred = *std::prev(it);
     if (pred.getType() == AutomationEventType::SET_VALUE_CURVE &&
-        automationTime <= pred.getEndTime()) {
-      return &pred;
+        automationTime < pred.getEndTime()) {
+      std::stringstream ss;
+      ss << "Cannot schedule event of type " << toString(newEvent.getType()) << " at time "
+         << newEvent.getAutomationTime()
+         << " because it conflicts with an existing curve event from time " << pred.getStartTime()
+         << " to " << pred.getEndTime() << ".";
+      return Err(ss.str());
     }
   }
-
-  // Check for an event with an exact automationTime match
-  auto lo = eventQueue_.lowerBound(automationTime);
-  if (lo != it) {
-    return &(*lo);
-  }
-
-  return nullptr;
+  return Ok(None);
 }
 
-const AutomationEvent *AutomationControlQueue::findEventInInterval(
+EventConflictResult AutomationControlQueue::isConflictInInterval(
+    const AutomationEvent &newEvent,
     double startTime,
     double endTime) {
   // Non-ramp events have automationTime == startTime, so lowerBound/lowerBound brackets them.
   // Ramp events have startTime == 0 (unresolved on the control thread), so getStartTime() >= startTime
   // filters them out.
   for (auto it = eventQueue_.lowerBound(startTime), hi = eventQueue_.lowerBound(endTime); it != hi;
        ++it) {
-    if (it->getStartTime() >= startTime) {
-      return &(*it);
+    if (it->getStartTime() > startTime) {
+      std::stringstream ss;
+      ss << "Cannot schedule curve event from time " << newEvent.getStartTime() << " to "
+         << newEvent.getEndTime() << " because it conflicts with an existing event of type "
+         << toString(it->getType()) << " at time " << it->getAutomationTime() << ".";
+      return Err(ss.str());
     }
   }
-  return nullptr;
+  return Ok(None);
 }
 
 } // namespace audioapi

packages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationControlQueue.h

@@ -1,24 +1,23 @@
 #pragma once
 
-#include <audioapi/core/types/AutomationEventType.h>
-#include <audioapi/core/utils/automation/AutomationEvent.hpp>
-#include <audioapi/core/utils/automation/AutomationQueueBase.hpp>
-#include <audioapi/utils/Result.hpp>
 #include <string>
+#include "audioapi/core/utils/automation/AutomationEvent.h"
+#include "audioapi/core/utils/automation/AutomationQueueBase.hpp"
+#include "audioapi/utils/Result.hpp"
 
 namespace audioapi {
 
+using EventConflictResult = Result<NoneType, std::string>;
+
 /// @brief A queue for managing audio parameter change events on the JS/control thread.
 /// @note The invariant of the queue is that its internal buffer always contains non-overlapping events.
 class AutomationControlQueue : public AutomationQueueBase<AutomationEvent> {
  public:
-  explicit AutomationControlQueue() = default;
-
   /// @brief Validate if a new event can be added to the queue without violating curve exclusion rules.
   /// See: https://webaudio.github.io/web-audio-api/#automation-event-time
   /// @param event The new event to validate.
   /// @return Ok if the event can be added, Err with a message if it cannot be added.
-  [[nodiscard]] Result<NoneType, std::string> checkCurveExclusion(const AutomationEvent &event);
+  [[nodiscard]] EventConflictResult checkCurveExclusion(const AutomationEvent &event);
 
   /// @brief Remove all events with automationTime strictly before currentTime.
   /// @note Should be called before push to prevent the queue from filling up with past events.
@@ -28,13 +27,14 @@ class AutomationControlQueue : public AutomationQueueBase<AutomationEvent> {
   /// @brief Find an event that occurs at the given time. For curve events, checks if time is within the event's interval.
   /// @param time The time to check for an event.
   /// @return A pointer to the conflicting event, or nullptr if no conflict.
-  [[nodiscard]] const AutomationEvent *findEventAtTime(double time);
+  [[nodiscard]] EventConflictResult isConflictAtTime(const AutomationEvent &event, double time);
 
   /// @brief Find an event that starts within the given time interval.
   /// @param startTime The start time of the interval.
   /// @param endTime The end time of the interval.
   /// @return A pointer to the conflicting event, or nullptr if no conflict.
-  [[nodiscard]] const AutomationEvent *findEventInInterval(double startTime, double endTime);
+  [[nodiscard]] EventConflictResult
+  isConflictInInterval(const AutomationEvent &event, double startTime, double endTime);
 };
 
 } // namespace audioapi

…ore/utils/automation/AutomationEvent.hpp …/core/utils/automation/AutomationEvent.hpackages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationEvent.hpp renamed to packages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationEvent.h packages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationEvent.hpp renamed to packages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationEvent.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#include <audioapi/core/types/AutomationEventType.h>
+#include "audioapi/core/types/AutomationEventType.h"
 
 namespace audioapi {
 
@@ -22,8 +22,8 @@ class AutomationEvent {
         startTime_(isRamp(type) ? 0.0 : automationTime),
         endTime_(isRamp(type) ? automationTime : 0.0) {}
 
-  AutomationEvent(const AutomationEvent &) = delete;
-  AutomationEvent &operator=(const AutomationEvent &) = delete;
+  AutomationEvent(const AutomationEvent &) = default;
+  AutomationEvent &operator=(const AutomationEvent &) = default;
 
   AutomationEvent(AutomationEvent &&other) noexcept
       : type_(other.type_), startTime_(other.startTime_), endTime_(other.endTime_) {}

packages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationQueueBase.hpp

@@ -3,6 +3,7 @@
 #include <utility>
 #include "audioapi/core/utils/Constants.h"
 #include "audioapi/utils/BoundedPriorityQueue.hpp"
+#include "audioapi/utils/Macros.h"
 
 namespace audioapi {
 
@@ -14,10 +15,7 @@ template <AutomationEventConcept TEvent>
 class AutomationQueueBase {
  public:
   AutomationQueueBase() = default;
-  AutomationQueueBase(const AutomationQueueBase &) = delete;
-  AutomationQueueBase &operator=(const AutomationQueueBase &) = delete;
-  AutomationQueueBase(AutomationQueueBase &&) noexcept = delete;
-  AutomationQueueBase &operator=(AutomationQueueBase &&) noexcept = delete;
+  DELETE_COPY_AND_MOVE(AutomationQueueBase);
   virtual ~AutomationQueueBase() = default;
 
   /// @brief Cancel scheduled parameter changes at or after the given time.

packages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationRenderQueue.cpp

@@ -1,9 +1,10 @@
-#include <audioapi/core/utils/automation/AutomationRenderEventFactory.hpp>
-#include <audioapi/core/utils/automation/AutomationRenderQueue.h>
+#include "audioapi/core/utils/automation/AutomationRenderQueue.h"
 #include <cstddef>
 #include <optional>
 #include <utility>
 #include "audioapi/core/types/AutomationEventType.h"
+#include "audioapi/core/utils/automation/AutomationQueueBase.hpp"
+#include "audioapi/core/utils/automation/AutomationRenderEventFactory.hpp"
 
 namespace audioapi {
 
@@ -31,7 +32,7 @@ std::optional<float> AutomationRenderQueue::computeValueAtTime(double time) {
 
 bool AutomationRenderQueue::push(RenderAutomationEvent &&event) {
   resolveEventValues(event);
-  return eventQueue_.push(std::move(event));
+  return AutomationQueueBase::push(std::move(event));
 }
 
 void AutomationRenderQueue::resolveEventValues(RenderAutomationEvent &event) {

packages/react-native-audio-api/common/cpp/audioapi/core/utils/automation/AutomationRenderQueue.h

@@ -1,9 +1,8 @@
 #pragma once
 
-#include <audioapi/core/types/AutomationEventType.h>
-#include <audioapi/core/utils/automation/AutomationQueueBase.hpp>
-#include <audioapi/core/utils/automation/RenderAutomationEvent.hpp>
 #include <optional>
+#include "audioapi/core/utils/automation/AutomationQueueBase.hpp"
+#include "audioapi/core/utils/automation/RenderAutomationEvent.hpp"
 
 namespace audioapi {