Skip to content

Commit

Permalink
fade-out sound players even when idle or buffering
Browse files Browse the repository at this point in the history
this helps maintain behaviour parity among contiguous and
non-contiguous sounds, which in-turn helps maintaining the
correct preset state during STOPPING and PAUSING transitions.

resolves #1155
  • Loading branch information
ashutoshgngwr committed Dec 31, 2023
1 parent 14aa445 commit 46dd6c6
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.lastOrNull
import kotlinx.coroutines.launch
import java.util.*
import java.util.Random
import kotlin.math.pow
import kotlin.time.Duration
import kotlin.time.Duration.Companion.seconds
Expand Down Expand Up @@ -49,24 +49,24 @@ class LocalSoundPlayer @VisibleForTesting constructor(

override fun onMediaPlayerStateChanged(state: MediaPlayer.State) {
Log.d(LOG_TAG, "onMediaPlayerStateChanged: state=${state}")
val isPausingOrStopping = this.state == State.PAUSING || this.state == State.STOPPING
if (shouldFadeIn && state == MediaPlayer.State.PLAYING) {
mediaPlayer.fadeTo(getScaledVolume(), fadeInDuration)
shouldFadeIn = false
if (!isPausingOrStopping) {
mediaPlayer.fadeTo(getScaledVolume(), fadeInDuration)

Check warning on line 56 in app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt#L56

Added line #L56 was not covered by tests
}
}

when (state) {
MediaPlayer.State.BUFFERING -> this.state = State.BUFFERING
MediaPlayer.State.PAUSED -> this.state = State.PAUSED
MediaPlayer.State.STOPPED -> {
when {

Check warning on line 60 in app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt#L60

Added line #L60 was not covered by tests
state == MediaPlayer.State.PAUSED -> this.state = State.PAUSED
state == MediaPlayer.State.STOPPED -> {
mediaPlayer.setListener(null)
this.state = State.STOPPED
}
else -> {
// do not overwrite pausing and stopping state.
if (this.state != State.PAUSING && this.state != State.STOPPING) {
this.state = State.PLAYING
}
}

isPausingOrStopping -> Unit // do not overwrite pausing and stopping state.
state == MediaPlayer.State.BUFFERING -> this.state = State.BUFFERING
else -> this.state = State.PLAYING

Check warning on line 69 in app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt#L69

Added line #L69 was not covered by tests
}
}

Expand Down Expand Up @@ -160,14 +160,9 @@ class LocalSoundPlayer @VisibleForTesting constructor(
}

override fun pause(immediate: Boolean) {
if (shouldPlayOnLoadingMetadata) {
shouldPlayOnLoadingMetadata = false
state = State.PAUSED
return
}

shouldPlayOnLoadingMetadata = false

Check warning on line 163 in app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt#L163

Added line #L163 was not covered by tests
queueNextSegmentJob?.cancel()
if (immediate || mediaPlayer.state != MediaPlayer.State.PLAYING) {
if (immediate) {
mediaPlayer.pause()
return
}
Expand All @@ -177,9 +172,10 @@ class LocalSoundPlayer @VisibleForTesting constructor(
}

override fun stop(immediate: Boolean) {
shouldPlayOnLoadingMetadata = false

Check warning on line 175 in app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/java/com/github/ashutoshgngwr/noice/engine/LocalSoundPlayer.kt#L175

Added line #L175 was not covered by tests
loadSoundMetadataJob.cancel()
queueNextSegmentJob?.cancel()
if (immediate || mediaPlayer.state != MediaPlayer.State.PLAYING) {
if (immediate) {
mediaPlayer.stop()
return
}
Expand Down Expand Up @@ -222,10 +218,12 @@ class LocalSoundPlayer @VisibleForTesting constructor(
currentSegment?.isBridgeSegment == true -> {
validSegments.find { it.name == currentSegment?.to }
}

maxSilenceSeconds == 0 && currentSegment != null -> {
val from = requireNotNull(currentSegment)
validSegments.filter { it.isBridgeSegment && it.from == from.name }.random(RANDOM)
}

else -> validSegments.random(RANDOM)
} ?: throw IllegalStateException("couldn't find a segment to queue next")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import androidx.media3.exoplayer.source.ProgressiveMediaSource
import androidx.media3.extractor.ExtractorsFactory
import androidx.media3.extractor.mp3.Mp3Extractor
import kotlin.math.abs
import kotlin.math.max
import kotlin.math.min
import kotlin.time.Duration
import kotlin.time.Duration.Companion.seconds

Expand Down Expand Up @@ -137,7 +139,7 @@ class DefaultMediaPlayer @VisibleForTesting constructor(
require(toVolume in 0F..1F) { "toVolume must be in range [0, 1]" }
handler.removeCallbacksAndMessages(FADE_CALLBACK_TOKEN)

if (duration == Duration.ZERO || !exoPlayer.isPlaying) {
if (duration == Duration.ZERO) {
setVolume(toVolume)
callback.invoke()
return
Expand All @@ -153,11 +155,10 @@ class DefaultMediaPlayer @VisibleForTesting constructor(
override fun run() {
val progress = (System.currentTimeMillis() - startMillis).toFloat() / durationMillis
val newVolume = fromVolume + (deltaVolume * progress * sign)
if ((sign < 0 && newVolume <= toVolume) || (sign > 0 && newVolume >= toVolume)) {
exoPlayer.volume = toVolume
exoPlayer.volume = if (sign < 0) max(toVolume, newVolume) else min(toVolume, newVolume)
if (progress >= 1) {
callback.invoke()
} else {
exoPlayer.volume = newVolume
HandlerCompat.postDelayed(handler, this, FADE_CALLBACK_TOKEN, 50)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ class LocalSoundPlayerTest {
TestCase(
mediaPlayerState = MediaPlayer.State.BUFFERING,
pauseImmediate = false,
isExpectingFadeTransition = false,
isExpectingFadeTransition = true,
),
TestCase(
mediaPlayerState = MediaPlayer.State.BUFFERING,
Expand Down Expand Up @@ -319,7 +319,6 @@ class LocalSoundPlayerTest {
soundPlayer.pause(testCase.pauseImmediate)
if (testCase.isExpectingFadeTransition) {
assertEquals(SoundPlayer.State.PAUSING, soundPlayer.state)
assertEquals(1F, fakeMediaPlayer.pendingFadeTransition?.fromVolume)
assertEquals(0F, fakeMediaPlayer.pendingFadeTransition?.toVolume)
fakeMediaPlayer.consumePendingFadeTransition()
}
Expand All @@ -340,7 +339,7 @@ class LocalSoundPlayerTest {
TestCase(
mediaPlayerState = MediaPlayer.State.BUFFERING,
stopImmediate = false,
isExpectingFadeTransition = false,
isExpectingFadeTransition = true,
),
TestCase(
mediaPlayerState = MediaPlayer.State.BUFFERING,
Expand Down Expand Up @@ -369,7 +368,6 @@ class LocalSoundPlayerTest {
soundPlayer.stop(testCase.stopImmediate)
if (testCase.isExpectingFadeTransition) {
assertEquals(SoundPlayer.State.STOPPING, soundPlayer.state)
assertEquals(1F, fakeMediaPlayer.pendingFadeTransition?.fromVolume)
assertEquals(0F, fakeMediaPlayer.pendingFadeTransition?.toVolume)
fakeMediaPlayer.consumePendingFadeTransition()
}
Expand Down

0 comments on commit 46dd6c6

Please sign in to comment.