Skip to content

Commit f883117

Browse files
maciejmakowski2003maciejmakowski2003
authored andcommitted
Fix/clear callback bug (#780)
* chore: clang code/formatting issues * chore: linting * fix: thread safe callbacks in source nodes * ci: lint * fix: nitpicks * chore: pods --------- Co-authored-by: maciejmakowski2003 <maciej.makowski2608@gmail.com>
1 parent 44925d7 commit f883117

File tree

17 files changed

+81
-69
lines changed

17 files changed

+81
-69
lines changed

apps/fabric-example/ios/Podfile.lock

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2495,9 +2495,38 @@ PODS:
24952495
- ReactCodegen
24962496
- ReactCommon/turbomodule/bridging
24972497
- ReactCommon/turbomodule/core
2498+
- RNAudioAPI/audioapi/audioapi_dsp (= 0.10.0)
24982499
- RNAudioAPI/audioapi/ios (= 0.10.0)
24992500
- SocketRocket
25002501
- Yoga
2502+
- RNAudioAPI/audioapi/audioapi_dsp (0.10.0):
2503+
- boost
2504+
- DoubleConversion
2505+
- fast_float
2506+
- fmt
2507+
- glog
2508+
- hermes-engine
2509+
- RCT-Folly
2510+
- RCT-Folly/Fabric
2511+
- RCTRequired
2512+
- RCTTypeSafety
2513+
- React-Core
2514+
- React-debug
2515+
- React-Fabric
2516+
- React-featureflags
2517+
- React-graphics
2518+
- React-ImageManager
2519+
- React-jsi
2520+
- React-NativeModulesApple
2521+
- React-RCTFabric
2522+
- React-renderercss
2523+
- React-rendererdebug
2524+
- React-utils
2525+
- ReactCodegen
2526+
- ReactCommon/turbomodule/bridging
2527+
- ReactCommon/turbomodule/core
2528+
- SocketRocket
2529+
- Yoga
25012530
- RNAudioAPI/audioapi/ios (0.10.0):
25022531
- boost
25032532
- DoubleConversion
@@ -3187,7 +3216,7 @@ SPEC CHECKSUMS:
31873216
ReactAppDependencyProvider: c5c4f5280e4ae0f9f4a739c64c4260fe0b3edaf1
31883217
ReactCodegen: 096bbbb2498ca55f385e2fbd465bfa0211ee8295
31893218
ReactCommon: 25c7f94aee74ddd93a8287756a8ac0830a309544
3190-
RNAudioAPI: 39ded048f05fe0b842c3d1f4f59a42ef364ba22d
3219+
RNAudioAPI: e448fe7d3de47b5889dc5caf14f13032b259b809
31913220
RNGestureHandler: f1dd7f92a0faa2868a919ab53bb9d66eb4ebfcf5
31923221
RNReanimated: e4993dd98196c698cbacc1441a4ac5b855ae56dc
31933222
RNScreens: 833237c48c756d40764540246a501b47dadb2cac

packages/react-native-audio-api/android/src/main/java/com/swmansion/audioapi/AudioAPIModule.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package com.swmansion.audioapi
22

33
import com.facebook.jni.HybridData
44
import com.facebook.react.bridge.LifecycleEventListener
5-
import com.facebook.react.bridge.NativeModule
65
import com.facebook.react.bridge.Promise
76
import com.facebook.react.bridge.ReactApplicationContext
87
import com.facebook.react.bridge.ReadableArray
@@ -27,7 +26,6 @@ class AudioAPIModule(
2726
val reactContext: WeakReference<ReactApplicationContext> = WeakReference(reactContext)
2827

2928
private val mHybridData: HybridData
30-
private var reanimatedModule: NativeModule? = null
3129

3230
external fun initHybrid(
3331
workletsModule: Any?,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ AudioBufferBaseSourceNodeHostObject::~AudioBufferBaseSourceNodeHostObject() {
2828
// When JSI object is garbage collected (together with the eventual callback),
2929
// underlying source node might still be active and try to call the
3030
// non-existing callback.
31-
sourceNode->clearOnPositionChangedCallback();
31+
sourceNode->setOnPositionChangedCallbackId(0);
3232
}
3333

3434
JSI_PROPERTY_GETTER_IMPL(AudioBufferBaseSourceNodeHostObject, detune) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ AudioBufferSourceNodeHostObject::~AudioBufferSourceNodeHostObject() {
3737
// When JSI object is garbage collected (together with the eventual callback),
3838
// underlying source node might still be active and try to call the
3939
// non-existing callback.
40-
audioBufferSourceNode->clearOnLoopEndedCallback();
40+
audioBufferSourceNode->setOnLoopEndedCallbackId(0);
4141
}
4242

4343
JSI_PROPERTY_GETTER_IMPL(AudioBufferSourceNodeHostObject, loop) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ AudioScheduledSourceNodeHostObject::~AudioScheduledSourceNodeHostObject() {
2222
// When JSI object is garbage collected (together with the eventual callback),
2323
// underlying source node might still be active and try to call the
2424
// non-existing callback.
25-
audioScheduledSourceNode->clearOnEndedCallback();
25+
audioScheduledSourceNode->setOnEndedCallbackId(0);
2626
}
2727

2828
JSI_PROPERTY_SETTER_IMPL(AudioScheduledSourceNodeHostObject, onEnded) {

packages/react-native-audio-api/common/cpp/audioapi/core/effects/ConvolverNode.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,16 @@
1111
namespace audioapi {
1212
ConvolverNode::ConvolverNode(
1313
BaseAudioContext *context,
14-
std::shared_ptr<AudioBuffer> buffer,
14+
const std::shared_ptr<AudioBuffer> &buffer,
1515
bool disableNormalization)
1616
: AudioNode(context),
17-
buffer_(nullptr),
18-
internalBuffer_(nullptr),
19-
signalledToStop_(false),
2017
remainingSegments_(0),
2118
internalBufferIndex_(0),
19+
signalledToStop_(false),
2220
scaleFactor_(1.0f),
23-
intermediateBus_(nullptr) {
21+
intermediateBus_(nullptr),
22+
buffer_(nullptr),
23+
internalBuffer_(nullptr) {
2424
channelCount_ = 2;
2525
channelCountMode_ = ChannelCountMode::CLAMPED_MAX;
2626
normalize_ = !disableNormalization;

packages/react-native-audio-api/common/cpp/audioapi/core/effects/ConvolverNode.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class AudioBuffer;
1919

2020
class ConvolverNode : public AudioNode {
2121
public:
22-
explicit ConvolverNode(BaseAudioContext *context, std::shared_ptr<AudioBuffer> buffer, bool disableNormalization);
22+
explicit ConvolverNode(BaseAudioContext *context, const std::shared_ptr<AudioBuffer>& buffer, bool disableNormalization);
2323

2424
[[nodiscard]] bool getNormalize_() const;
2525
[[nodiscard]] const std::shared_ptr<AudioBuffer> &getBuffer() const;

packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.cpp

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,15 @@ std::shared_ptr<AudioParam> AudioBufferBaseSourceNode::getPlaybackRateParam()
3636
return playbackRateParam_;
3737
}
3838

39-
void AudioBufferBaseSourceNode::clearOnPositionChangedCallback() {
40-
if (onPositionChangedCallbackId_ == 0 || context_ == nullptr ||
41-
context_->audioEventHandlerRegistry_ == nullptr) {
42-
return;
43-
}
44-
45-
context_->audioEventHandlerRegistry_->unregisterHandler(
46-
"positionChanged", onPositionChangedCallbackId_);
47-
onPositionChangedCallbackId_ = 0;
48-
}
49-
5039
void AudioBufferBaseSourceNode::setOnPositionChangedCallbackId(
5140
uint64_t callbackId) {
52-
onPositionChangedCallbackId_ = callbackId;
41+
auto oldCallbackId = onPositionChangedCallbackId_.exchange(
42+
callbackId, std::memory_order_acq_rel);
43+
44+
if (oldCallbackId != 0) {
45+
audioEventHandlerRegistry_->unregisterHandler(
46+
"positionChanged", oldCallbackId);
47+
}
5348
}
5449

5550
void AudioBufferBaseSourceNode::setOnPositionChangedInterval(int interval) {
@@ -66,14 +61,16 @@ std::mutex &AudioBufferBaseSourceNode::getBufferLock() {
6661
}
6762

6863
void AudioBufferBaseSourceNode::sendOnPositionChangedEvent() {
69-
if (onPositionChangedCallbackId_ != 0 &&
70-
onPositionChangedTime_ > onPositionChangedInterval_ &&
71-
context_->audioEventHandlerRegistry_ != nullptr) {
64+
auto onPositionChangedCallbackId =
65+
onPositionChangedCallbackId_.load(std::memory_order_acquire);
66+
67+
if (onPositionChangedCallbackId != 0 &&
68+
onPositionChangedTime_ > onPositionChangedInterval_) {
7269
std::unordered_map<std::string, EventValue> body = {
7370
{"value", getCurrentPosition()}};
7471

75-
context_->audioEventHandlerRegistry_->invokeHandlerWithEventBody(
76-
"positionChanged", onPositionChangedCallbackId_, body);
72+
audioEventHandlerRegistry_->invokeHandlerWithEventBody(
73+
"positionChanged", onPositionChangedCallbackId, body);
7774

7875
onPositionChangedTime_ = 0;
7976
}

packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferBaseSourceNode.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ class AudioBufferBaseSourceNode : public AudioScheduledSourceNode {
1919
[[nodiscard]] std::shared_ptr<AudioParam> getDetuneParam() const;
2020
[[nodiscard]] std::shared_ptr<AudioParam> getPlaybackRateParam() const;
2121

22-
void clearOnPositionChangedCallback();
2322
void setOnPositionChangedCallbackId(uint64_t callbackId);
2423
void setOnPositionChangedInterval(int interval);
2524
[[nodiscard]] int getOnPositionChangedInterval() const;

packages/react-native-audio-api/common/cpp/audioapi/core/sources/AudioBufferSourceNode.cpp

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -123,19 +123,13 @@ void AudioBufferSourceNode::disable() {
123123
alignedBus_.reset();
124124
}
125125

126-
void AudioBufferSourceNode::clearOnLoopEndedCallback() {
127-
if (onLoopEndedCallbackId_ == 0 || context_ == nullptr ||
128-
context_->audioEventHandlerRegistry_ == nullptr) {
129-
return;
130-
}
131-
132-
context_->audioEventHandlerRegistry_->unregisterHandler(
133-
"loopEnded", onLoopEndedCallbackId_);
134-
onLoopEndedCallbackId_ = 0;
135-
}
136-
137126
void AudioBufferSourceNode::setOnLoopEndedCallbackId(uint64_t callbackId) {
138-
onLoopEndedCallbackId_ = callbackId;
127+
auto oldCallbackId =
128+
onLoopEndedCallbackId_.exchange(callbackId, std::memory_order_acq_rel);
129+
130+
if (oldCallbackId != 0) {
131+
audioEventHandlerRegistry_->unregisterHandler("loopEnded", oldCallbackId);
132+
}
139133
}
140134

141135
std::shared_ptr<AudioBus> AudioBufferSourceNode::processNode(
@@ -171,8 +165,8 @@ void AudioBufferSourceNode::sendOnLoopEndedEvent() {
171165
auto onLoopEndedCallbackId =
172166
onLoopEndedCallbackId_.load(std::memory_order_acquire);
173167
if (onLoopEndedCallbackId != 0) {
174-
context_->audioEventHandlerRegistry_->invokeHandlerWithEventBody(
175-
"loopEnded", onLoopEndedCallbackId_, {});
168+
audioEventHandlerRegistry_->invokeHandlerWithEventBody(
169+
"loopEnded", onLoopEndedCallbackId, {});
176170
}
177171
}
178172

0 commit comments

Comments
 (0)