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): promises leak caused by player not in the HashMap #136

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 28 additions & 30 deletions android/src/main/java/com/audiowaveform/AudioWaveformModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,10 @@ class AudioWaveformModule(context: ReactApplicationContext): ReactContextBaseJav
@ReactMethod
fun startPlayer(obj: ReadableMap, promise: Promise) {
val finishMode = obj.getInt(Constants.finishMode)
val key = obj.getString(Constants.playerKey)
val speed = obj.getDouble(Constants.speed)
if (key != null) {
audioPlayers[key]?.start(finishMode ?: 2, speed.toFloat(),promise)
} else {
promise.reject("startPlayer Error", "Player key can't be null")
}

val player = getPlayerOrReject(obj, promise, "startPlayer Error");
player?.start(finishMode ?: 2, speed.toFloat(),promise)
}

@ReactMethod
Expand All @@ -182,25 +179,18 @@ class AudioWaveformModule(context: ReactApplicationContext): ReactContextBaseJav

@ReactMethod
fun pausePlayer(obj: ReadableMap, promise: Promise) {
val key = obj.getString(Constants.playerKey)
if (key != null) {
audioPlayers[key]?.pause(promise)
} else {
promise.reject("pausePlayer Error", "Player key can't be null")
}
val player = getPlayerOrReject(obj, promise, "pausePlayer Error");
player?.pause(promise);
}

@ReactMethod
fun seekToPlayer(obj: ReadableMap, promise: Promise) {
try {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
val progress = obj.getInt(Constants.progress)
val key = obj.getString(Constants.playerKey)
if (key != null) {
audioPlayers[key]?.seekToPosition(progress.toLong(), promise)
} else {
promise.reject("seekTo Error", "Player key can't be null")
}

val player = getPlayerOrReject(obj, promise, "seekTo Error");
player?.seekToPosition(progress.toLong(), promise)
} else {
Log.e(
Constants.LOG_TAG,
Expand All @@ -216,24 +206,18 @@ class AudioWaveformModule(context: ReactApplicationContext): ReactContextBaseJav
@ReactMethod
fun setVolume(obj: ReadableMap, promise: Promise) {
val volume = obj.getInt(Constants.volume)
val key = obj.getString(Constants.playerKey)
if (key != null) {
audioPlayers[key]?.setVolume(volume.toFloat(), promise)
} else {
promise.reject("setVolume error", "Player key can't be null")
}

val player = getPlayerOrReject(obj, promise, "setVolume Error");
player?.setVolume(volume.toFloat(), promise)
}

@ReactMethod
fun getDuration(obj: ReadableMap, promise: Promise) {
val key = obj.getString(Constants.playerKey)
val duration = obj.getInt(Constants.durationType)
val type = if (duration == 0) DurationType.Current else DurationType.Max
if (key != null) {
audioPlayers[key]?.getDuration(type, promise)
} else {
promise.reject("getDuration Error", "Player key can't be null")
}

val player = getPlayerOrReject(obj, promise, "getDuration Error");
player?.getDuration(type, promise)
}

@ReactMethod
Expand Down Expand Up @@ -429,4 +413,18 @@ class AudioWaveformModule(context: ReactApplicationContext): ReactContextBaseJav
handler.removeCallbacks(emitLiveRecordValue)
}

private fun getPlayerOrReject(arguments: ReadableMap, promise: Promise, errorCode: String): AudioPlayer? {
val key = getPlayerKeyOrReject(arguments, promise, errorCode)
return audioPlayers[key] ?: run {
promise.reject(errorCode, "$errorCode: Player not in the list")
null
}
}

private fun getPlayerKeyOrReject(arguments: ReadableMap, promise: Promise, errorCode: String): String? {
return arguments.getString(Constants.playerKey) ?: run {
promise.reject(errorCode, "$errorCode: Player key can't be null")
null
}
}
}
27 changes: 22 additions & 5 deletions src/components/Waveform/Waveform.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,11 @@ export const Waveform = forwardRef<IWaveformRef, IWaveform>((props, ref) => {
);
}
} catch (error) {
if (playerState === PlayerState.paused) {
// If the player is not prepared, triggering the stop will reset the player for next click. Fix blocked paused player after a call to `stopAllPlayers`
await stopPlayerAction();
}

return Promise.reject(error);
}
} else {
Expand Down Expand Up @@ -423,7 +428,7 @@ export const Waveform = forwardRef<IWaveformRef, IWaveform>((props, ref) => {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [viewLayout?.width, mode, candleWidth, candleSpace]);

useEffect(() => {
const seekToPlayerAction = async () => {
if (!isNil(seekPosition)) {
if (mode === 'static') {
const seekAmount =
Expand All @@ -432,10 +437,18 @@ export const Waveform = forwardRef<IWaveformRef, IWaveform>((props, ref) => {
const clampedSeekAmount = clamp(seekAmount, 0, 1);

if (!panMoving) {
seekToPlayer({
playerKey: `PlayerFor${path}`,
progress: clampedSeekAmount * songDuration,
});
try {
await seekToPlayer({
playerKey: `PlayerFor${path}`,
progress: clampedSeekAmount * songDuration,
});
} catch (e) {
if (playerState === PlayerState.paused) {
// If the player is not prepared, triggering the stop will reset the player for next click. Fix blocked paused player after a call to `stopAllPlayers`
await stopPlayerAction();
}
}

if (playerState === PlayerState.playing) {
startPlayerAction();
}
Expand All @@ -444,6 +457,10 @@ export const Waveform = forwardRef<IWaveformRef, IWaveform>((props, ref) => {
setCurrentProgress(clampedSeekAmount * songDuration);
}
}
};

useEffect(() => {
seekToPlayerAction();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [seekPosition, panMoving, mode, songDuration]);

Expand Down