Fix audio processing delegate lifecycle #784
Conversation
|
|
| set { | ||
| capturePostProcessingDelegateAdapter.set(target: newValue) | ||
| capturePostProcessingDelegateSubject.send(newValue) | ||
| public weak var capturePostProcessingDelegate: AudioCustomProcessingDelegate? { |
There was a problem hiding this comment.
do we really need to set it weak ? or we make sure it will be set to nil when AudioRenderer get removed from the track ?
There was a problem hiding this comment.
I generally agree with this comment as posted elsewhere, for delegates it's the "default" approach, but for "top-down" ownership model (no cycles) we should use weak sparingly.
There was a problem hiding this comment.
Either option works for me, as long as we’re okay with having inconsistencies related to the delegate weakness in the SDK, since anything using MulticastDelegate is weak now, e.g. room.add(delegate:), AudioManager.shared.add(localAudioRenderer:), etc.
There was a problem hiding this comment.
Unfortunately I don't see any option to "automate" it e.g. via swiftlint as every use case is vastly different and must be evaluated separately 😿
xianshijing-lk
left a comment
xianshijing-lk
left a comment
There was a problem hiding this comment.
one nit, I am OK with the change if you think weak is better
|
Hmm... looks like something broke with this PR 😢 |
#784 Broke `LocalAudioTrack.add(audioRenderer: AudioRenderer)` so this PR fixes it...
No description provided.