fix(audio): apply normalization immediately from YouTube response, reset stale cross-track gain#3905
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)
📝 WalkthroughWalkthroughAdds thread-safe normalization processor storage, volatile in-memory loudness/gain caches, a helper to compute/apply normalization from loudness inputs, setup/fetch path updates for missing loudness, and refactors VolumeNormalizationAudioProcessor to reuse an output buffer and simplify gain application. ChangesVolume normalization from fresh loudness data (MusicService)
Volume normalization processor refactor
Sequence Diagram(s)sequenceDiagram
participant StreamFetch
participant applyNormalizationFromLoudnessData
participant MusicService
participant VolumeNormalizationAudioProcessor
StreamFetch->>applyNormalizationFromLoudnessData: call(mediaId, loudnessDb, perceptualLoudnessDb)
applyNormalizationFromLoudnessData->>MusicService: cache measuredLufs and mediaId
applyNormalizationFromLoudnessData->>MusicService: check cachedNormalizationEnabled
alt normalization disabled
applyNormalizationFromLoudnessData->>VolumeNormalizationAudioProcessor: disable()
else normalization enabled & measuredLufs present
applyNormalizationFromLoudnessData->>applyNormalizationFromLoudnessData: compute & clamp targetGainMb
applyNormalizationFromLoudnessData->>VolumeNormalizationAudioProcessor: setTargetGain(targetGainMb), setEnabled(true)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
|
@yarikeua @Bec-de-Xorbin It seems to work, can you test and confirm that it works? |
|
@kairosci I tested your version and now everything works without delays. !Update |
Thanks for the quick reaction! Unfortunately no, it still takes ~3 seconds for normalization to kick in. |
Hmm. I'm using Metrolist PR 3905 and when this (https://music.youtube.com/watch?v=i97OkCXwotE) ends and this (https://music.youtube.com/watch?v=dKWfA9LCCOg) begins it takes ~3 seconds to lower volume. |
|
@kairosci There's a slight delay only when changing the normalization settings, specifically the loudness level. Is this how it should be? |
@Bec-de-Xorbin Yes, you are right, when I tested with these tracks, the problem persisted. |
kairosci
force-pushed
the
fix-audio-normalization-timing
branch
from
17c1b46 to
36016fe
Compare
|
@yarikeua @Bec-de-Xorbin could you take another look? Thanks |
|
Yes, this. Not working at all. |
|
@yarikeua @Bec-de-Xorbin could you take another look? Thanks |
Same. Not working at all. |
|
do you have log about this? |
|
|
thanks |
|
@kairosci It works now, but just as it did originally, with a delay of 3-5 seconds. |
|
@yarikeua do you have log also about this? |
|
|
@kairosci Nothing has changed, only the progress bar is now staying at 0:00 while the track is playing. |
kairosci
force-pushed
the
fix-audio-normalization-timing
branch
from
6495f2f to
0502262
Compare
|
@kairosci After a clean install, the progress bar started working again. The delay is still there. Record_2026-06-06-01-11-40_d800bdcb138acf830d1e1a76bd31f651.mp4 |
kairosci
force-pushed
the
fix-audio-normalization-timing
branch
from
0502262 to
29d0896
Compare
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/kotlin/com/metrolist/music/playback/audio/VolumeNormalizationAudioProcessor.kt (1)
185-191: 💤 Low valueRemove unused
read24Bitfunction.This function is now dead code—the 24-bit sample reading logic was inlined directly in the
ENCODING_PCM_24BITbranch at lines 104-107.🧹 Proposed removal
- private fun read24Bit(buffer: ByteBuffer): Int { - val b0 = buffer.get().toInt() and 0xFF - val b1 = buffer.get().toInt() and 0xFF - val b2 = buffer.get().toInt() - return (b2 shl 16) or (b1 shl 8) or b0 - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/kotlin/com/metrolist/music/playback/audio/VolumeNormalizationAudioProcessor.kt` around lines 185 - 191, Remove the now-unused helper function read24Bit from VolumeNormalizationAudioProcessor: locate the private fun read24Bit(buffer: ByteBuffer) and delete it, and ensure no other references to read24Bit remain (the 24-bit logic has been inlined in the ENCODING_PCM_24BIT branch), leaving the class without dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@app/src/main/kotlin/com/metrolist/music/playback/audio/VolumeNormalizationAudioProcessor.kt`:
- Around line 185-191: Remove the now-unused helper function read24Bit from
VolumeNormalizationAudioProcessor: locate the private fun read24Bit(buffer:
ByteBuffer) and delete it, and ensure no other references to read24Bit remain
(the 24-bit logic has been inlined in the ENCODING_PCM_24BIT branch), leaving
the class without dead code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be58b35b-af80-4a81-86f3-c7671c34b7e1
📒 Files selected for processing (2)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.ktapp/src/main/kotlin/com/metrolist/music/playback/audio/VolumeNormalizationAudioProcessor.kt
|
@kairosci same( Record_2026-06-06-01-39-08_d800bdcb138acf830d1e1a76bd31f651.mp4 |
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)
2226-2228:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset normalization when the
FormatEntityrow is missing.This branch still keeps the previous track's gain alive.
onMediaItemTransition()callssetupAudioNormalization()before the new row exists, so the next track can start with stale normalization and, if playback is served from the URL/cache path, keep it until some later async update happens.Suggested fix
isFormatNull -> { - Timber.tag(TAG).d("Loudness row not ready yet; keeping cached normalization state") + Timber.tag(TAG).d("Loudness row not ready yet; resetting normalization to neutral") + cachedNormalizationGainMb = null + cachedNormalizationEnabled = false + playerNormalizationProcessors.values.forEach { it.enabled = false } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt` around lines 2226 - 2228, The isFormatNull branch in MusicService.kt should clear/reset the per-track normalization state instead of keeping the previous track's gain; update the isFormatNull handling inside setupAudioNormalization() (used by onMediaItemTransition()) to call the normalization reset path (e.g., invoke whatever method or logic resets normalization state and related fields that current/previous track normalization uses) so a missing FormatEntity row does not leave stale gain applied; ensure you reference and reset the same variables/flags used by the normal setup path so subsequent async updates can populate fresh values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 2152-2156: The early return guarded by isCrossfading skips
applying the freshly computed track gain to the incoming player's normalization
processor; instead, always cache the new clampedGain and ensure it is applied to
the processor instance for the player being prepared (or to the secondary
ExoPlayer's processor) even during a crossfade: modify the block around
startCrossfade()/isCrossfading so you do not return before calling
playerNormalizationProcessors.values.forEach (or target the specific processor
for the incoming player) to call setTargetGain(clampedGain) and enable the
processor, or alternatively construct the secondary ExoPlayer with normalization
forced neutral until the track-specific loudness arrives.
- Around line 2143-2158: When measuredLufs is null (i.e. no loudness data),
immediately neutralize normalization: set cachedNormalizationGainMb to 0 and
cachedNormalizationEnabled to false, and if not isCrossfading update every
playerNormalizationProcessors entry by calling setTargetGain(0) and setting
enabled = false; if isCrossfading, still update the cached values but return
early as current code does. Locate this logic in MusicService.kt around the
block using targetLufs, measuredLufs, cachedNormalizationGainMb,
cachedNormalizationEnabled, isCrossfading and playerNormalizationProcessors, and
add an else branch handling the null measuredLufs case with the steps above (use
MIN_GAIN_MB/MAX_GAIN_MB only for the normal path where measuredLufs exists).
---
Outside diff comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Around line 2226-2228: The isFormatNull branch in MusicService.kt should
clear/reset the per-track normalization state instead of keeping the previous
track's gain; update the isFormatNull handling inside setupAudioNormalization()
(used by onMediaItemTransition()) to call the normalization reset path (e.g.,
invoke whatever method or logic resets normalization state and related fields
that current/previous track normalization uses) so a missing FormatEntity row
does not leave stale gain applied; ensure you reference and reset the same
variables/flags used by the normal setup path so subsequent async updates can
populate fresh values.
🪄 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: 637f4ca8-4337-4195-a206-ceeeca4b30cf
📒 Files selected for processing (1)
app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt
… during crossfade
|
@yarikeua Thanks for several tests, I changed my approach since the previous one wasn't working. Can you check if this one works? Thanks again! |
|
Damn, but isn't the problem with the song itself? Can you try another song? |
@kairosci Ok. No problem. Record_2026-06-06-15-06-24_d800bdcb138acf830d1e1a76bd31f651.mp4 |
…ng the current playback
|
@yarikeua could you take another look? Thanks again |
3 participants
Problem
After PR #3807, transitioning between tracks causes playback to start at full volume — normalization takes ~3 seconds to kick in, even with crossfade disabled.
Cause
Two root causes:
Stale cross-track state retained: In
setupAudioNormalization(), when the Room database has no loudness data yet for the new track (first playback), theformat == nullbranch did nothing — keeping the previous track's gain values on the processor. This caused wrong boost/cut to be applied to the new track.Slow async pipeline: Loudness data fetched from the YouTube response goes through Room→Flow→Combine→coroutine before being applied, adding latency even when the data is already available at stream fetch time.
Solution
Reset to neutral on missing data: The
format == nullbranch insetupAudioNormalization()now immediately resets the processor to neutral (enabled=false, gain=null) instead of keeping stale cross-track state. This prevents the previous track's gain from leaking into the new track.Bypass the Room pipeline: New
applyNormalizationFromLoudnessData()is called directly from the data source factory right after the YouTube response provides loudness data — applying gain to theVolumeNormalizationAudioProcessorsynchronously, before a single audio sample reaches the processor. The Room→Flow→Combine reactive path becomes a secondary convergence mechanism.Testing
assembleFossDebugbuilds successfullyRelated Issues
Summary by CodeRabbit
Bug Fixes
Improvements
Performance