feat: audio normalization loudness level selector#3358
Open
Jeff0945 wants to merge 8 commits intoMetrolistGroup:mainfrom
Open
feat: audio normalization loudness level selector#3358Jeff0945 wants to merge 8 commits intoMetrolistGroup:mainfrom
Jeff0945 wants to merge 8 commits intoMetrolistGroup:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a persisted Loudness Level preference and UI, surfaces the selected level in media info, and integrates cached loudness-normalization state into MusicService with a refactored loudness-enhancer lifecycle and session-handling flow. Changes
Sequence DiagramsequenceDiagram
autonumber
participant User as User/Settings
participant DataStore as DataStore
participant MusicService as MusicService
participant Enhancer as LoudnessEnhancer
participant Audio as Audio Output
User->>DataStore: set LoudnessLevel / AudioNormalization
DataStore-->>MusicService: emit preference change
MusicService->>MusicService: update cached (enabled, level)
MusicService->>MusicService: compute target LUFS & gain (async, generation-guarded)
MusicService->>Enhancer: create/apply gain or enable/disable
Enhancer->>Audio: normalize output
Audio-->>User: play normalized audio
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 378-383: The fields normalizationEnabledCached and
loudnessLevelCached are initialized to live defaults which can be applied before
prefs are loaded; update onCreate() to read the persisted preferences from your
DataStore and seed normalizationEnabledCached and loudnessLevelCached before any
playback callbacks can run (or alternatively default them to a disabled/quieter
state), so that setupLoudnessEnhancer() uses the persisted values rather than
the temporary true/AGGRESSIVE defaults; ensure you reference the DataStore
collector codepath used elsewhere to obtain the same LoudnessLevel enum and
boolean value when seeding.
- Around line 2015-2024: openAudioEffectSession currently gated only by
isAudioEffectSessionOpened so when performCrossfadeSwap() swaps players the new
player's audioSessionId isn't detected; modify the logic to track the
activeAudioSessionId (store the session id when opening) and in the entry path
that checks player.audioSessionId compare player.audioSessionId to
activeAudioSessionId: if different, close/cleanup the existing loudnessEnhancer
for activeAudioSessionId (ensure teardown closes that exact id), mark
isAudioEffectSessionOpened false, and then call
createLoudnessEnhancerForSessionId(audioSessionId) followed by
applyCachedLoudnessEnhancerNow() and setupLoudnessEnhancer() as needed; update
openAudioEffectSession()/teardown()/createLoudnessEnhancerForSessionId() to
read/set activeAudioSessionId and to only skip creation when the session id
matches the stored activeAudioSessionId.
In `@app/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt`:
- Line 147: The ShowMediaInfo display currently uses
currentFormat?.perceptualLoudnessDb directly, but playback
(MusicService.setupLoudnessEnhancer) falls back to loudnessDb when
perceptualLoudnessDb is absent; change the derivation to mirror playback by
computing a measuredLufs value first (e.g., measuredLufs =
currentFormat?.perceptualLoudnessDb ?: currentFormat?.loudnessDb) and then use
"${measuredLufs - targetLufs} dB" (or show "Unknown" if measuredLufs is null) so
the UI matches actual loudness normalization behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96512934-b9c7-47da-b921-306f9feee048
📒 Files selected for processing (5)
app/src/main/kotlin/com/metrolist/music/constants/PreferenceKeys.ktapp/src/main/kotlin/com/metrolist/music/playback/MusicService.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.ktapp/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.ktapp/src/main/res/values/metrolist_strings.xml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt (1)
1957-1959: Align fallback comment with actual loudness precedence.The comment says “use
loudnessDbfirst,” but the code prefersperceptualLoudnessDbfirst. Please update the comment (or logic) so future maintenance doesn’t misread this path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt` around lines 1957 - 1959, The comment describing loudness precedence is incorrect: the code currently prefers format?.perceptualLoudnessDb and only falls back to format?.loudnessDb adjusted by LoudnessLevel.AGGRESSIVE.targetLufs; update the inline comment near measuredLufs (and/or at the assignment of measuredLufs) to state that perceptualLoudnessDb is used first and loudnessDb (plus LoudnessLevel.AGGRESSIVE.targetLufs) is the fallback, referencing format.perceptualLoudnessDb, format.loudnessDb, measuredLufs, and LoudnessLevel.AGGRESSIVE.targetLufs so future readers see the true precedence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 2015-2017: releaseLoudnessEnhancer() currently resets
cachedNormalizationGainMb and cachedNormalizationEnabled causing the cache to be
lost during session swaps before applyCachedLoudnessEnhancerNow() runs; change
the behavior so releasing the loudness enhancer for a session swap does not
clear the cachedNormalizationGainMb/cachedNormalizationEnabled values.
Concretely, update releaseLoudnessEnhancer() to stop nulling
cachedNormalizationGainMb and toggling cachedNormalizationEnabled (or add a
parameter like releaseLoudnessEnhancer(preserveCache: Boolean) and only clear
the cache when preserveCache is false), and ensure any true teardown/shutdown
path still clears the cache as needed while the session-swap path calls release
without clearing so applyCachedLoudnessEnhancerNow() can reuse the cached
values.
---
Nitpick comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 1957-1959: The comment describing loudness precedence is
incorrect: the code currently prefers format?.perceptualLoudnessDb and only
falls back to format?.loudnessDb adjusted by
LoudnessLevel.AGGRESSIVE.targetLufs; update the inline comment near measuredLufs
(and/or at the assignment of measuredLufs) to state that perceptualLoudnessDb is
used first and loudnessDb (plus LoudnessLevel.AGGRESSIVE.targetLufs) is the
fallback, referencing format.perceptualLoudnessDb, format.loudnessDb,
measuredLufs, and LoudnessLevel.AGGRESSIVE.targetLufs so future readers see the
true precedence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97ef1d08-cb21-4b24-aca7-36b892a55eac
📒 Files selected for processing (2)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.ktapp/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt (1)
1958-1963: Consider simplifying the arithmetic expression.Inside the
if (measuredLufs != null)block,measuredLufsis smart-cast to non-null, making the.letwrapper unnecessary. Direct subtraction improves readability:♻️ Suggested simplification
if (measuredLufs != null) { - val loudnessDb = measuredLufs.let { it - targetLufs } + val gainDb = measuredLufs - targetLufs - val targetGain = (-loudnessDb * 100.0).toInt() + val targetGain = (-gainDb * 100.0).toInt() val clampedGain = targetGain.coerceIn(MIN_GAIN_MB, MAX_GAIN_MB)Also,
loudnessDbas a variable name is slightly misleading since it holds the difference between measured and target LUFS, not an absolute loudness value.gainDboradjustmentDbwould be clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt` around lines 1958 - 1963, In MusicService (inside the withContext(Dispatchers.Main) block) simplify the arithmetic by removing the unnecessary measuredLufs.let wrapper and perform direct subtraction using the smart-cast (e.g., val adjustmentDb = measuredLufs - targetLufs), rename loudnessDb to a clearer name like adjustmentDb or gainDb to reflect it is a difference, and then compute targetGain from that variable (e.g., val targetGain = (-adjustmentDb * 100.0).toInt()) so the intent and readability of the targetGain calculation are improved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 1958-1963: In MusicService (inside the
withContext(Dispatchers.Main) block) simplify the arithmetic by removing the
unnecessary measuredLufs.let wrapper and perform direct subtraction using the
smart-cast (e.g., val adjustmentDb = measuredLufs - targetLufs), rename
loudnessDb to a clearer name like adjustmentDb or gainDb to reflect it is a
difference, and then compute targetGain from that variable (e.g., val targetGain
= (-adjustmentDb * 100.0).toInt()) so the intent and readability of the
targetGain calculation are improved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f91b842e-b8b5-4f47-ad7e-267fc2af8b30
📒 Files selected for processing (1)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt
Jeff0945
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt (1)
1953-1958: Fix the misleading comment.The comment says "Use loudnessDb if available, otherwise fall back to perceptualLoudnessDb" but the code does the opposite—it prefers
perceptualLoudnessDband falls back toloudnessDb.📝 Suggested fix
- // Use loudnessDb if available, otherwise fall back to perceptualLoudnessDb + // Use perceptualLoudnessDb if available, otherwise fall back to loudnessDb val measuredLufs: Double? = format?.perceptualLoudnessDb ?: format?.loudnessDb?.let { it + LoudnessLevel.AGGRESSIVE.targetLufs }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt` around lines 1953 - 1958, The inline comment is incorrect: the code assigns measuredLufs by preferring format?.perceptualLoudnessDb and falling back to format?.loudnessDb adjusted by LoudnessLevel.AGGRESSIVE.targetLufs, so update the comment near the measuredLufs assignment (and references to format?.perceptualLoudnessDb and format?.loudnessDb) to accurately state "Use perceptualLoudnessDb if available, otherwise fall back to loudnessDb (adjusted by LoudnessLevel.AGGRESSIVE.targetLufs)". Ensure the explanatory text around the Timber debug logs (TAG) remains consistent with the corrected comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 1953-1958: The inline comment is incorrect: the code assigns
measuredLufs by preferring format?.perceptualLoudnessDb and falling back to
format?.loudnessDb adjusted by LoudnessLevel.AGGRESSIVE.targetLufs, so update
the comment near the measuredLufs assignment (and references to
format?.perceptualLoudnessDb and format?.loudnessDb) to accurately state "Use
perceptualLoudnessDb if available, otherwise fall back to loudnessDb (adjusted
by LoudnessLevel.AGGRESSIVE.targetLufs)". Ensure the explanatory text around the
Timber debug logs (TAG) remains consistent with the corrected comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78849f21-d56a-4760-a0ad-f953365f812a
📒 Files selected for processing (3)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.ktapp/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.kt
Jeff0945
force-pushed
the
feat/normalization-level-selector
branch
from
49c3d97 to
e6d0e3b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt (1)
1937-2004:⚠️ Potential issue | 🟠 MajorCancel stale loudness recomputations before they touch the enhancer.
setupLoudnessEnhancer()launches intoscope, so callers at Line 797 and Line 2163 can leave multiple jobs in flight. An older job can finish after a media/session change and overwritecachedNormalizationGainMbor re-enable normalization after it was turned off.💡 Suggested direction
Add a class-level
private var loudnessSetupJob: Job? = null, then gate the work so only the latest media/session can apply:- scope.launch { + loudnessSetupJob?.cancel() + loudnessSetupJob = scope.launch { + val requestedMediaId = withContext(Dispatchers.Main) { player.currentMediaItem?.mediaId } try { - val currentMediaId = withContext(Dispatchers.Main) { - player.currentMediaItem?.mediaId - } + val currentMediaId = requestedMediaId ... withContext(Dispatchers.Main) { + if (player.audioSessionId != audioSessionId || + player.currentMediaItem?.mediaId != requestedMediaId + ) return@withContext + if (measuredLufs != null) { ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt` around lines 1937 - 2004, setupLoudnessEnhancer launches into scope and can leave multiple jobs that race to apply/reset the loudness enhancer; add a class-level private var loudnessSetupJob: Job? = null, cancel previous loudnessSetupJob?.cancel() before starting a new job, assign loudnessSetupJob = scope.launch { ... } (or capture the Job returned by launch), and inside the launched coroutine check isActive (or handle CancellationException) before writing cachedNormalizationGainMb, cachedNormalizationEnabled, toggling loudnessEnhancer, or calling releaseLoudnessEnhancer so only the latest job can mutate state; ensure you still catch/report exceptions but let cancellation short-circuit the apply logic.
🧹 Nitpick comments (1)
app/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.kt (1)
232-250: Extract the loudness-level label mapping into one helper.This
when (loudnessLevel)block now exists twice here and again inapp/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt, so label changes or new enum entries have three update points.Also applies to: 459-470
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.kt` around lines 232 - 250, Extract the repeated loudness-label mapping into a single helper (e.g., an extension function on LoudnessLevel like labelRes() that returns the `@StringRes` id or a function getLoudnessLabel(context)), then replace the inline when(...) used in EnumDialog's valueText and the duplicate block in ShowMediaInfo.kt with calls to that helper (use stringResource(loudnessLevel.labelRes()) or the helper that returns a localized string) so all mapping logic (AGGRESSIVE/LOUD/BALANCED/QUIET) lives in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 2030-2057: The code marks isAudioEffectSessionOpened and sets
openedAudioEffectSessionId before confirming createLoudnessEnhancerForSessionId
produced an enhancer, which can leave the fast path stuck; move the state
updates so they happen only after
createLoudnessEnhancerForSessionId(audioSessionId) succeeds (i.e.,
loudnessEnhancer != null). Concretely: call createLoudnessEnhancerForSessionId,
check loudnessEnhancer, and only then set isAudioEffectSessionOpened = true and
openedAudioEffectSessionId = audioSessionId and run
applyCachedLoudnessEnhancerNow()/setupLoudnessEnhancer; if creation failed, do
not set the opened-session flags and instead call
releaseLoudnessEnhancer(clearNormalizationCache = false) or
closeAudioEffectSession as appropriate to avoid the stale-cache fast path.
In `@app/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt`:
- Line 149: The loudness label is built from a raw Float in ShowMediaInfo (the
mapping that uses stringResource(R.string.loudness) to measuredLufs?.let { ...
}), so change the mapping to format the numeric delta before appending the unit
and include a separator; compute the delta using measuredLufs and targetLufs and
format it (e.g., one decimal) into a string like "X.Y dB" using a locale-aware
formatter or String.format, then return that formatted string instead of
concatenating the raw Float with "dB".
---
Outside diff comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 1937-2004: setupLoudnessEnhancer launches into scope and can leave
multiple jobs that race to apply/reset the loudness enhancer; add a class-level
private var loudnessSetupJob: Job? = null, cancel previous
loudnessSetupJob?.cancel() before starting a new job, assign loudnessSetupJob =
scope.launch { ... } (or capture the Job returned by launch), and inside the
launched coroutine check isActive (or handle CancellationException) before
writing cachedNormalizationGainMb, cachedNormalizationEnabled, toggling
loudnessEnhancer, or calling releaseLoudnessEnhancer so only the latest job can
mutate state; ensure you still catch/report exceptions but let cancellation
short-circuit the apply logic.
---
Nitpick comments:
In
`@app/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.kt`:
- Around line 232-250: Extract the repeated loudness-label mapping into a single
helper (e.g., an extension function on LoudnessLevel like labelRes() that
returns the `@StringRes` id or a function getLoudnessLabel(context)), then replace
the inline when(...) used in EnumDialog's valueText and the duplicate block in
ShowMediaInfo.kt with calls to that helper (use
stringResource(loudnessLevel.labelRes()) or the helper that returns a localized
string) so all mapping logic (AGGRESSIVE/LOUD/BALANCED/QUIET) lives in one
place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46420a34-7337-4c7f-996c-2fe017db01d7
📒 Files selected for processing (3)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.ktapp/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt
Jeff0945
force-pushed
the
feat/normalization-level-selector
branch
from
f78d758 to
1fcf578
Compare
Collaborator
|
@coderabbitai Review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 1961-1964: The code currently treats format being null the same as
“no loudness data” and clears cachedNormalizationGainMb on track change, causing
a volume jump; change the logic so cachedNormalizationGainMb is only cleared
when the FormatEntity exists but contains no loudness info. Specifically, keep
measuredLufs calculation (measuredLufs: Double? = format?.perceptualLoudnessDb
?: format?.loudnessDb?.let { ... }) but only set cachedNormalizationGainMb =
null when format != null && measuredLufs == null (i.e., FormatEntity present but
loudness fields absent); apply the same conditional fix wherever
cachedNormalizationGainMb is cleared (including the block around measuredLufs
and the similar section at the 1992-1997 area).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49822a72-b39b-41a5-a6da-ac374997e1ab
📒 Files selected for processing (4)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.ktapp/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.ktapp/src/main/res/values/metrolist_strings.xml
✅ Files skipped from review due to trivial changes (2)
- app/src/main/res/values/metrolist_strings.xml
- app/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…display formatting
Jeff0945
force-pushed
the
feat/normalization-level-selector
branch
from
3511b95 to
fa2f1cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 1940-1942: The teardown path may early-return from
setupLoudnessEnhancer() when loudnessEnhancer is null and
createLoudnessEnhancerForSessionId(audioSessionId) fails, which skips releasing
a previously created standalone enhancer and leaves cached gain/stale state;
modify setupLoudnessEnhancer() (and symmetric idle/destroy code paths) to always
call the release routine (e.g., releaseLoudnessEnhancer() or explicit release of
loudnessEnhancer and clearing of cached gain) before returning when
createLoudnessEnhancerForSessionId(...) fails or when isAudioEffectSessionOpened
is not set, and ensure loudnessEnhancer and cached normalization state are
nulled/reset after release so subsequent sessions start clean.
- Around line 2060-2064: The current logic closes the global audio effect
session (closeAudioEffectSession/releaseLoudnessEnhancer) while fadingPlayer is
still audible, stripping per-song normalization during crossfade; modify the
implementation so each active player keeps its own LoudnessEnhancer/session
(e.g., track sessionId per player instance or map player->sessionId) or defer
closing the previous session until cleanupCrossfade() completes; update the
block using isAudioEffectSessionOpened/openedAudioEffectSessionId and the code
path that swaps players (the place that currently calls closeAudioEffectSession
at the player swap) to either (1) only close the session tied to the player that
is actually done, or (2) move the close/release call into cleanupCrossfade(),
ensuring fadingPlayer retains its enhancer until crossfade cleanup runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 568d8aac-5be7-4dac-8321-dc170c91786a
📒 Files selected for processing (5)
app/src/main/kotlin/com/metrolist/music/constants/PreferenceKeys.ktapp/src/main/kotlin/com/metrolist/music/playback/MusicService.ktapp/src/main/kotlin/com/metrolist/music/ui/screens/settings/PlayerSettings.ktapp/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.ktapp/src/main/res/values/metrolist_strings.xml
✅ Files skipped from review due to trivial changes (1)
- app/src/main/res/values/metrolist_strings.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/kotlin/com/metrolist/music/constants/PreferenceKeys.kt
- app/src/main/kotlin/com/metrolist/music/ui/utils/ShowMediaInfo.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Audio normalization feature only sets the normalization of audio tracks to the default target loudness of -7dB. Audios having a different value compared to the default target loudness are either boosted or reduced in volume to match it. Most noticeable issue Audio normalization has is the very audible aggressive compression it applies when the audio stream exceeds the dB limit due to the boost of volume.
Cause
Issues arise when the
loudnessvalue is negative. A negative value causes a boost, which in turn, may cause the system to compress the audio when it exceeds the dB limit. The purpose of theloudnessvalue is to provide the difference between the perceived loudness and the target loudness of the media. The values of which are given by the API response. This single reference of truth causes the audio issues (lower audio quality, aggressive compression) that we hear with Audio normalization enabled for the tracks with negative value forloudness.Solution
Note: Balanced is set as default the Loudness Level for consistency with major streaming services' default normalization level target.
Testing
Screenshots
Related Issues
Summary by CodeRabbit
New Features
Bug Fixes / Stability