-
-
Notifications
You must be signed in to change notification settings - Fork 670
Fix: AudioRingBuffer wrap-around crash #1864
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
Conversation
| guard let outputPtr = outputBuffer.int16ChannelData?[0], | ||
| let inputPtr = audioPCMBuffer.int16ChannelData?[0] else { return } |
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.
When pcmFormatInt16 is used, the following value is guaranteed to be present, so the guard block seems redundant:
- outputBuffer.int16ChannelData?[0]
shogo4405
left a comment
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.
In what kind of use cases would this type of issue occur?
Also, since there are no test cases, it is not possible to determine whether this change is good or bad, or whether it introduces other issues, so I cannot accept it.
|
I believe this crash occurs because the data exceeds the maximum buffer size (16,384) that the AudioRingBuffer can store. As a possible fix, I think it would be better to skip processing instead of calling To better understand the situation, I would like to see the output of the CMSampleBuffer at the time of the crash so we can know how much data was actually being buffered. HaishinKit.swift/HaishinKit/Sources/Mixer/AudioRingBuffer.swift Lines 44 to 52 in 47ccc4b
|
|
I believe this has been improved on our side here: |
|
@shogo4405 Confirmed that no crash occurs. #1867 |
|
Thank you for checking. I will close this PR. Thanks for reporting. |

Description & motivation
AudioRingBuffer.append(_:offset:)could crash withEXC_BAD_ACCESSwhen the ring wrapped, because recursive calls restarted atnumSamplesinstead of accumulating the original offset.offset/headstay within[0, frameLength).nextOffset = offset + numSamplesso the copy resumes from the correct position.Type of change
Screenshots: