#2086 Audio Playback Manager can leak memory if audio queue processin…

…g thread is never started.
DSheirer · Nov 8, 2024 · 6979814 · 6979814
1 parent 3bf425f
commit 6979814
Showing 1 changed file with 88 additions and 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
@@ -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<AudioSegment>, IAudioContr
     private LinkedTransferQueue<AudioSegment> 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<AudioSegment>
+    {
+        @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());
+            }
+        }
+    }
 }