livekit / client-sdk-swift

LiveKit Swift Client SDK. Easily build live audio or video experiences into your mobile app, game or website.
https://livekit.io
Apache License 2.0
174 stars 84 forks source link

RemoteAudioTrack.add(audioRenderer:) doesn't work #350

Closed floschliep closed 4 months ago

floschliep commented 4 months ago

Describe the bug This is a follow-up to https://github.com/livekit/client-sdk-swift/issues/188.

The solution advertised by @hiroshihorie of subscribing to an audio track via RemoteAudioTrack.add(audioRenderer:) doesn't work as expected as the wrapped object AudioRendererAdapter gets released immediately due to the memory semantics of WebRTC.

Specifically, RemoteAudioTrack internally calls audioTrack.add(AudioRendererAdapter(target: audioRenderer)), which wraps the supplied audio renderer in an adapter and sends it to the LKRTCAudioTrack object. This adapter, however, is not retained by anyone and thus released immediately. See the code documentation Does not retain. in RTCAudioTrack.h.

As a fix, I would suggest either of the 2 solutions:

  1. Return a type-erased object from add(audioRenderer:) which the caller is responsible for keeping a reference to. This would mimic the memory semantics of WebRTC, but adds complexity on the integrator's side.

  2. Store references to the created adapter objects inside of RemoteAudioTrack until remove(audioRenderer:) is called. This comes with the added complexity of removing adapters when their internal target is deallocated (i.e. the renderer which the caller supplied).

As a side note to No 2, reading through the RemoteAudioTrack.remove(audioRenderer:) implementation, I highly suspect this doesn't work because it creates a new adapter and tries to remove that from audioTrack. I haven't tested this myself, but I think what should happen here is the following:

SDK Version 2.0.4

iOS/macOS Version iOS

Steps to Reproduce

Expected behavior render(sampleBuffer:) of my AudioRenderer implementation should be called.

Actual behavior Nothing happens.

floschliep commented 4 months ago

For reference, this fixes the issue for me.

hiroshihorie commented 4 months ago

Hello, thanks for reporting this. Will look into.

hiroshihorie commented 4 months ago

Fixing this issue on this PR, thanks for reporting!