-
Notifications
You must be signed in to change notification settings - Fork 139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Crash] Default Player crash with MTAudioProcessingTapCallbacks #2420
[Crash] Default Player crash with MTAudioProcessingTapCallbacks #2420
Conversation
podcasts/DefaultPlayer.swift
Outdated
@@ -342,7 +386,14 @@ class DefaultPlayer: PlaybackProtocol, Hashable { | |||
} | |||
|
|||
let tapProcess: MTAudioProcessingTapProcessCallback = { tap, numberFrames, _, bufferListInOut, numberFramesOut, flagsOut in | |||
var referenceToSelf = Unmanaged<DefaultPlayer>.fromOpaque(MTAudioProcessingTapGetStorage(tap)).takeUnretainedValue() | |||
let referenceToSelf: DefaultPlayer | |||
if FeatureFlag.useDefaultPlayerTapCookie.enabled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we encapsulate the code bellow to method and used on all places above?
Something like: getPlayerFromTap(_) -> DefaultPlayer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this on device and it's working correctly, just left a suggestion for code improvement.
Thanks @SergioEstevao . Added a static function in 22c2c8d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to #1711
An attempt to fix the crashes related to
MTAudioProcessingTapCallbacks
.As a tap can not be synchronously stopped, I suspect we try to access the unretained instance already deallocated.
This solution introduces basically 3 things:
AudioProcessingTapProxy
which wraps as weak the reference to the current playerMTAudioProcessingTapCallbacks
is created, theclientInfo
is now anUnsafeMutableRawPointer
created with the retained instance of the proxyfinalize
closure where we release the previously retained tap instanceTo test
[AudioProcessingTapProxy] Finalize tap: <MTAudioProcessingTap 0x6000030513b0> Retain count 3 Created with i/f/p/u/t callbacks = {0x10aa14448/0x10aa14600/0x10aa149b4/0x10aa14d08/0x10aa15330} flags = 0x1
[AudioProcessingTapProxy] Deinit proxy
• Switch the FF off to check everything still works as before
Checklist
CHANGELOG.md
if necessary.