Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix audio player state not becoming AudioPlayerState.IDLE when queue emptied #83

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

puckey
Copy link
Contributor

@puckey puckey commented Jun 29, 2023

  • when queue becomes empty, make player state become AudioPlayerState.IDLE (which denotes “No AudioItem is loaded and the player is doing nothing.”) instead of AudioPlayerState.ENDED which denotes Playback stopped due to the end of the queue being reached.
  • change docs for AudioPlayerState.ENDED
  • add test

- when queue becomes empty, make player state become `AudioPlayerState.IDLE` (which denotes “No AudioItem is loaded and the player is doing nothing.”) instead of `AudioPlayerState.ENDED` which denotes `Playback stopped due to the end of the queue being reached.`
- change docs for `AudioPlayerState.ENDED`
- add test
Copy link
Contributor

@dcvz dcvz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Nice catch!

@dcvz dcvz merged commit 5b82ce3 into doublesymmetry:main Jul 4, 2023
1 of 2 checks passed
@hschmied
Copy link

hschmied commented Jul 5, 2023

Am I correct, that this will change the current behavior (react-native-track-player v4.0.0-rc5) of abandoning the audio-focus?

Investigating my issue, I came across this code-portion in BaseAudioPlayer.kt ...

            if (value != field) {
                field = value
                playerEventHolder.updateAudioPlayerState(value)
                if (!playerConfig.handleAudioFocus) {
                    when (value) {
                        AudioPlayerState.IDLE,
                        AudioPlayerState.ERROR -> abandonAudioFocusIfHeld()
                        AudioPlayerState.READY -> requestAudioFocus()
                        else -> {}
                    }
                }
            }

If I understand it right, the player currently only returns focus, if either IDLE/ERROR state is set or destroy is called.... ENDED would have not abandoned the focus.

Great if that's the case and I finally have a lead to deal with my issue. Thanks :)

@hschmied
Copy link

hschmied commented Jul 5, 2023

And if I may ask -- if the player is paused/stopped, the AudioPlayerState is set to PAUSED/STOPPED and remains that or is the lifecycle to later become IDLE?

I guess if the player is playing Music/Main-Audio content, it would make sense to keep the focus, but if the player is used for 'secondary-/temporary-audio', I guess even if pause/stop is triggered, the focus should be returned.

@puckey
Copy link
Contributor Author

puckey commented Jul 5, 2023

@hschmied IDLE is only reached when the queue is empty

@puckey
Copy link
Contributor Author

puckey commented Jul 5, 2023

Am I correct, that this will change the current behavior (react-native-track-player v4.0.0-rc5) of abandoning the audio-focus?

Investigating my issue, I came across this code-portion in BaseAudioPlayer.kt ...

            if (value != field) {
                field = value
                playerEventHolder.updateAudioPlayerState(value)
                if (!playerConfig.handleAudioFocus) {
                    when (value) {
                        AudioPlayerState.IDLE,
                        AudioPlayerState.ERROR -> abandonAudioFocusIfHeld()
                        AudioPlayerState.READY -> requestAudioFocus()
                        else -> {}
                    }
                }
            }

If I understand it right, the player currently only returns focus, if either IDLE/ERROR state is set or destroy is called.... ENDED would have not abandoned the focus.

Great if that's the case and I finally have a lead to deal with my issue. Thanks :)

@hschmied I am curious – what bug were you seeing? We could add ENDED and STOPPED to the list of states where focus should be abandoned...

@hschmied
Copy link

hschmied commented Jul 5, 2023

Thank you for replying and taking the time; my issue is, that I am using rntp to queue and play spoken audio (content-type speech), intended for temporary focus only. Hence if music from another app is playing and my app requests focus, plays the short audio (maybe a second one, if queued) and when done abandon the focus, so the regular music or audio-book app can resume. (Another scenario would be that the user pauses/stops manually, in which case the focus also would have to return... but I digress).

With what I've tried so far, music playing in the background was halted, when my app triggered & played its audio, but once it was done, the music (or audiobook) from the 'other' app didn't resume. So my assumption is either my app communicated that I need the audio-focus permanent as 'music' or my app never communicated that I am releasing my audio-focus again.

I also posted on the rntp-discord, since I wasn't sure if it might be problem or just a clarification issue on my part.

@puckey
Copy link
Contributor Author

puckey commented Jul 5, 2023

Hmm I don't have any experience with creating temporary audio interruptions. Pull requests are welcome for something like this..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants