Bug: Logcat exception output

[ryanheise]

Bug: Logcat exception output

[ryanheise]

I see, I suspected something like that. I think this is a good reason why it would be convenient to be able to stop in the middle of connecting. But unfortunately there are a number of other things that need to be implemented first.

In the meantime, I would suggest that you queue up requests on the player and process the queue serially with await. Hopefully that makes sense.

[ryanheise]

Perhaps an alternative to this behaviour would be to make this method return a duration of null rather than throw an exception.

[ryanheise]

Can you provide the audio file and the steps to reproduce?

[rohansohonee]

mp3 audio files are being used.
The steps to reproduce are:

1. Play mp3.
2. Stop and play next mp3 immediately.
3. Then seek and jump back to previous mp3, that is when the exception occurs.

Due to this reason I guess i am not able to perform crossfade between tracks cause app just crashes.
I have two instances of audioplayer object but only one play thread is running. The minute I start a new play thread very very quickly it causes this exeption I think.

Is using a thread correct in this approach, like in this particular way?? i don't know really??

[rohansohonee]

I am referring to the PlayThread of just_audio in the native code.

I call play() method from dart which in turn starts the play thread on native side.

[rohansohonee]

I think you should readSampleData first then getSampleTime. And check if sampleSize is >0
What do you think??

[ryanheise]

In terms of the steps to reproduce, can you share the actual code? It is important to know the exact sequence of method calls, where await is being used, how many instances you're using, etc. Also, are the audio files local or remote?

Looking through the code, I can see that setUrl does not actually check that the play thread is stopped so that might be the problem here if calling setUrl soon after the previous track finished, and so I should insert an ensureStopped call at the top of setUrl.

[rohansohonee]

await audioplayer.setFilePath()
await tempplayer.setFilePath()
audioplayer.play()
//Skip to next song
await audioplayer.stop()
// Again calling setFilePath() for both players
// Skipping immediately to previous song
await audioplayer.stop()
await audioplayer.setFilePath()
await tempplayer.setFilePath()
audioplayer.play()

When performing the skip operations even in profile mode causes the exception. The reason is PlayThread that is running in native code is taking time to start and stop very quickly.

[rohansohonee]

With tempplayer i am only calling setFilePath and not even doing play yet.
Stopping and starting audioplayer quickly is always crashing the app.
Adding breakpoints only revealed that the last play command is run and then the illegal state exception occurs as mentioned in the above logcat output.

[ryanheise]

Sorry I edited my previous message without noticing your reply. I think calling ensureStopped at the top of setUrl should help. Reading your example though, I'm not clear on this bit:

//Skip to next song
await audioplayer.stop()
// Again calling setFilePath() for both players
// Skipping immediately to previous song
await audioplayer.stop()

Can you write out in code what you're doing between the two stops?

[rohansohonee]

await audioplayer.stop()
// setting new path
await audioplayer.setFilePath()
await tempplayer.setFilePath()
audioplayer.play()
await audioplayer.stop()
// again i am setting new path
await audioplayer.setFilePath()
await tempplayer.setFilePath()
audioplayer.play()

There is always a glitch in audio sound when calling play() and ofcourse the crash happens when doing stops and plays very quickly like this. (tested even in profile mode)

Let's see whether ensureStopped on top of setUrl works.

Can you also try ConcatenatingMediaSource like in exoplayer.

public final class ConcatenatingMediaSource
extends CompositeMediaSource<com.google.android.exoplayer2.source.ConcatenatingMediaSource.MediaSourceHolder>

Concatenates multiple MediaSources. The list of MediaSources can be modified during playback. It is valid for the same MediaSource instance to be present more than once in the concatenation. Access to this class is thread-safe.

[ryanheise]

Can you open a separate issue for the feature request of a composite media source? To help my workflow, it helps to have a separate issue for each feature request and a separate issue for each bug report. Perhaps you could close #3 also which duplicates a number of issues. Then, we can keep the present issue for resolving the IllegalStateException bug.

[rohansohonee]

Ok will do so.

[ryanheise]

With this bug, does it only occur when you involve the tempplayer.setFilePath? Also, what files are you setting in each case?

[ryanheise]

I have tried your code sequence with two mp3 files. When skipping to track B, I pass in the B filepath to setFilePath on both audio players, and when skipping backwards to track A, I pass the A filepath into both audio player.

However, I was unable to reproduce the bug, so there may be some other condition required to reproduce this. Can you share a minimal reproduction project?

[ryanheise]

Or, perhaps since you can reproduce it, have you tried inserting ensureStopped(); right after the precondition check in setUrl? i.e.:

	public void setUrl(final String url, final Result result) throws IOException {
		if (state != PlaybackState.none && state != PlaybackState.stopped && state != PlaybackState.completed) {
			throw new IllegalStateException("Can call setUrl only from none/stopped/completed states");
		}
		ensureStopped(); // ADD THIS

Anyway, if I look at what you're ultimately trying to do, I think if you fully implement the cross fade, then the same instance of AudioPlayer would not immediately try to initialise the next track because that's what the second instance is for, and this illegal state might be avoided. Even so, I still would be interested to know if ensureStopped prevents the illegal state from happening.

[rohansohonee]

Will add ensureStopped and test.

[rohansohonee]

Skipping songs back and forth slightly fast causes illegalstateexceptions such as:
"Illegal state: Can call stop only from playing, paused and buffering states"
This means even when i am in one of the legal states, doing fast skipping of songs will cause a delay and thus the illegal state gets formed.

[rohansohonee]

I also added ensureStopped

[rohansohonee]

Not getting this error anymore:

java.lang.IllegalStateException
android.media.MediaExtractor.getSampleTime(Native Method)
com.ryanheise.just_audio.AudioPlayer$PlayThread.run(AudioPlayer.java:458)

There was bug in my dart code. I was awaiting stop() which was in a function. That function I was not awaiting, that is why audio continued playing and thus caused this crash.

Now I am adding if conditions to all the await player code and testing.

[rohansohonee]

Since you are not returning any result the dart code keeps awaiting. Eg:

public void stop(final Result result) {
		switch (state) {
		case stopped:
			break;
		case completed:
			transition(PlaybackState.stopped);
			break;
		

The stopped and completed case do not return result thus at dart side it keeps awaiting for result.
This is tested with adding if conditions to check whether the state is legal.

[rohansohonee]

You need to return result.success(null) or something.

[rohansohonee]

I added the return statements and finally found the final issue.
We cannot add if condition for setFilePath at all since playbackstate is null.

Merging this pull request shall solve the issue:
#11

Also I will test and report.

[ryanheise]

Well spotted on the missed result.success. I'll fix it and commit.

[rohansohonee]

Just caught a new error:
Error: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.media.AudioTrack.pause()' on a null object reference

[rohansohonee]

After adding all the if statements and making sure i am not in illegal state and
also adding the return statements and
#11 using this fix

The new error is audioTrack is null!!

[ryanheise]

I've just committed those changes above. I'm about to have dinner, so I'll continue looking into this afterwards. Can you copy and paste the full stacktrace for the null pointer exception? Which line does it occur on?

[rohansohonee]

It's in the stop method:

case paused:
                synchronized (monitor) {
                    // It takes some time for the PlayThread to actually wind down
                    // so other methods that transition from the stopped state should
                    // wait for playThread == null with ensureStopped().
                    PlaybackState oldState = state;
                    transition(PlaybackState.stopped);
                    if (oldState == PlaybackState.paused) {
                        monitor.notifyAll();
                    } else {
                        if(audioTrack!=null)
                        audioTrack.pause();
                    }
                    new Thread(() -> {
                        ensureStopped();
                        handler.post(() -> result.success(null));
                    }).start();
                }
                break;

i just added a simple check for if(audioTrack!=null)

[ryanheise]

After adding the null check, were there any further errors?

[rohansohonee]

Yes the final error is this:
IllegalStateException("Can call stop only from playing, paused and buffering states");

[rohansohonee]

I am handling all the cases for stop yet I keep getting this exception.
Step to reproduce this:
Skip songs quickly i.e calls to stop() are done very quickly.

This causes the synchronized block here

case paused:
                synchronized (monitor) {
                    // It takes some time for the PlayThread to actually wind down
                    // so other methods that transition from the stopped state should
                    // wait for playThread == null with ensureStopped().
                    PlaybackState oldState = state;
                    transition(PlaybackState.stopped);
                    if (oldState == PlaybackState.paused) {
                        monitor.notifyAll();
                    } else {
                        if (audioTrack != null)
                            audioTrack.pause();
                    }...........

to cause this exception

[rohansohonee]

Is there a better way to handle PlayThread ????

[ryanheise]

I've just added the null check, and also modified the exception message to show the current state, because I'm curious what state you are calling "stop" from.

When you say you keep getting this exception, are you talking about IllegalStateException("Can call stop only from playing, paused and buffering states") or the NullPointerException?

As for whether a thread can be avoided, there is another way to implement it using asynchronous callbacks. The API docs actually recommend new code to be written that way. However, as some of the method calls involved in the processing pipeline are blocking, and Flutter would prefer you not to run blocking code on the main thread, it would still be necessary to use a separate thread for that blocking code. So, it may be possible to structure the code that way, but it wouldn't necessarily solve any problems.

[rohansohonee]

I am talking about IllegalStateException("Can call stop only from playing, paused and buffering states")

[rohansohonee]

And it has this IllegalStateException("Can call stop only from playing, paused and buffering states",null)

a null at the end. I guess the state is null thus the error!!

[ryanheise]

That's not the state, the new code in the latest commit looks like this:

throw new IllegalStateException("Can call stop only from playing, paused and buffering states (" + state + ")");

So the state should appear in parentheses, and if it isn't, you musn't have that code.

[rohansohonee]

Exception has occurred.
PlatformException (PlatformException(Illegal state: Can call setUrl only from none/stopped/completed states ::(connecting), null, null))

[rohansohonee]

I added "::" before the brackets just cause

[ryanheise]

Interesting, so basically you are calling stop right after calling setUrl and without waiting for setUrl to complete. This error shouldn't happen if you await the setUrl call before doing anything else. The error message is slightly wrong about which are valid states, although it is at least true that you can't call stop while setUrl is in progress (i.e. the connecting state).

(I think it would actually be better to be able to call stop from any state, but I figure that there are much more important things to work on first, and this illegal state exception can be avoided by waiting for setUrl to complete before doing anything else.)

[rohansohonee]

I have put await before setFilePath

[rohansohonee]

state is in connecting whenever the exception happens

I figure that there are much more important things to work on first

Ok

[ryanheise]

I've checked the code, and it should be impossible for this illegal state error to occur unless you call stop before setFilePath completes. So this would give the error:

player.setFilePath(...); // state becomes connecting
await player.stop(); // still connecting, so illegal state

And the error would go away with an await on the first line.

await player.setFilePath(...); // state is connecting
// now state is stopped
await player.stop(); // we're in a legal state, so it's fine

The problem, though, is that your stop call is unlikely to be right there in line 2, because why would anyone want to stop the player before even entering the playing state? So more likely, you have called stop asynchronously in some other part of code without regard for checking the current state to see if it's legal. You can inspect player.playbackState to ensure that it's no longer connecting, or you can listen to the stream to know when it's finished connecting.

[rohansohonee]

I am listening to player.playbackState and i also have await player.setFilePath(..) before await player.stop().

If I perform these operations very quickly only then do I get a state of connecting.

[ryanheise]

But why would you want to stop the player right after setting the file path? That was what I asked in my previous post.

[rohansohonee]

When I am skipping I stop the player and then setpath and play music.

now what i did was press skip multiple times, this then causes the error.

[ryanheise]

@rohansohonee The latest commit makes state transitions more permissive, so hopefully that should allow you to skip between tracks more easily.

[rohansohonee]

@ryanheise
Two exceptions:

• Exception has occurred.
  PlatformException (PlatformException(setUrl aborted, null, null))

• I am correctly handling all the if conditions at dart side here is a snippet of play:

@override
void onPlay(){
final state = audioPlayer.playbackState;
    if (state != AudioPlaybackState.none ||
        state != AudioPlaybackState.connecting) audioPlayer.play();
}

Even after handling for the none and connecting states it threw this error:
"Illegal state: Cannot call play from connecting or none states (connecting)"

[ryanheise]

I should have mentioned, you must now put a try/catch around setUrl since if you stop the player in the middle of this call, the call will fail. But if you catch that exception, it should work.

[ryanheise]

I've just updated it to return null.

[rohansohonee]

Version 0.0.5

Exception has occurred.
NoSuchMethodError (NoSuchMethodError: The method '_mulFromInteger' was called on null.
Receiver: null
Tried calling: _mulFromInteger(1000))

Consequences of null

[ryanheise]

Whether or not I make this method throw an exception or return null, you still need to handle that case in your own code.

[rohansohonee]

Not my code but yours because duration future is null and you have assigned null to duration

ms is null
So Duration (milliseconds:ms) causes the error

[ryanheise]

I see, can you try the latest commit?

[rohansohonee]

This exception still arises as I call onPlay() method in onSkipToNext()

@override
void onPlay(){
final state = audioPlayer.playbackState;
    if (state != AudioPlaybackState.none ||
        state != AudioPlaybackState.connecting) audioPlayer.play();
}

Even after handling for the none and connecting states it threw this error:

Exception has occurred.
PlatformException (PlatformException(Illegal state: Cannot call play from connecting or none states (connecting), null, null))

How do I solve this in my code??

(EDIT)
Could this be the issue??
Your comment in dart code:

// TODO: It will be more reliable to let the platform
// side wait for completion since events on the flutter
// side can lag behind the platform side.

[ryanheise]

There are several issues here. First, to achieve the behaviour you want, you should not call onPlay from onSkipToNext as mentioned above because calling it will not cause the platform side to do its thing which you need. You need to instead trigger the play from the client if that's the behaviour you want.

Second, can you tell me whether the "ms is null" issue is resolved? It sounds like it's not, but if it is, please let me know.

The third issue about the flutter state lagging behind the platform state is likely an issue. Currently, you'll need to await setUrl and only after the returned future completes should you call play.

[rohansohonee]

ms is null solved.

You need to instead trigger the play from the client if that's the behaviour you want

how to do this?? I am not able to understand how i can trigger the play from client. All i know is calling audioplayer.play()

[ryanheise]

How did user trigger the skip to next?

[rohansohonee]

from the notification

[rohansohonee]

AudioService.java

@Override
		public void onPause() {
			if (listener == null) return;
			listener.onPause();
			unregisterNoisyReceiver();
			if (androidStopForegroundOnPause) {
				stopForeground(false);
			}
		}

@Override
		public void onSkipToNext() {
			if (listener == null) return;
			listener.onSkipToNext();
		}

stopForeground(false) occurs in onPause() and then user hits skip to next which calls onSkipToNext() but stopForeground(false) still remains. That is why notification can be dismissed.

[ryanheise]

I just noticed that you're asking about audio_service which is a discussion which should take place in the other issue. Over there, I said:

You have several clients:

1. The Flutter app
2. The notification or control center
3. The headset

In each of these, it is possible to control play/pause as well as skip to next/previous. Since you said that the scenario starts off in a paused state and then the user triggers a skip to next, through one of these 3 clients, the user should be able to use that same client to click the play button.

The reason I asked you which client you used to trigger the skip is because you can use the very same client to trigger the play.

It is difficult because this discussion belongs over there, but the answer was given over there on that issue. See the last comment on that issue:

Thanks for that. Seeing the full context, I can confirm that it is as I said before, and you can solve it in the way I said before. When I get a chance, I'll update the official example to demonstrate this.

Basically:

Calling onPlay in your background task does not go through the platform side which is what you need to happen. The only way to do this is by invoking play from a client. skipToNext will not in itself cause play to happen, so the client side must sequentially call skipToNext then play if you want this behaviour.

The combination of these two answers should explain the way it must be done. The user triggered the skip by clicking the skip button in the notification. Therefore, the presence of that notification means that the user can also trigger a play by clicking the play button. You should not call onPlay directly to achieve the behaviour you want. Instead, you need the user to trigger the play by clicking the play button on the client.

[rohansohonee]

But the user has closed the app and is running music in background. So there is no play button. The only buttons available are previous, pause and skip in the notification.

[rohansohonee]

Anyhow I fixed it in java code of audio_service
here

[ryanheise]

At this point, I can only copy and paste my previous answers:

You have several clients:

1. The Flutter app
2. The notification or control center
3. The headset

In each of these, it is possible to control play/pause as well as skip to next/previous. Since you said that the scenario starts off in a paused state and then the user triggers a skip to next, through one of these 3 clients, the user should be able to use that same client to click the play button.

Even though you have swiped (1) away, (2) is still a client. I asked you which client the user used to press the skip button, and you told me it is (2), and what I'm saying is that you should use the same client to trigger the play button. In other words, the user needs to click on the play button in (2) to trigger onPlay. So when you say there is no play button, I don't know why you say that. There is a play button in the same place where the user clicked on the skip button.

[rohansohonee]

I fixed the issue @ryanheise

here is the pullrequest

[ryanheise]

I am closing this issue, but please continue the discussion on the audio_service issue since it relates to that project, not this one. That issue also provides some important context about the way audio_service works.

[rohansohonee]

@ryanheise I solved the #140 from audio_service and I have put a pullrequest.

The issue that persists is this:
Exception has occurred. PlatformException (PlatformException(Illegal state: Cannot call play from connecting or none states (connecting), null, null))
This might be because of your comment you added in just_audio
// TODO: It will be more reliable to let the platform
// side wait for completion since events on the flutter
// side can lag behind the platform side.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs, or use StackOverflow if you need help with just_audio.