-
Notifications
You must be signed in to change notification settings - Fork 347
No replay gain applied on some random mix tracks and remote album tracks. #1481
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
base: public/9.1
Are you sure you want to change the base?
Conversation
Track metadata, including replay gain, is not retrieved for remote tracks in a random mix under some circumstances. This results in replay gain not being applied to those tracks. Signed-off-by: Sam Y <[email protected]>
| # Show getting-track-info message if still in TRACKWAIT & STOPPED or PLAYING | ||
| if ( ($self->{'playingState'} == STOPPED || $self->{'playingState'} == PLAYING) | ||
| && $self->{'streamingState'} == TRACKWAIT) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... looks simple. But wouldn't that cause side-effects with playing local tracks? Eg. when would a local track be in playing state vs. a remote track?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, and one for which I have no answer at this point. A few observations though:
-
In doing some further testing today without this change applied (unintentionally, due to installing the latest nightly build over it), I found that the problem was worse than I thought. Trying to play a Qobuz album with "Smart gain" turned on failed miserably, with no replay gain at all being applied to any tracks after the second one due to the absence of remote metadata. After realizing (with relief) that my fix was not present and reapplying it, Album gain was applied to each track as expected.
-
I have a feeling that the behavior being addressed here might actually be due to some other recent change(s) to the code, especially given what happened when trying to play Qobuz albums today in Smart gain mode. I would definitely have noticed a problem like that when originally testing the added support for replay gain in Qobuz. Additionally, at least two users without streaming services have recently reported problems with replay gain randomly not being applied to a local track when using Random Mix on their collections of music, one of whom says that every track in his collection is tagged with track gain, so this problem may not affect only remote tracks.
-
I have been testing as many track types and mixes as I can think of, including mixtures of local and remote tracks, along with radio streams, and haven't yet encountered any side effects from the change. Given the complexity of the StreamingController code, this might be one of those situations where the best thing to do is to promote the fix in 9.1 and be on the lookout for reports of any unusual behavior from the user base.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. After sleeping on it...
Forget item 2 above. This problem has likely been present for a long time. The reason it wasn't noticed before was that, until a change made a while back, the replay gain was calculated by fetchReplayGain(), passed to the player (squeezelite, e.g.) with the song for playback, and then forgotten. Then, at the request of client app developers, a change was made to return the RG value as part of the status query call. Until recently, this was done by re-calculating it via another call to fetchReplayGain() on each status query, which we now know could (and sometimes did) result in a different value than the one passed to the player. Since PR #1386, however (published last May), the original RG value calculated by fetchReplayGain() is now stored in the song object and retrieved from there when requested by the status query. This changes everything. The problem with the metadata not being available in the original call, which had previously been masked by the recalculation in the status query calls, is now made evident. The only way the problem could have been detected previous to these changes was with a decibel meter or a very loud song that was not being normalized. Even then, though, the RG value returned by the status query would lead one to believe that the RG had been applied when, in fact, it had not.
In summary, I feel that this change should be merged as soon as possible so that we can fix this long standing problem and begin to get some feedback on it, whether positive or negative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... looks simple. But wouldn't that cause side-effects with playing local tracks? Eg. when would a local track be in playing state vs. a remote track?
Okay, Michael. Hold the presses. Although this change does fix the problem, it does so only by virtue of the fact that it causes the remote track's metadata to be retrieved for a totally unrelated reason, that being to populate an informational message to be displayed by hardware players (I think). So just put this on hold for now. The problem needs some more research and a more focused solution. Sorry for the false start. If you have any other ideas on how to ensure that the replay gain metadata for remote tracks is available before starting playback, please feel free to share. Thanks.
Housecleaning from a previous fix. Populate $song->replayGain outside of (before) the player loop. Signed-off-by: Sam Y <[email protected]>
…nto public/9.1
Track metadata, including replay gain, may not be retrieved for remote tracks in a random mix under some circumstances. This results in replay gain not being applied to those tracks.