Skip to content

Commit

Permalink
fix: Timeout on setting same source twice (#1520)
Browse files Browse the repository at this point in the history
# Description

When setting the same source twice for a player, the `onPrepared` event
was not sent on most platform implementations.
Also fixing listening and canceling event stream on Android multiple times.
  • Loading branch information
Gustl22 authored May 28, 2023
1 parent 9f39e95 commit 5d164d1
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 11 deletions.
72 changes: 63 additions & 9 deletions packages/audioplayers/example/integration_test/lib_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ void main() {
final isAndroid = !kIsWeb && Platform.isAndroid;
final isLinux = !kIsWeb && Platform.isLinux;

// FIXME(gustl22): Cannot reuse event channel with same id on Linux (flutter/flutter#126209)
var linuxPlayerCount = 0;

final wavUrl1TestData = LibSourceTestData(
source: UrlSource(wavUrl1),
duration: const Duration(milliseconds: 451),
Expand Down Expand Up @@ -245,8 +248,7 @@ void main() {
testWidgets('Emit platform log', (tester) async {
final logCompleter = Completer<String>();

// FIXME(gustl22): Cannot reuse event channel with same id on Linux (flutter/flutter#126209)
final playerId = isLinux ? 'somePlayerId0' : 'somePlayerId';
final playerId = 'somePlayerId${isLinux ? linuxPlayerCount++ : ''}';
final player = AudioPlayer(playerId: playerId);
final onLogSub = player.onLog.listen(
logCompleter.complete,
Expand Down Expand Up @@ -331,8 +333,7 @@ void main() {
testWidgets('#create and #dispose', (tester) async {
final platform = AudioplayersPlatformInterface.instance;

// FIXME(gustl22): Cannot reuse event channel with same id on Linux (flutter/flutter#126209)
final playerId = isLinux ? 'somePlayerId1' : 'somePlayerId';
final playerId = 'somePlayerId${isLinux ? linuxPlayerCount++ : ''}';
await platform.create(playerId);
await tester.pumpAndSettle();
await platform.dispose(playerId);
Expand All @@ -353,8 +354,7 @@ void main() {
testWidgets('#setSource #getPosition and #getDuration', (tester) async {
final platform = AudioplayersPlatformInterface.instance;

// FIXME(gustl22): Cannot reuse event channel with same id on Linux (flutter/flutter#126209)
final playerId = isLinux ? 'somePlayerId2' : 'somePlayerId';
final playerId = 'somePlayerId${isLinux ? linuxPlayerCount++ : ''}';
await platform.create(playerId);

final preparedCompleter = Completer<void>();
Expand Down Expand Up @@ -392,10 +392,65 @@ void main() {
await platform.dispose(playerId);
}
});

testWidgets('Set same source twice (#1520)', (tester) async {
final platform = AudioplayersPlatformInterface.instance;

final playerId = 'somePlayerId${isLinux ? linuxPlayerCount++ : ''}';
await platform.create(playerId);

final eventStream = platform.getEventStream(playerId);
for (var i = 0; i < 2; i++) {
final preparedCompleter = Completer<void>();
final onPreparedSub = eventStream
.where((event) => event.eventType == AudioEventType.prepared)
.map((event) => event.isPrepared!)
.listen(
(isPrepared) {
if (isPrepared) {
preparedCompleter.complete();
}
},
onError: preparedCompleter.completeError,
);
if (isLinux) {
// FIXME(gustl22): Linux needs additional pump (#1507)
await tester.pump();
}
await platform.setSourceUrl(
playerId,
(wavUrl1TestData.source as UrlSource).url,
);
await preparedCompleter.future.timeout(const Duration(seconds: 30));
await onPreparedSub.cancel();
}
if (!isLinux) {
// FIXME(gustl22): Linux not disposing properly (#1507)
await platform.dispose(playerId);
}
});
});

group('Platform event channel', () {
// TODO(gustl22): remove once https://github.com/flutter/flutter/issues/126209 is fixed
testWidgets('Listen and cancel twice', (tester) async {
final platform = AudioplayersPlatformInterface.instance;

final playerId = 'somePlayerId${isLinux ? linuxPlayerCount++ : ''}';
await platform.create(playerId);

final eventStream = platform.getEventStream(playerId);
for (var i = 0; i < 2; i++) {
final eventSub = eventStream.listen(null);
await eventSub.cancel();
}
if (!isLinux) {
// FIXME(gustl22): Linux not disposing properly (#1507)
await platform.dispose(playerId);
}
});

// TODO(gustl22): remove once https://github.com/flutter/flutter/issues/126209
// is fixed, as tests should cover the problem in flutter engine.
testWidgets(
'Reuse same platform event channel id',
(tester) async {
Expand Down Expand Up @@ -425,8 +480,7 @@ void main() {
final errorCompleter = Completer<Object>();
final platform = AudioplayersPlatformInterface.instance;

// FIXME(gustl22): Cannot reuse event channel with same id on Linux (flutter/flutter#126209)
final playerId = isLinux ? 'somePlayerId3' : 'somePlayerId';
final playerId = 'somePlayerId${isLinux ? linuxPlayerCount++ : ''}';
await platform.create(playerId);

final eventStreamSub = platform
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,6 @@ class EventHandler(private val eventChannel: EventChannel) : EventChannel.Stream

override fun onCancel(arguments: Any?) {
eventSink = null
eventChannel.setStreamHandler(null)
}

fun success(method: String, arguments: Map<String, Any> = HashMap()) {
Expand All @@ -369,5 +368,6 @@ class EventHandler(private val eventChannel: EventChannel) : EventChannel.Stream
it.endOfStream()
onCancel(null)
}
eventChannel.setStreamHandler(null)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class WrappedPlayer internal constructor(
playing = false
player?.release()
}
} else {
ref.handlePrepared(this, true)
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/audioplayers_linux/linux/audio_player.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ void AudioPlayer::SetSourceUrl(std::string url) {
}
}
}
} else {
this->OnPrepared(true);
}
}

Expand Down
8 changes: 7 additions & 1 deletion packages/audioplayers_web/lib/wrapped_player.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,13 @@ class WrappedPlayer {

Future<void> setUrl(String url) async {
if (_currentUrl == url) {
return; // nothing to do
eventStreamController.add(
const AudioEvent(
eventType: AudioEventType.prepared,
isPrepared: true,
),
);
return;
}
_currentUrl = url;

Expand Down
2 changes: 2 additions & 0 deletions packages/audioplayers_windows/windows/audio_player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ void AudioPlayer::SetSourceUrl(std::string url) {
// Forward errors to event stream, as this is called asynchronously
this->OnError("WindowsAudioError", "Error setting url to '" + url + "'.", nullptr);
}
} else {
OnPrepared(true);
}
}

Expand Down

0 comments on commit 5d164d1

Please sign in to comment.