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

Refactor Volume control, allow for a fixed volume option #447

Merged
merged 1 commit into from
Jul 25, 2020

Conversation

ashthespy
Copy link
Member

@ashthespy ashthespy commented Mar 9, 2020

This is a (long) overdue follow up to #286 (comment).

I have refactored the old --linear-volume flag to a more generic --volume-ctrl option that takes the options of [linear, log, fixed]. It defaults as previously to log.

What fixed does is disable the client side volume control, by setting kVolumeSteps to 0, so no volume events are passed to the client.
image

As usual, this will need a update of the wiki, and should be considered a "breaking change" when considering the versioning.

Fixes #48, Closes #286

@sashahilton00
Copy link
Member

Thanks for this, will give it a test. W.R.T the breaking aspect, we might have to hold off merging this for a bit. We might be best off considering this for the v1 release, along with #134. Also, at some point we need to start thinking about fleshing out librespotd, and stabilising librespot as the core library. That way changes such as these can be added to the former without breaking the latter.

@ashthespy
Copy link
Member Author

@sashahilton00 just to clarify the "breaking" change here would be the flag passed into the binary, and the VolumeCtrl enum that libraries using spirc will need.
Not sure if this would be considered a version bump, and we are still merging to the dev branch anyway..

@sashahilton00
Copy link
Member

This can be reviewed and merged to dev now that v0.1.2 is released.

@sashahilton00
Copy link
Member

@ashthespy since the docs are broken and shouldn't really be, I think we can just merge this into dev along with #504 (assuming this is actually a fix) and release it as v0.1.3 - let's face it, we're not exactly rigorously sticking to semver atm ;)

@ashthespy
Copy link
Member Author

Sounds like a plan, I'll fix the conflicts later today.

@ashthespy ashthespy merged commit f0b3b2c into librespot-org:dev Jul 25, 2020
@ashthespy
Copy link
Member Author

Rebased on dev, and fixed the conflicts. I was a bit trigger happy with the merge, but it should be all good now. Wiki has also been updated.

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

Successfully merging this pull request may close these issues.

Add mixer fixedvol
2 participants