Skip to content

Commit

Permalink
fix(android): Released wrong source in LOW_LATENCY mode (#1672)
Browse files Browse the repository at this point in the history
# Description

Fix issue where source field in WrappedPlayer is updated before SoundPoolPlayer calls
release(), which should affect previous source, not the new source.

---------

Co-authored-by: Gustl22 <git@reb0.org>
  • Loading branch information
jasharpe and Gustl22 committed Nov 14, 2023
1 parent 7c9f0d6 commit d9c5f69
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 47 deletions.
Expand Up @@ -52,8 +52,13 @@ final mpgaUrlTestData = LibSourceTestData(
duration: null,
);

final wavAssetTestData = LibSourceTestData(
source: AssetSource(wavAsset),
final wavAsset1TestData = LibSourceTestData(
source: AssetSource(wavAsset1),
duration: const Duration(milliseconds: 451),
);

final wavAsset2TestData = LibSourceTestData(
source: AssetSource(wavAsset2),
duration: const Duration(seconds: 1, milliseconds: 068),
);

Expand Down Expand Up @@ -95,15 +100,15 @@ Future<List<LibSourceTestData>> getAudioTestDataList() async {
if (_features.hasUrlSource && _features.hasPlaylistSourceType)
m3u8UrlTestData,
if (_features.hasUrlSource) mpgaUrlTestData,
if (_features.hasAssetSource) wavAssetTestData,
if (_features.hasAssetSource) wavAsset2TestData,
/*if (_features.hasAssetSource)
LibSourceTestData(
source: AssetSource(mp3Asset),
duration: const Duration(minutes: 1, seconds: 34, milliseconds: 119),
),*/
if (_features.hasBytesSource)
LibSourceTestData(
source: BytesSource(await AudioCache.instance.loadAsBytes(wavAsset)),
source: BytesSource(await AudioCache.instance.loadAsBytes(wavAsset2)),
duration: const Duration(seconds: 1, milliseconds: 068),
),
/*if (_features.hasBytesSource)
Expand Down
36 changes: 36 additions & 0 deletions packages/audioplayers/example/integration_test/lib_test.dart
Expand Up @@ -245,4 +245,40 @@ void main() async {
skip: !features.hasRespectSilence || !features.hasLowLatency,
);
});

group(
'Android only:',
() {
/// The test is auditory only!
/// It will succeed even if the wrong source is played.
testWidgets('Released wrong source on LOW_LATENCY (#1672)',
(WidgetTester tester) async {
var player = AudioPlayer()
..setPlayerMode(PlayerMode.lowLatency)
..setReleaseMode(ReleaseMode.stop);

await player.play(wavAsset1TestData.source);
await tester.pump(const Duration(seconds: 1));
await player.stop();

await player.play(wavAsset2TestData.source);
await tester.pump(const Duration(seconds: 1));
await player.stop();

player = AudioPlayer()
..setPlayerMode(PlayerMode.lowLatency)
..setReleaseMode(ReleaseMode.stop);

// This should play the new source, not the old one:
await player.play(wavAsset1TestData.source);
await tester.pump(const Duration(seconds: 1));
await player.stop();

await player.play(wavAsset2TestData.source);
await tester.pump(const Duration(seconds: 1));
await player.stop();
});
},
skip: !isAndroid,
);
}
Expand Up @@ -337,7 +337,7 @@ void main() async {
testData: td,
);

if (td.source == wavAssetTestData.source) {
if (td.source == wavAsset2TestData.source) {
await tester.pumpLinux();
}

Expand Down
13 changes: 7 additions & 6 deletions packages/audioplayers/example/lib/tabs/sources.dart
Expand Up @@ -28,7 +28,8 @@ final mpgaStreamUrl = useLocalServer
? '$host/stream/mpeg'
: 'https://timesradio.wireless.radio/stream';

const wavAsset = 'laser.wav';
const wavAsset1 = 'coins.wav';
const wavAsset2 = 'laser.wav';
const mp3Asset = 'nasa_on_a_mission.mp3';
const invalidAsset = 'invalid.txt';
const specialCharAsset = 'coins_non_ascii_и.wav';
Expand Down Expand Up @@ -154,20 +155,20 @@ class _SourcesTabState extends State<SourcesTab>
),
_createSourceTile(
setSourceKey: const Key('setSource-asset-wav'),
title: 'Asset 1',
title: 'Asset WAV',
subtitle: 'laser.wav',
source: AssetSource(wavAsset),
source: AssetSource(wavAsset2),
),
_createSourceTile(
setSourceKey: const Key('setSource-asset-mp3'),
title: 'Asset 2',
title: 'Asset MP3',
subtitle: 'nasa.mp3',
source: AssetSource(mp3Asset),
),
_SourceTile(
setSource: () => _setSourceBytesAsset(_setSource, asset: wavAsset),
setSource: () => _setSourceBytesAsset(_setSource, asset: wavAsset2),
setSourceKey: const Key('setSource-bytes-local'),
play: () => _setSourceBytesAsset(_play, asset: wavAsset),
play: () => _setSourceBytesAsset(_play, asset: wavAsset2),
removeSource: _removeSourceWidget,
title: 'Bytes - Local',
subtitle: 'laser.wav',
Expand Down
Expand Up @@ -41,9 +41,6 @@ class SoundPoolPlayer(

private var soundPoolWrapper: SoundPoolWrapper

val urlSource: UrlSource?
get() = wrappedPlayer.source as? UrlSource

private val soundPool: SoundPool
get() = soundPoolWrapper.soundPool

Expand Down Expand Up @@ -77,6 +74,7 @@ class SoundPoolPlayer(
playersForSoundId.remove(this)
}
this.soundId = null
this.urlSource = null
}
}

Expand All @@ -92,39 +90,40 @@ class SoundPoolPlayer(
source.setForSoundPool(this)
}

fun setUrlSource(urlSource: UrlSource) {
if (soundId != null) {
release()
}
synchronized(soundPoolWrapper.urlToPlayers) {
val urlPlayers = soundPoolWrapper.urlToPlayers.getOrPut(urlSource) { mutableListOf() }
val originalPlayer = urlPlayers.firstOrNull()

if (originalPlayer != null) {
// Sound has already been loaded - reuse the soundId.
val prepared = originalPlayer.wrappedPlayer.prepared
wrappedPlayer.prepared = prepared
soundId = originalPlayer.soundId
wrappedPlayer.handleLog("Reusing soundId $soundId for $urlSource is prepared=$prepared $this")
} else {
// First one for this URL - load it.
val start = System.currentTimeMillis()

wrappedPlayer.prepared = false
wrappedPlayer.handleLog("Fetching actual URL for $urlSource")
val actualUrl = urlSource.getAudioPathForSoundPool()
wrappedPlayer.handleLog("Now loading $actualUrl")
val intSoundId = soundPool.load(actualUrl, 1)
soundPoolWrapper.soundIdToPlayer[intSoundId] = this
soundId = intSoundId

wrappedPlayer.handleLog(
"time to call load() for $urlSource: ${System.currentTimeMillis() - start} player=$this",
)
var urlSource: UrlSource? = null
set(value) {
if (value != null) {
synchronized(soundPoolWrapper.urlToPlayers) {
val urlPlayers = soundPoolWrapper.urlToPlayers.getOrPut(value) { mutableListOf() }
val originalPlayer = urlPlayers.firstOrNull()

if (originalPlayer != null) {
// Sound has already been loaded - reuse the soundId.
val prepared = originalPlayer.wrappedPlayer.prepared
wrappedPlayer.prepared = prepared
soundId = originalPlayer.soundId
wrappedPlayer.handleLog("Reusing soundId $soundId for $value is prepared=$prepared $this")
} else {
// First one for this URL - load it.
val start = System.currentTimeMillis()

wrappedPlayer.prepared = false
wrappedPlayer.handleLog("Fetching actual URL for $value")
val actualUrl = value.getAudioPathForSoundPool()
wrappedPlayer.handleLog("Now loading $actualUrl")
val intSoundId = soundPool.load(actualUrl, 1)
soundPoolWrapper.soundIdToPlayer[intSoundId] = this
soundId = intSoundId

wrappedPlayer.handleLog(
"time to call load() for $value: ${System.currentTimeMillis() - start} player=$this",
)
}
urlPlayers.add(this)
}
}
urlPlayers.add(this)
field = value
}
}

override fun setVolume(leftVolume: Float, rightVolume: Float) {
streamId?.let { soundPool.setVolume(it, leftVolume, rightVolume) }
Expand Down
Expand Up @@ -27,7 +27,6 @@ class WrappedPlayer internal constructor(
var source: Source? = null
set(value) {
if (field != value) {
field = value
if (value != null) {
val player = getOrCreatePlayer()
player.setSource(value)
Expand All @@ -38,6 +37,7 @@ class WrappedPlayer internal constructor(
playing = false
player?.release()
}
field = value
} else {
ref.handlePrepared(this, true)
}
Expand Down
Expand Up @@ -17,7 +17,8 @@ data class UrlSource(
}

override fun setForSoundPool(soundPoolPlayer: SoundPoolPlayer) {
soundPoolPlayer.setUrlSource(this)
soundPoolPlayer.release()
soundPoolPlayer.urlSource = this
}

fun getAudioPathForSoundPool(): String {
Expand Down

0 comments on commit d9c5f69

Please sign in to comment.