From 697981412561d8f112bfc17ebe96c0841fc47c7b Mon Sep 17 00:00:00 2001 From: Dennis Sheirer Date: Fri, 8 Nov 2024 03:50:28 -0500 Subject: [PATCH] #2086 Audio Playback Manager can leak memory if audio queue processing thread is never started. --- .../audio/playback/AudioPlaybackManager.java | 125 ++++++++++++------ 1 file changed, 88 insertions(+), 37 deletions(-) diff --git a/src/main/java/io/github/dsheirer/audio/playback/AudioPlaybackManager.java b/src/main/java/io/github/dsheirer/audio/playback/AudioPlaybackManager.java index 626226445..2e6b648f9 100644 --- a/src/main/java/io/github/dsheirer/audio/playback/AudioPlaybackManager.java +++ b/src/main/java/io/github/dsheirer/audio/playback/AudioPlaybackManager.java @@ -41,6 +41,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.ReentrantLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -65,6 +66,8 @@ public class AudioPlaybackManager implements Listener, IAudioContr private LinkedTransferQueue mNewAudioSegmentQueue = new LinkedTransferQueue<>(); private ScheduledExecutorService mScheduledExecutorService = Executors.newSingleThreadScheduledExecutor(new NamingThreadFactory("sdrtrunk audio manager")); + private AudioSegmentPrioritySorter mAudioSegmentPrioritySorter = new AudioSegmentPrioritySorter(); + private ReentrantLock mAudioOutputLock = new ReentrantLock(); /** * Constructs an instance. @@ -95,6 +98,9 @@ public AudioPlaybackManager(UserPreferences userPreferences) { mLog.warn("No audio output devices available"); } + + mProcessingTask = mScheduledExecutorService.scheduleAtFixedRate(new AudioSegmentProcessor(), + 0, 100, TimeUnit.MILLISECONDS); } /** @@ -189,36 +195,53 @@ else if(audioSegment.completeProperty().get()) } else if(audioSegment.isLinked()) { - for(AudioOutput audioOutput: mAudioOutputs) + mAudioOutputLock.lock(); + + try { - if(audioOutput.isLinkedTo(audioSegment)) + for(AudioOutput audioOutput: mAudioOutputs) { - it.remove(); - audioOutput.play(audioSegment); + if(audioOutput.isLinkedTo(audioSegment)) + { + it.remove(); + audioOutput.play(audioSegment); + } } } + finally + { + mAudioOutputLock.unlock(); + } } } //Sort audio segments by playback priority and assign to empty audio outputs if(!mAudioSegments.isEmpty()) { - //TODO: change this to take audio segment start time into account also - mAudioSegments.sort(Comparator.comparingInt(o -> o.monitorPriorityProperty().get())); + mAudioSegments.sort(mAudioSegmentPrioritySorter); - //Assign empty audio outputs first - for(AudioOutput audioOutput: mAudioOutputs) + mAudioOutputLock.lock(); + + try { - if(audioOutput.isEmpty()) + //Assign empty audio outputs first + for(AudioOutput audioOutput: mAudioOutputs) { - audioOutput.play(mAudioSegments.remove(0)); - - if(mAudioSegments.isEmpty()) + if(audioOutput.isEmpty()) { - return; + audioOutput.play(mAudioSegments.remove(0)); + + if(mAudioSegments.isEmpty()) + { + return; + } } } } + finally + { + mAudioOutputLock.unlock(); + } } //Remove any audio segments marked as complete that didn't get assigned to an output @@ -288,37 +311,39 @@ public void setMixerChannelConfiguration(MixerChannelConfiguration entry) throws { mControllerBroadcaster.broadcast(CONFIGURATION_CHANGE_STARTED); - if(mProcessingTask != null) - { - mProcessingTask.cancel(true); - } + mAudioOutputLock.lock(); - for(AudioOutput audioOutput: mAudioOutputs) + try { - audioOutput.dispose(); - } + for(AudioOutput audioOutput: mAudioOutputs) + { + audioOutput.dispose(); + } - mAudioOutputs.clear(); + mAudioOutputs.clear(); - switch(entry.getMixerChannel()) + switch(entry.getMixerChannel()) + { + case MONO: + AudioOutput mono = new MonoAudioOutput(entry.getMixer(), mUserPreferences); + mAudioOutputs.add(mono); + break; + case STEREO: + AudioOutput left = new StereoAudioOutput(entry.getMixer(), MixerChannel.LEFT, mUserPreferences); + mAudioOutputs.add(left); + + AudioOutput right = new StereoAudioOutput(entry.getMixer(), MixerChannel.RIGHT, mUserPreferences); + mAudioOutputs.add(right); + break; + default: + throw new AudioException("Unsupported mixer channel configuration: " + entry.getMixerChannel()); + } + } + finally { - case MONO: - AudioOutput mono = new MonoAudioOutput(entry.getMixer(), mUserPreferences); - mAudioOutputs.add(mono); - break; - case STEREO: - AudioOutput left = new StereoAudioOutput(entry.getMixer(), MixerChannel.LEFT, mUserPreferences); - mAudioOutputs.add(left); - - AudioOutput right = new StereoAudioOutput(entry.getMixer(), MixerChannel.RIGHT, mUserPreferences); - mAudioOutputs.add(right); - break; - default: - throw new AudioException("Unsupported mixer channel configuration: " + entry.getMixerChannel()); + mAudioOutputLock.unlock(); } - mProcessingTask = mScheduledExecutorService.scheduleAtFixedRate(new AudioSegmentProcessor(), - 0, 100, TimeUnit.MILLISECONDS); mControllerBroadcaster.broadcast(CONFIGURATION_CHANGE_COMPLETE); mMixerChannelConfiguration = entry; } @@ -389,4 +414,30 @@ public void run() } } } + + /** + * Audio segment comparator for sorting audio segments by: 1)Playback priority and 2)Segment start time + */ + public class AudioSegmentPrioritySorter implements Comparator + { + @Override + public int compare(AudioSegment segment1, AudioSegment segment2) + { + if(segment1 == null || segment2 == null) + { + return -1; + } + + //If priority is the same, sort by start time + if(segment1.monitorPriorityProperty().get() == segment2.monitorPriorityProperty().get()) + { + return Long.compare(segment1.getStartTimestamp(), segment2.getStartTimestamp()); + } + //Otherwise, sort by priority + else + { + return Integer.compare(segment1.monitorPriorityProperty().get(), segment2.monitorPriorityProperty().get()); + } + } + } }