just_audio icon indicating copy to clipboard operation
just_audio copied to clipboard

Fix error handling on play without load

Open addie9000 opened this issue 4 years ago • 5 comments

When play without load like below, no error reported from native platform.

final player = AudioPlayer();
await player.setAudioSource(<NotFoundAudioSource>, preload = false);
player.playbackEventStream.listen((d) => print('data: $d'), onError: (e) => print('error: $e'));
await player.play();
// should print 'error: PlatformException(...)' after play, but it doesn't.

This is because before playbackEventStream reports error, platform is deactivated and the stream is canceled. The deactivation is caused by error handling on duration report logic in setPlatform. https://github.com/ryanheise/just_audio/blob/2a22b69db6e7696a615c96ecf456d42022d41894/just_audio/lib/just_audio.dart#L1356

Also fixed stuck in playing after such error occurs.

ref #390

addie9000 avatar Oct 14 '21 07:10 addie9000

@ryanheise Personally I do not use 'play without load' anymore, thus this is not blocking problem for me. However I believe this could be helpful to someone who (like #390, and myself in the future) uses 'play without load', so if this PR looks OK, could you merge it?

addie9000 avatar Feb 07 '22 11:02 addie9000

I'm not sure if I fully understand whether this is correct. The original code would only call _setPlatformActive(false) if load failed. Your new code will do it for any error, even for example when load initially succeeds, the first 2 items in a ConcatenatingAudioSource play without error, but the 3rd item produces an error. I don't think we want the platform to be deactivated in that case. Similarly, if load succeeds, and it starts playing, but then the network connection is lost and we get an error, we also don't want the platform to be deactivated then. The only time we do want to do this is when the platform was originally inactive, and the attempt to transition to active via a load failed, in which case it should be returned to inactive.

Did I misunderstand something?

ryanheise avatar Feb 07 '22 13:02 ryanheise

Ah, probably you are right.

So, the problem is that we cannot detect any error on play after non-preloaded setAudioSource. On the other hand, it seems that at least we can detect the error on load with the same circumstance, because load returns durationCompleter.future which report duration load error. (The source error of hiding platform error by set platform inactive.) How about change the play like below to report duration load error?

https://github.com/ryanheise/just_audio/blob/b1a7d5099072f92855b7c36815c2709cb92490c6/just_audio/lib/just_audio.dart#L874-L877

// If the native platform wasn't already active, activating it will
// implicitly restore the playing state and send a play request.
_setPlatformActive(true, playCompleter: playCompleter)
  ?.catchError((dynamic e) {
     // report error to play completer.
     playCompleter.completeError(e);
});

I haven't tested anything about this, if the way of correction seems right, I'll test it and update this PR.

addie9000 avatar Feb 08 '22 03:02 addie9000

Maybe something like that will work, it's worth a try.

ryanheise avatar Feb 08 '22 03:02 ryanheise