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(android): remove pausePlayback when audio focus loss event #3496

Conversation

YangJonghun
Copy link
Collaborator

@YangJonghun YangJonghun commented Jan 23, 2024

Update the documentation

fix(android): remove pausePlayback when audio focus loss event

Update the changelog

fix(android): remove pausePlayback when audio focus loss event

Describe the changes

pausePlayback is added within PR #1897 (commit 1ab82f7)
but, it makes unintended pauses can be happen with below flow
source ready(auto play) -> pause -> play

Copy link
Collaborator

@freeboub freeboub left a comment

Choose a reason for hiding this comment

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

I agree with that, this behavior is an application choice !

@KrzysztofMoch KrzysztofMoch self-requested a review January 23, 2024 21:20
@KrzysztofMoch KrzysztofMoch merged commit b1ab0f2 into TheWidlarzGroup:master Jan 23, 2024
3 checks passed
@YangJonghun YangJonghun deleted the fix/play-when-loss-audio-focus branch January 24, 2024 11:12
@freeboub
Copy link
Collaborator

@KrzysztofMoch @YangJonghun I think again of this patch, I think it will cause some regressions ...
What would be the behaviour when user receive an incoming call ?
Users will have to patch their application to restaure this default behavior...
Do you agree to revert it ?
Ideally we should make this behavior configurable by application with a new prop !

@YangJonghun
Copy link
Collaborator Author

@freeboub
Your suggestion is very good.
but usually, we have to pause with AUDIOFOCUS_LOSS event. the problem is developer doesn't have any chance to resume their playback.
I think simple revert would continue the problem, so I was wondering about adding the below code to the AUDIOFOCUS_GAIN event handler.

            switch (focusChange) {
                case AudioManager.AUDIOFOCUS_LOSS:
                    view.hasAudioFocus = false;
                    view.eventEmitter.audioFocusChanged(false);
                    view.pausePlayback();
                    view.audioManager.abandonAudioFocus(this);
                    break;
                case AudioManager.AUDIOFOCUS_LOSS_TRANSIENT:
                    view.eventEmitter.audioFocusChanged(false);
                    break;
                case AudioManager.AUDIOFOCUS_GAIN:
                    view.hasAudioFocus = true;
                    view.eventEmitter.audioFocusChanged(true);
                    if (!view.isPaused) {
                        view.startPlayback();
                    }
                    break;
                default:
                    break;
            }

@freeboub
Copy link
Collaborator

@YangJonghun Ok, I understand correctly the request know!
You are right, in case the playback has been paused due to audioFocusLoss, we should resume it (but only in that precise case!)
I will revert the change for now.
Ideally, this behavior shall be configurable by application...

freeboub pushed a commit that referenced this pull request Jan 28, 2024
freeboub added a commit that referenced this pull request Jan 29, 2024
#3504)

This reverts commit b1ab0f2.

Co-authored-by: olivier <olivier.bouillet@ifeelsmart.com>
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