Skip to content
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

Volume normalization #15

Closed
cortegedusage opened this issue Jan 29, 2018 · 8 comments
Closed

Volume normalization #15

cortegedusage opened this issue Jan 29, 2018 · 8 comments

Comments

@cortegedusage
Copy link

I think volume normalisation also should be added to this repo.
I've been using
https://github.com/herrernst/librespot/tree/volume-normalization/src

for a couple of weeks now, and it's working quite well

@alin23
Copy link

alin23 commented Jan 29, 2018

@cortegedusage Sounds good!
I will prepare a PR for this if @ComlOnline is ok with it.

@ComlOnline
Copy link
Contributor

Yeah that would be great, the only thing I ask is that you add a compile time flag like --features "normalization" as we're discussing in other issues we plan on keeping librespot relatively slimline in its default build state and also have a proper daemon style thing with all the bells and whistles.

@herrernst
Copy link
Contributor

@ComlOnline I don't see a point hiding this few lines of code (that don't have any dependencies) behind a compile-time feature flag; also, they are not executed if the command line flag isn't added when running.

@alin23 If you don't mind waiting, I will prepare a pull request in the next days which fixes the edge cases where clipping (distortion) does occur.

@alin23
Copy link

alin23 commented Jan 29, 2018

@herrernst I can wait ^_^

@ComlOnline
Copy link
Contributor

@herrernst I see your point and you are correct. I didn't see the command line flag, my fault.
My next question is should normalization be the default function and you have to pass a flag to turn it off?
I think most people would unknowingly appreciate this.

@herrernst
Copy link
Contributor

I now consider my branch ready for merging: https://github.com/herrernst/librespot/tree/volume-normalization
I wouldn't want this feature enabled by default; the remaining problem is that I'm not able to figure out if an album is played vs. a playlist, so it only applies track-specific normalization. But that is not what you want if you play an album, because you lose inter-track dynamics, and if there are gapless transitions (like a live album), volume would change abruptly when changing tracks.

@ComlOnline
Copy link
Contributor

ComlOnline commented Jan 31, 2018

@herrernst Please could you make a PR?
And I agree about inter track dynamics, usually the album will be mixed so the tracks are complimentary.

@sashahilton00
Copy link
Member

Added by #162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants