Skip to content

fix: Incorrect stop reason#20

Merged
iamszabo merged 2 commits intomasterfrom
incorrect_stop_reason_when_seeking
Feb 22, 2022
Merged

fix: Incorrect stop reason#20
iamszabo merged 2 commits intomasterfrom
incorrect_stop_reason_when_seeking

Conversation

@iamszabo
Copy link
Copy Markdown

At some cases an STKAudioPlayerStopReasonNone declared although the file reached the end. The issue can be reproduced from time to time when seeked to the end of the file.

It happens as one thread declares that the file is ended on OutputRenderCallback and another thread declares the ending reason on the processRunloop. If the file ended thread happens first the stop reason won't be set. My fix makes sure when the lastFrame is played the end reason will be set as STKAudioPlayerStopReasonEof

…the file reached the end. The issue can be reproduced from time to time when seeked to the end of the file. It happens as one thread declares that the file is ended and another thread declares the ending reason. If the file ended thread happens first the stop reason won't be set. My fix makes sure when the lastFrame is played the end reason will be set as STKAudioPlayerStopReasonEof
@trafico-bot trafico-bot Bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Feb 21, 2022
Copy link
Copy Markdown

@ruduss ruduss left a comment

Choose a reason for hiding this comment

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

Would it be best for us to do the additional check of STKAudioPlayerStopReasonNone in our code rather than make the change in STKAudioPlayer?
eg in FileAudioService the change could be:

@objc func audioPlayer(_ audioPlayer: STKAudioPlayer!, didFinishPlayingQueueItemId queueItemId: NSObject!,
                          with stopReason: STKAudioPlayerStopReason, andProgress progress: Double, andDuration duration: Double) {
       if stopReason == STKAudioPlayerStopReasonEof || stopReason == STKAudioPlayerStopReasonNone  {
           self.audioCompletedSubject.onNext(())
       }
   }

@iamszabo
Copy link
Copy Markdown
Author

@ruduss I considered this option, the issue with STKAudioPlayerStopReasonNone is that, we don't know what is the reason why the audio has stopped. That is the default value when no other reason has specified. For example, it will return STKAudioPlayerStopReasonNone when you change the audio source by playing something else.

STKAudioPlayerStopReasonEof is retuned when we reached the end of the file itself, and it's a signal for us to play the next episode on the list.

@trafico-bot trafico-bot Bot added ✅ Approved Pull Request has been approved and can be merged and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Feb 21, 2022
@iamszabo iamszabo merged commit 90b6e4a into master Feb 22, 2022
@trafico-bot trafico-bot Bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved Pull Request has been approved and can be merged labels Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ Merged Pull Request has been merged successfully size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants