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] #371 전화 수신, 발신 시 소리 재생 오류 해결 #374

Merged
merged 7 commits into from
Jan 29, 2025

Conversation

l1004ga
Copy link
Collaborator

@l1004ga l1004ga commented Jan 29, 2025

개요

전화 수신, 발신 시 소리 재생 오류 해결

작업 내용

1. 전화 수신/발신 시 메트로놈 재생 중단

  • NotificationCenter를 통해서 AudioSession 인터럽트 관측하도록 구현
  • 오디오 인터럽트(알람, 전화) 발생 시 메트로놈 재생 중단(화면, 소리)
  • MetronomeOnOffUseCase에서 화면과 소리 모두 play, stop 처리하고 있으나 인터럽트 확인은 AVFoundation을 통해서 진행하는 것으로 SoundManager에서 인터럽트 여부를 Combine으로 전달하여 중단 여부를 확인하도록 구현함

2. 전화 종료 후 재실행 시 오디오 재실행

  • 오디오 인터럽트 종료 시 Engine 재실행 -> 시스템적으로 오락가락함
  • MetronomeOnOffUseCase에서 시작 버튼을 누를 때 AudioEngine isRunning 여부 체크 후 재시작하도록 구현함

비고

  • Beep() 실행 시 playNode 부착 과정에서 체크되는 사항임을 확인하여 내부적으로 AudioEngine.isRunning 확인하여 최초 실행 시 AudioEngine.start() 처리하고자

작업 스크린샷

리뷰어에게 남길 말

인터럽트 종료시 실행되는 .ended 부분에서 Engine.Start()가 정상 처리(isRunning) 되는 것으로 찍히는데, 실제 Beep() 실행 시점에 isRunning하지 않는 것으로 나오는지 모르겠어요.
위 사항을 해결하면 코드가 조금 더 많이 깔끔해질 것 같은데,,,

CheckList <!-- 리뷰어가 체크할 항목입니다. >

  • PR의 Target이 올바르게 설정되어 있나요?
  • Assignee는 올바르게 할당되어있나요?
  • Label이 적절하게 설정되어 있나요?
  • 변경사항에 의문이 드는 곳은 없나요?

@l1004ga l1004ga added bug Something isn't working enhancement New feature or request labels Jan 29, 2025
@l1004ga l1004ga self-assigned this Jan 29, 2025
@l1004ga l1004ga requested a review from YunKi-H as a code owner January 29, 2025 12:24
self.setupNotifications()
}

@objc func handleInterruption(notification: Notification) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 메서드는 private으로 설정하지 않으신 이유가 궁금합니다.
하단에서 selector로 등록할때 문제가 생길까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아뇽,, 마지막에 private 체크했는데 objc가 앞에 있어서 당연히 뒤에 달려있다고 생각했나봅니다~ 수정해서 올리겠습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

0723f0b 해당 커밋을 통해서 수정해두었습니다.


self.soundManager.callInterruptPublisher.sink {[weak self] callInterrupt in
guard let self else { return }
callInterrupt == true ? self.stop() : nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bool값을 == true 인지 검증하는게 조금 어색해 보입니다
isCalling 같이 자체적으로 Bool값을 암시하는 변수명으로 대체하는게 어떨가요

Copy link
Collaborator

Choose a reason for hiding this comment

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

받는 값이 false일때 (전화가 끊겼을때) 아무 일도 하지 않는다고 하면
Bool값 대신 Void 타입으로 이벤트 발생만 전달하는 방안도 있을것 같습니다.
전화가 올때 재생이 멈추고 유저가 전화를 끊고 다시 재생을 눌러야 하는 방식으로 구현될것 같아요

Copy link
Collaborator Author

@l1004ga l1004ga Jan 29, 2025

Choose a reason for hiding this comment

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

임시변수를 만들어도 해당 코드 내부에서 사용처가 없기 때문에 안만드는 것이 더 적합하다고 생각합니다. 방식은 말씀하신 것처럼 현재 전화 오면 재생이 멈추고 전화를 끊고 유저가 다시 재생을 누르는 식으로 구현되어 있습니다.

처리 방식을 변수로 처리하기보다 callInterrupt ? self.stop : nil 로 수정하는게 더 나을 것 같고,
이벤트 발생 여부만 전달하는 것이 가장 좋을 것 같습니다. 이벤트 여부 전달 방식으로 바꿔도 괜찮을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

MetronomeOnOffUseCase의 tickPublisher가 Void 타입 Publisher로 구현되어 있습니다
참고가 되면 좋겠네여

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@YunKi-H 수정했습니다~ 확인 부탁드리며, 추가적으로 피드백 주실 부분이 있을지 확인 후 머지하겠습니다!

@YunKi-H YunKi-H added this to the 2.4.0 milestone Jan 29, 2025
Copy link
Collaborator

@YunKi-H YunKi-H left a comment

Choose a reason for hiding this comment

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

좋아요!


case .ended:
self.audioEngineStart()
self.publisher.send()
Copy link
Collaborator

Choose a reason for hiding this comment

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

전화 받는 이벤트만 처리한다면 .ended 케이스를 처리할 필요가 없을거같습니다
혹시 모르니 남겨두셨나욤?

Copy link
Collaborator Author

@l1004ga l1004ga Jan 29, 2025

Choose a reason for hiding this comment

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

publisher 처리는 삭제하겠습니다.
다만 self.audioEngineStart()의 경우에는 물론 항상 작동하지는 않지만,,, 시스템 상황적으로 끝나고 정상적으로 Engine이 start 되는게 가장 안정적이라고 생각합니다. 어떻게 생각하시나욥...?

@l1004ga l1004ga merged commit 3a36c69 into dev Jan 29, 2025
3 checks passed
@YunKi-H YunKi-H deleted the fix/#371-SoundInterruptError branch January 30, 2025 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fix] 소리 재생 중 전화모드 이동 시 소리 출력이 안되는 문제
2 participants