hyochan / react-native-audio-recorder-player

react-native native module for audio recorder and player.
MIT License
730 stars 216 forks source link

Implemented a fix for addPeriodicTimeObserver() nil issue in RNAudioRecorderPlayer.swift + question about Android seekToPlayer #335

Open HarryLinCS opened 3 years ago

HarryLinCS commented 3 years ago

IOS issue + fix

Hi guys, great work with this audio player!

I found an error with the addPeriodicTimeObserver() function in RNAudioRecorderPlayer.swift. This happens after creating a player and setting a relatively fast subscription duration (around 0.05s or 0.1s), On older/slower devices (I tested with a physical iphone 6s and a simulated iphone 8), sometimes pausing or closing the player right after playback would cause the following crash:

2021-06-23 09:40:44.362062-0400 Porj[71255:2112236] RNAudioRecorderPlayer/RNAudioRecorderPlayer.swift:287: Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value

It seems like in addPeriodicTimeObserver, sometimes audioPlayer can be nil, meaning the app crashes when trying to send back information. I found the crash happens more often when the subscription duration of the player is set to faster intervals (e.g, happens more often on 0.05s refresh than 0.1s) Here's the original code:

func addPeriodicTimeObserver() {
    let timeScale = CMTimeScale(NSEC_PER_SEC)
    let time = CMTime(seconds: subscriptionDuration, preferredTimescale: timeScale)

    timeObserverToken = audioPlayer.addPeriodicTimeObserver(forInterval: time,
                                                            queue: .main) {_ in
        self.sendEvent(withName: "rn-playback", body: [
            "isMuted": self.audioPlayer.isMuted,
            "currentPosition": self.audioPlayerItem.currentTime().seconds * 1000,
            "duration": self.audioPlayerItem.duration.seconds * 1000,
        ])
    }
}

I made the following change to the function which seemed to fix it:

func addPeriodicTimeObserver() {
    let timeScale = CMTimeScale(NSEC_PER_SEC)
    let time = CMTime(seconds: subscriptionDuration, preferredTimescale: timeScale)

    timeObserverToken = audioPlayer.addPeriodicTimeObserver(forInterval: time,
                                                            queue: .main) {_ in
        if (self.audioPlayer != nil){             <--- added a check to see if audioPlayer is not nil before sending information back
            self.sendEvent(withName: "rn-playback", body: [
                "isMuted": self.audioPlayer.isMuted,
                "currentPosition": self.audioPlayerItem.currentTime().seconds * 1000,
                "duration": self.audioPlayerItem.duration.seconds * 1000,
            ])
        }
    }
}

Of course, this means that sometimes event info is not sent back. This didn't seem to cause a problem on my end however, since the subscription rate is fast enough that subsequent updates would return the correct information in time.

Android Question

Also, as a side note, I'm currently trying to develop for Android as well. Our app plays small sound samples of 5-20 seconds in length. We'd like to be able to scrub through the files and seek to certain parts. However, the seekToPlayer function only works with seconds on the android side (it works with milliseconds on the ios side). This makes scrolling and scrubbing through the player to seek to certain segments very inaccurate on smaller audio files. Can we expect millisecond support for android side sometime soon?

Version of react-native-audio-recorder-player

3.0.8

Version of React Native

react-native-cli: 2.0.1 react-native: 0.64.2

Platforms you faced the error (IOS or Android or both?)

IOS

Expected behavior

Pausing or closing the player right after playback works without issue

Actual behavior

Pause or closing the player right after playback crashes the app with nil error

Steps to reproduce the behavior

Create an audioplayer, load a file, set subscription duration to something high (e.g. 0.05s). Then play and immediately pause. Rapidly clicking a pause/play button will usually achieve this crash in 3-10 clicks for older devices. On iphone 6s, this crash happened even with up to 1 second of waiting between playing and pausing.

hyochan commented 3 years ago

Looks like this is very helpful! @HarryLinCS Could you kindly open a PR for this?