From 19d18b96b3d0d16508105697cca49af9fc18f20b Mon Sep 17 00:00:00 2001 From: Denny Sheirer Date: Sun, 1 Nov 2026 07:22:39 -0500 Subject: [PATCH] #2333 Fixes call audio prioritization (#2555) Co-authored-by: denny --- .../dsheirer/audio/AbstractAudioModule.java | 10 +- .../audio/codec/mbe/JmbeAudioModule.java | 2 +- .../dsheirer/audio/playback/AudioChannel.java | 27 ++- .../audio/playback/AudioPlaybackManager.java | 49 +++++ .../audio/AudioSegmentRolloverTest.java | 59 ++++++ .../playback/AudioPlaybackPriorityTest.java | 170 ++++++++++++++++++ 6 files changed, 310 insertions(+), 7 deletions(-) create mode 100644 src/test/java/io/github/dsheirer/audio/AudioSegmentRolloverTest.java create mode 100644 src/test/java/io/github/dsheirer/audio/playback/AudioPlaybackPriorityTest.java diff --git a/src/main/java/io/github/dsheirer/audio/AbstractAudioModule.java b/src/main/java/io/github/dsheirer/audio/AbstractAudioModule.java index beae5d6e..b67832d1 100644 --- a/src/main/java/io/github/dsheirer/audio/AbstractAudioModule.java +++ b/src/main/java/io/github/dsheirer/audio/AbstractAudioModule.java @@ -35,7 +35,7 @@ import org.slf4j.LoggerFactory; public abstract class AbstractAudioModule extends Module implements IAudioSegmentProvider, IdentifierUpdateListener { private final static Logger mLog = LoggerFactory.getLogger(AbstractAudioModule.class); - public static final long DEFAULT_SEGMENT_AUDIO_SAMPLE_LENGTH = 60 * 8000; // 1 minute @ 8kHz + public static final long DEFAULT_MAX_SEGMENT_LENGTH_MILLIS = 60_000; // 1 minute public static final int DEFAULT_TIMESLOT = 0; private final int mMaxSegmentAudioSampleLength; private Listener mAudioSegmentListener; @@ -51,12 +51,12 @@ public abstract class AbstractAudioModule extends Module implements IAudioSegmen * Constructs an abstract audio module * * @param aliasList for aliasing identifiers - * @param maxSegmentAudioSampleLength in milliseconds + * @param maxSegmentLengthMillis maximum audio segment length in milliseconds */ - public AbstractAudioModule(AliasList aliasList, int timeslot, long maxSegmentAudioSampleLength) + public AbstractAudioModule(AliasList aliasList, int timeslot, long maxSegmentLengthMillis) { mAliasList = aliasList; - mMaxSegmentAudioSampleLength = (int)(maxSegmentAudioSampleLength * 8); //Convert milliseconds to samples + mMaxSegmentAudioSampleLength = (int)(maxSegmentLengthMillis * 8); //Convert milliseconds to samples mTimeslot = timeslot; mIdentifierCollection = new MutableIdentifierCollection(getTimeslot()); mIdentifierUpdateNotificationBroadcaster.addListener(mIdentifierCollection); @@ -67,7 +67,7 @@ public abstract class AbstractAudioModule extends Module implements IAudioSegmen */ public AbstractAudioModule(AliasList aliasList) { - this(aliasList, DEFAULT_TIMESLOT, DEFAULT_SEGMENT_AUDIO_SAMPLE_LENGTH); + this(aliasList, DEFAULT_TIMESLOT, DEFAULT_MAX_SEGMENT_LENGTH_MILLIS); } /** diff --git a/src/main/java/io/github/dsheirer/audio/codec/mbe/JmbeAudioModule.java b/src/main/java/io/github/dsheirer/audio/codec/mbe/JmbeAudioModule.java index 614b21fb..d17b81b9 100644 --- a/src/main/java/io/github/dsheirer/audio/codec/mbe/JmbeAudioModule.java +++ b/src/main/java/io/github/dsheirer/audio/codec/mbe/JmbeAudioModule.java @@ -59,7 +59,7 @@ public abstract class JmbeAudioModule extends AbstractAudioModule implements Lis public JmbeAudioModule(UserPreferences userPreferences, AliasList aliasList, int timeslot) { - super(aliasList, timeslot, DEFAULT_SEGMENT_AUDIO_SAMPLE_LENGTH); + super(aliasList, timeslot, DEFAULT_MAX_SEGMENT_LENGTH_MILLIS); mUserPreferences = userPreferences; MyEventBus.getGlobalEventBus().register(this); loadConverter(); diff --git a/src/main/java/io/github/dsheirer/audio/playback/AudioChannel.java b/src/main/java/io/github/dsheirer/audio/playback/AudioChannel.java index ab2618a3..d434f4b0 100644 --- a/src/main/java/io/github/dsheirer/audio/playback/AudioChannel.java +++ b/src/main/java/io/github/dsheirer/audio/playback/AudioChannel.java @@ -46,6 +46,12 @@ public class AudioChannel implements Listener * sample rate. */ public static final int SAMPLES_PER_INTERVAL = 160; + /** + * Quantity of consecutive intervals without new audio (1 second) before an incomplete audio segment is treated as + * stalled and abandoned. An audio segment is delivered once to a single channel, so abandoning a segment that is + * still live loses the rest of the call. This has to tolerate decoder gaps, such as a missed P25 LDU (180 ms). + */ + static final int MAX_STALLED_INTERVALS = 50; private static final Logger LOGGER = LoggerFactory.getLogger(AudioChannel.class); private final AudioBuffer mAudioBuffer = new AudioBuffer(); private final Broadcaster mAudioEventBroadcaster = new Broadcaster<>(); @@ -225,7 +231,7 @@ public class AudioChannel implements Listener } //Dispose completed/fully-played audio segments and segments that have stalled/failed to progress - if(mCurrentAudioSegment != null && ((mNoAudioFromSegmentIntervalCount >= 6) || + if(mCurrentAudioSegment != null && ((mNoAudioFromSegmentIntervalCount >= MAX_STALLED_INTERVALS) || (mCurrentAudioSegment.isComplete() && (mCurrentBufferIndex >= mCurrentAudioSegment.getAudioBufferCount())))) { disposeCurrentAudioSegment(); @@ -401,6 +407,24 @@ public class AudioChannel implements Listener return mAudioSegmentQueue.isEmpty(); } + /** + * Next audio segment queued for playback, without removing it from the queue. + * @return queued audio segment or null if the queue is empty. + */ + AudioSegment peekQueuedAudioSegment() + { + return mAudioSegmentQueue.peek(); + } + + /** + * Removes the next audio segment from the queue, transferring it (and its consumer count) to the caller. + * @return queued audio segment or null if the queue is empty. + */ + AudioSegment pollQueuedAudioSegment() + { + return mAudioSegmentQueue.poll(); + } + /** * Schedules the audio segment for playback. The audio segment user count should already be incremented by the * calling entity. @@ -475,6 +499,7 @@ public class AudioChannel implements Listener } mCurrentBufferIndex = -1; + mNoAudioFromSegmentIntervalCount = 0; } /** 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 79ea7976..e5aaaf0c 100644 --- a/src/main/java/io/github/dsheirer/audio/playback/AudioPlaybackManager.java +++ b/src/main/java/io/github/dsheirer/audio/playback/AudioPlaybackManager.java @@ -173,6 +173,7 @@ public class AudioPlaybackManager implements Listener, IAudioContr { Iterator it = mAudioSegments.iterator(); AudioSegment audioSegment; + List displaced = new ArrayList<>(); //Remove any audio segments flagged as do not monitor. Don't remove completed segments yet, because //we want to give them a brief chance at playback. Automatically assign linked audio segments to the @@ -200,6 +201,16 @@ public class AudioPlaybackManager implements Listener, IAudioContr if(audioOutput.isLinkedTo(audioSegment)) { it.remove(); + + //Linked audio continues the current call, so it goes ahead of anything queued + AudioSegment queued = audioOutput.pollQueuedAudioSegment(); + + while(queued != null) + { + displaced.add(queued); + queued = audioOutput.pollQueuedAudioSegment(); + } + audioOutput.play(audioSegment); } } @@ -212,6 +223,8 @@ public class AudioPlaybackManager implements Listener, IAudioContr } } + mAudioSegments.addAll(displaced); + //Sort audio segments by playback priority and assign to empty audio outputs if(!mAudioSegments.isEmpty() && mAudioOutput != null) { @@ -232,6 +245,8 @@ public class AudioPlaybackManager implements Listener, IAudioContr } } } + + replaceLowerPriorityQueuedSegments(mAudioSegments, mAudioOutput.getAudioProvider().getAudioChannels()); } finally { @@ -255,6 +270,40 @@ public class AudioPlaybackManager implements Listener, IAudioContr } } + /** + * Swaps audio segments queued on the audio channels for waiting audio segments that have a higher playback priority, + * so that a segment queued behind the current playback can't hold a higher priority segment out of the channel. + * Audio that is currently playing is not interrupted. Displaced segments are returned to the waiting list. + * + * @param waiting audio segments, sorted by playback priority. + * @param audioChannels to evaluate. + */ + static void replaceLowerPriorityQueuedSegments(List waiting, List audioChannels) + { + for(AudioChannel audioChannel: audioChannels) + { + if(waiting.isEmpty()) + { + return; + } + + AudioSegment queued = audioChannel.peekQueuedAudioSegment(); + + if(queued != null && + waiting.getFirst().monitorPriorityProperty().get() < queued.monitorPriorityProperty().get()) + { + //The audio thread may have taken the queued segment since the peek, so only return what we removed. + queued = audioChannel.pollQueuedAudioSegment(); + audioChannel.play(waiting.removeFirst()); + + if(queued != null) + { + waiting.add(queued); + } + } + } + } + public void dispose() { MyEventBus.getGlobalEventBus().unregister(this); diff --git a/src/test/java/io/github/dsheirer/audio/AudioSegmentRolloverTest.java b/src/test/java/io/github/dsheirer/audio/AudioSegmentRolloverTest.java new file mode 100644 index 00000000..7adb4afc --- /dev/null +++ b/src/test/java/io/github/dsheirer/audio/AudioSegmentRolloverTest.java @@ -0,0 +1,59 @@ +/* + * ***************************************************************************** + * Copyright (C) 2014-2026 Dennis Sheirer + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see + * **************************************************************************** + */ + +package io.github.dsheirer.audio; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.github.dsheirer.alias.AliasList; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Tests audio segment rollover at the maximum audio segment length. + */ +public class AudioSegmentRolloverTest +{ + private static final float[] ONE_SECOND_OF_AUDIO = new float[8000]; + + @Test + public void defaultMaximumSegmentLengthIsOneMinute() + { + AudioModule module = new AudioModule(new AliasList("test"), Collections.emptyList()); + List segments = new ArrayList<>(); + module.setAudioSegmentListener(segments::add); + + for(int seconds = 0; seconds < 60; seconds++) + { + module.addAudio(ONE_SECOND_OF_AUDIO); + } + + assertEquals(1, segments.size(), "A full minute of audio should fit in a single segment"); + + module.addAudio(ONE_SECOND_OF_AUDIO); + + assertEquals(2, segments.size(), "Audio beyond one minute should roll over to a new segment"); + assertEquals(60_000, segments.get(0).getDuration()); + assertTrue(segments.get(0).isComplete(), "First segment should be closed"); + assertTrue(segments.get(1).isLinkedTo(segments.get(0)), "New segment should link to the first segment"); + } +} diff --git a/src/test/java/io/github/dsheirer/audio/playback/AudioPlaybackPriorityTest.java b/src/test/java/io/github/dsheirer/audio/playback/AudioPlaybackPriorityTest.java new file mode 100644 index 00000000..e512f0d6 --- /dev/null +++ b/src/test/java/io/github/dsheirer/audio/playback/AudioPlaybackPriorityTest.java @@ -0,0 +1,170 @@ +/* + * ***************************************************************************** + * Copyright (C) 2014-2026 Dennis Sheirer + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see + * **************************************************************************** + */ + +package io.github.dsheirer.audio.playback; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.github.dsheirer.alias.AliasList; +import io.github.dsheirer.audio.AudioSegment; +import io.github.dsheirer.preference.UserPreferences; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Tests that a higher priority audio segment isn't displaced by lower priority audio during playback (issue #2333). + */ +public class AudioPlaybackPriorityTest +{ + private static final float[] AUDIO = new float[AudioChannel.SAMPLES_PER_INTERVAL]; + private AudioChannel mChannel; + + @BeforeEach + public void setup() + { + mChannel = new AudioChannel(new UserPreferences(), "TEST"); + } + + @AfterEach + public void teardown() + { + mChannel.dispose(); + } + + private static AudioSegment segment(int priority) + { + AudioSegment segment = new AudioSegment(new AliasList("test"), 0); + segment.incrementConsumerCount(); + segment.monitorPriorityProperty().set(priority); + segment.addAudio(AUDIO); + return segment; + } + + /** + * Pulls audio until the buffered audio (start tone and first frame) is exhausted and the channel is waiting on the + * audio segment for more audio. + */ + private void drainBufferedAudio() + { + while(mChannel.getAudio() != null) + { + //Discard buffered audio + } + } + + private void pullIntervals(int count) + { + for(int x = 0; x < count; x++) + { + mChannel.getAudio(); + } + } + + @Test + public void liveSegmentSurvivesDecoderGapWithLowerPriorityQueued() + { + AudioSegment priority1 = segment(1); + AudioSegment priority2 = segment(2); + mChannel.play(priority1); + mChannel.getAudio(); + mChannel.play(priority2); + drainBufferedAudio(); + + //300 ms without audio exceeds a missed P25 LDU + pullIntervals(15); + + assertTrue(mChannel.hasAudioSegment(), "Live segment should still be assigned"); + assertSame(priority2, mChannel.peekQueuedAudioSegment(), "Queued lower priority segment should not have started"); + + priority1.addAudio(AUDIO); + assertTrue(mChannel.getAudio() != null, "Audio arriving after the gap should play"); + } + + @Test + public void stalledSegmentIsAbandonedSoChannelIsReleased() + { + mChannel.play(segment(1)); + mChannel.getAudio(); + drainBufferedAudio(); + pullIntervals(AudioChannel.MAX_STALLED_INTERVALS + 1); + + assertFalse(mChannel.hasAudioSegment(), "Stalled segment should be released"); + } + + @Test + public void completedSegmentIsReleasedWithoutWaitingForStallTimeout() + { + AudioSegment segment = segment(1); + mChannel.play(segment); + mChannel.getAudio(); + segment.completeProperty().set(true); + drainBufferedAudio(); + pullIntervals(2); + + assertFalse(mChannel.hasAudioSegment(), "Completed segment should be released"); + } + + @Test + public void higherPriorityWaitingSegmentReplacesLowerPriorityQueuedSegment() + { + AudioSegment priority1 = segment(1); + AudioSegment priority3 = segment(3); + mChannel.play(priority3); + List waiting = new ArrayList<>(List.of(priority1)); + + AudioPlaybackManager.replaceLowerPriorityQueuedSegments(waiting, List.of(mChannel)); + + assertSame(priority1, mChannel.peekQueuedAudioSegment()); + assertEquals(List.of(priority3), waiting, "Displaced segment should return to the waiting list"); + } + + @Test + public void equalAndLowerPriorityWaitingSegmentsDoNotReplaceQueuedSegment() + { + AudioSegment queued = segment(2); + mChannel.play(queued); + + for(int priority: new int[]{2, 3}) + { + AudioSegment other = segment(priority); + List waiting = new ArrayList<>(List.of(other)); + AudioPlaybackManager.replaceLowerPriorityQueuedSegments(waiting, List.of(mChannel)); + + assertSame(queued, mChannel.peekQueuedAudioSegment()); + assertEquals(List.of(other), waiting); + } + } + + @Test + public void noReplacementWhenNothingIsQueued() + { + List waiting = new ArrayList<>(List.of(segment(1))); + AudioPlaybackManager.replaceLowerPriorityQueuedSegments(waiting, List.of(mChannel)); + + assertNull(mChannel.peekQueuedAudioSegment()); + assertEquals(1, waiting.size()); + } +}