-
Notifications
You must be signed in to change notification settings - Fork 22
Fix ReplayGain Issue #194
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
Open
timusus
wants to merge
6
commits into
main
Choose a base branch
from
claude/investigate-issue-121-011CV3oHGwBzxJAiKiURXxEX
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Fix ReplayGain Issue #194
timusus
wants to merge
6
commits into
main
from
claude/investigate-issue-121-011CV3oHGwBzxJAiKiURXxEX
Conversation
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
ReplayGain volume adjustments were being applied after playback began, causing a noticeable volume shift approximately half a second into each track. This occurred because setReplayGain() was called in the success callback after player.prepare() had already been executed. The fix moves setReplayGain() calls to execute before load() in all playback functions, ensuring the ReplayGainAudioProcessor has the correct gain values before the player begins preparing and processing audio samples. Changes: - attemptLoad(): Moved setReplayGain() before load() call - skipToNext(): Added setReplayGain() before load() call - skipToPrev(): Added setReplayGain() before load() call - skipTo(): Added setReplayGain() before load() call This eliminates the race condition and ensures volume is correct from the first audio sample of each track.
During automatic track transitions (when ExoPlayer moves to the next track), there was still a potential race condition where audio processing could start before ReplayGain values were updated. This adds an explicit setReplayGain() call immediately after skipToNext() in the onTrackEnded() handler when trackWentToNext is true. This ensures ReplayGain values are set synchronously as soon as the queue position changes, minimizing any delay. This complements the existing onQueuePositionChanged callback and provides additional protection against timing issues during automatic transitions.
This commit addresses the fundamental architectural issue with ReplayGain timing. The previous approach set ReplayGain values reactively after track changes, which created race conditions during both manual and automatic transitions. The new architecture embeds ReplayGain values directly into each MediaItem: 1. MediaItem Creation: ReplayGain values are attached as a tag when creating MediaItems in both load() and loadNext(). This ensures each queued track carries its own gain information. 2. Manual Track Changes: In ExoPlayerPlayback.load(), ReplayGain is set immediately after setMediaItem() and before prepare(), ensuring correct gain from the first sample. 3. Automatic Transitions: In onMediaItemTransition(), the ReplayGain values are read from the transitioning MediaItem's tag and applied immediately. This eliminates the need to look up gain values from the queue manager. 4. Centralized Logic: All ReplayGain management is now in ExoPlayerPlayback, removing scattered setReplayGain() calls from PlaybackManager. This approach ensures ReplayGain values are associated with specific audio tracks throughout ExoPlayer's pipeline, preventing any samples from being processed with incorrect gain values. Fixes #121
This addresses the gapless playback timing issue: when ExoPlayer buffers the next track during gapless playback, audio samples can flow through the AudioProcessor before onMediaItemTransition callbacks fire. The solution makes ReplayGainAudioProcessor directly aware of which MediaItem is currently playing: 1. The processor now holds a reference to the Player instance 2. On every audio buffer, it queries player.currentMediaItem 3. It reads ReplayGain values from the MediaItem's tag in real-time 4. Falls back to manually-set values if tag is unavailable This ensures the correct gain is applied based on what's actually being rendered, not based on callback timing. Even if Track B's samples are buffered while Track A plays, they'll use Track A's gain until the player actually transitions to Track B. Benefits: - Eliminates race conditions during gapless transitions - Handles buffering edge cases automatically - Gain switches at the exact moment playback transitions - No dependency on callback ordering or timing This is the proper architectural fix for #121
This commit removes the Player reference from ReplayGainAudioProcessor, addressing critical architectural and thread-safety issues: ## Issues Fixed 1. **Layering Violation**: AudioProcessor (low-level DSP) was holding a reference to Player (high-level controller), creating a circular dependency and violating separation of concerns. 2. **Thread Safety Violation**: player.currentMediaItem was being accessed from the audio rendering thread, but Player API is explicitly designed for main-thread-only access. This violated ExoPlayer's threading model. 3. **Performance**: Querying player.currentMediaItem on every audio buffer (thousands of times per second) added unnecessary overhead in the hot path. 4. **Redundancy**: Two update mechanisms existed (proactive updates in onMediaItemTransition + reactive queries), adding complexity. ## Solution The processor now uses a clean proactive update model: - Manual transitions: ReplayGain set in load() BEFORE prepare() - Automatic transitions: ReplayGain set in onMediaItemTransition Thread safety ensured via: - @volatile for memory visibility across threads - @synchronized for atomic read/write operations ## Timing Analysis The original issue (#121) reported ~500ms delay, caused by setting ReplayGain AFTER prepare() completed. The new approach sets it BEFORE prepare() (manual) and in onMediaItemTransition (automatic). During gapless transitions, there may be a brief window (~10-50ms, a few audio buffers) where samples are processed with the previous track's gain. This is a fundamental limitation of async Player.Listener callbacks vs continuous audio rendering, but is imperceptible compared to the original 500ms issue. MediaItem tagging remains in place as the proper way to associate ReplayGain metadata with tracks through ExoPlayer's pipeline.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description:
Summary
Fixes #121 - ReplayGain volume adjustments are now applied before playback starts instead of after, eliminating the audible volume shift that occurred ~500ms into each track.
Problem
The original issue reported that "ReplayGain volume adjustments are only applied after the track begins playing, which leads to a noticeable change in volume a fraction of a second into the track."
Root Cause:
setReplayGain()was called in the success callback AFTERplayer.prepare()completedSolution
The fix uses a proactive update model with ReplayGain values embedded in MediaItems:
1. MediaItem Tagging Architecture