Skip to content

DOC-14148: Limit maximum size of the audio buffer #58

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 29, 2025
Merged

DOC-14148: Limit maximum size of the audio buffer #58

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cfac7d6
DOC-14148: Limit maximum size of the audio buffer
Jul 7, 2025
1cec777
Revert "DOC-14148: Limit maximum size of the audio buffer"
Jul 29, 2025
106f615
DOC-14148: Make weak reference
Jul 29, 2025
1a29965
DOC-14148: Capture weak self only in necessary places
Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 21 additions & 17 deletions RDMPEG/RDMPEGPlayer/RDMPEGPlayer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,9 @@ public class RDMPEGPlayer: NSObject {
}
}

framebuffer.atomicSubtitleFramesAccess {
framebuffer.atomicSubtitleFramesAccess { [weak self] in
guard let self = self else { return }

while let nextSubtitleFrame = self.framebuffer.nextSubtitleFrame {
let nextSubtitleStartTime = nextSubtitleFrame.position
let nextSubtitleEndTime = nextSubtitleStartTime + nextSubtitleFrame.duration
Expand Down Expand Up @@ -875,7 +877,7 @@ public class RDMPEGPlayer: NSObject {
autoreleasepool {
var outData = outData

if videoStreamExist && correctionInfo == nil {
if self.videoStreamExist && self.correctionInfo == nil {
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property access occurs within an audio callback closure but doesn't use weak self capture like other closures in this method. This could lead to a strong reference cycle and potential crashes when the player is deallocated during audio processing.

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only closure here is the autoreleasepool. Self has been properly captured as weak below

#if RD_DEBUG_MPEG_PLAYER
L4Logger.logger(forName: "rd.mediaplayer.RDMPEGPlayer").debug("Silence audio while correcting video")
#endif
Expand All @@ -887,7 +889,7 @@ public class RDMPEGPlayer: NSObject {
var numFramesLeft = numFrames

while numFramesLeft > 0 {
if rawAudioFrame == nil {
if self.rawAudioFrame == nil {
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to line 880, this property access in the audio callback doesn't use weak self capture, which could cause retain cycles and crashes during player deallocation.

Copilot uses AI. Check for mistakes.

var nextAudioFrame: RDMPEGAudioFrame?
var isAudioOutrun = false
var isAudioLags = false
Expand All @@ -896,7 +898,9 @@ public class RDMPEGPlayer: NSObject {
let loggingScope = L4Logger.logger(forName: "rd.mediaplayer.RDMPEGPlayer").loggingScope()
#endif

framebuffer.atomicAudioFramesAccess {
framebuffer.atomicAudioFramesAccess { [weak self] in
guard let self = self else { return }

if let nextFrame = self.framebuffer.nextAudioFrame {
let delta = self.correctionInfo?.correctionInterval(
withCurrentTime: nextFrame.position
Expand All @@ -916,8 +920,8 @@ public class RDMPEGPlayer: NSObject {

nextAudioFrame = self.framebuffer.popAudioFrame()

if videoStreamExist == false {
currentInternalTime = nextAudioFrame?.position ?? 0
if self.videoStreamExist == false {
self.currentInternalTime = nextAudioFrame?.position ?? 0
}

if delta < -0.1, self.framebuffer.nextAudioFrame != nil {
Expand Down Expand Up @@ -947,29 +951,29 @@ public class RDMPEGPlayer: NSObject {
.debug("Audio frame will be rendered: \(audioFrame.position) \(audioFrame.duration)")
#endif

rawAudioFrame = RDMPEGRawAudioFrame(rawAudioData: audioFrame.samples)
self.rawAudioFrame = RDMPEGRawAudioFrame(rawAudioData: audioFrame.samples)

if videoStreamExist == false {
correctionInfo = RDMPEGCorrectionInfo(
if self.videoStreamExist == false {
self.correctionInfo = RDMPEGCorrectionInfo(
playbackStartDate: Date(),
playbackStartTime: currentInternalTime
playbackStartTime: self.currentInternalTime
)

DispatchQueue.main.async {
self.setBufferingStateIfNeededAndNotify(false)
DispatchQueue.main.async { [weak self] in
self?.setBufferingStateIfNeededAndNotify(false)
}
}
}
else if videoStreamExist == false {
correctionInfo = nil
else if self.videoStreamExist == false {
self.correctionInfo = nil

DispatchQueue.main.async {
self.setBufferingStateIfNeededAndNotify(true)
DispatchQueue.main.async { [weak self] in
self?.setBufferingStateIfNeededAndNotify(true)
}
}
}

if let rawAudioFrame = rawAudioFrame {
if let rawAudioFrame = self.rawAudioFrame {
#if RD_DEBUG_MPEG_PLAYER
L4Logger.logger(forName: "rd.mediaplayer.RDMPEGPlayer").debug("Rendering raw audio frame")
#endif
Expand Down