mirror of
https://github.com/librespot-org/librespot.git
synced 2026-04-27 08:15:50 +03:00
[GH-ISSUE #655] normalisation data for --passthrough #384
Labels
No labels
A-Alsa
SpotifyAPI
Tokio 1.0
audio
bug
can't reproduce
compilation
dependencies
duplicate
enhancement
good first issue
help wanted
high priority
imported
imported
invalid
new api
pull-request
question
reverse engineering
wiki
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/librespot#384
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @DeLub on GitHub (Feb 26, 2021).
Original GitHub issue: https://github.com/librespot-org/librespot/issues/655
Originally assigned to: @philippe44 on GitHub.
Hi,
Currently the normalisation options seem to have no effect when --passthrough is used, which makes sense. Is there an option when piping into ffmpeg with --passthrough to also receive the replaygain values? Can they be inserted at the start of the track in the headers?
@philippe44 commented on GitHub (Mar 27, 2021):
It would require a more extensive rewrite of the headers which purposely very minimal at that moment. I can look into that later once the updated passthrough option has been adopted and feedback is good.
@roderickvd commented on GitHub (May 24, 2021):
@philippe44 I took the liberty of assigning this one to you, if you'd have the time to spare and think this is a good idea.
@roderickvd commented on GitHub (Aug 7, 2021):
@philippe44 is this something you would want to look into?
@philippe44 commented on GitHub (Aug 7, 2021):
Do you think this is an important feature?
My last attempt to change something in librespot was rejected (https://github.com/librespot-org/librespot/discussions/741 and https://github.com/philippe44/librespot/tree/HTTP-sink) and I can understand why but as I'm not good (to say the least) at Rust, these modifications are a lot of hard work for me and I lost a bit my motivation since last time.
I will maintain, when needed, my original contribution, but I'm not sure I'm willing to take the challenge for this replaygain option, especially because it's potentially a fair bit of extra work as one now need to dig a level lower into headers (and in Rust...). Give me a couple of weeks to think about that.
@roderickvd commented on GitHub (Aug 9, 2021):
I feel the best way I can answer is by principle: I think that the passthrough decoder should do exactly what's on the tin, namely, passing through the audio files as they are.
Taking that as a guiding principle, I think it would be good that if there's normalisation data in the incoming audio file, that should also be in the passed through audio files.
TL;DR yes I think so for feature-completeness.
I am sorry that you feel that way. I never intended for that.
Thanks for that commitment of maintainership.
If it's of any help, these lines show where the normalisation data is at in Ogg Vorbis:
github.com/librespot-org/librespot@68bec41e08/playback/src/player.rs (L217-L234)@philippe44 commented on GitHub (Aug 9, 2021):
Thanks for the answer - I understand the statement "the passthrough decoder should do exactly what's on the tin, namely, passing through the audio files as they are" and that's what I'm doing but I think the OP wants something different.
AFAII, the original problem in librespot was that replaygain was ignored by the decoder and a PR was made in 2018 to apply gain on PCM samples. Then an option was added to add a manual replaygain tweak (and maybe a few other options I don't know). So my understanding is that the ask is to use that "extra" replaygain and to add/insert it in re-written headers.
But I'll re-check all that as I might have misunderstood something there
@DeLub commented on GitHub (Aug 9, 2021):
My suggested feature request was to have the original replaygain values that go along with the original, untouched audio data.
@philippe44 commented on GitHub (Aug 9, 2021):
So I'm a bit confused then b/c the replaygain, as set in the ogg headers given by spotify is passthrough as well. I don't remove/touch it
@philippe44 commented on GitHub (Aug 24, 2021):
@DeLub: can you comment? Do you see something different?
@DeLub commented on GitHub (Sep 2, 2021):
I'm sorry for the late reply.
When I created this request, I was not able to normalise with ffmpeg. That's why I put forward this request.
However, at this moment I'm not using Spotify anymore. I switched to Apple Music as the Apple One subscription saves me money, and actually AM and Spotify are quite comparable in my opinion.
@roderickvd commented on GitHub (Sep 2, 2021):
OK, let's close this then. Thanks for getting back.
@roderickvd commented on GitHub (Jan 2, 2022):
To revisit this for posterity:
Having worked on the audio data handling, I now understand where this is coming from -- and @philippe44 was right. Spotify doesn't store the ReplayGain values in the Vorbis comments where you'd expect them (and which are passed through 1-on-1), but in a custom Ogg packet before it.
If you want to have the ReplayGain fields as Vorbis comments, then you'd have to modify the stream to insert them. This is currently not what the passthrough decoder does nor should, probably. Modifying is not passing through.