Skip to content

spirc: Configurable volume control steps#1498

Merged
photovoltex merged 7 commits into
librespot-org:devfrom
ssmccoy:volume-steps
May 1, 2025
Merged

spirc: Configurable volume control steps#1498
photovoltex merged 7 commits into
librespot-org:devfrom
ssmccoy:volume-steps

Conversation

@ssmccoy

@ssmccoy ssmccoy commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Allow the volume control steps to be configured via the --volume-steps command line parameter. The author personally found the default volume steps of 1024 to be completely unusable, and is presently using 128 as his configuration. Updated the default to 64 which more closely mirrors the official client behavior.

Additionally, reduce the delay in volume update from a wopping two seconds to 500ms, again for usability.

Also clean up the seemingly unnecessary use of a pattern match on whether or not --initial-volume was supplied.

Allow the volume control steps to be configured via the `--volume-steps`
command line parameter. The author personally found the default volume
steps of `1024` to be completely unusable, and is presently using `128`
as his configuration. Perhaps consider this as a more reasonable
default.

Additionally, reduce the delay in volume update from a wopping two
seconds to 500ms, again for usability.

Also clean up the seemingly unnecessary use of a pattern match on
whether or not `--initial-volume` was supplied.
@photovoltex photovoltex requested a review from Copilot April 29, 2025 17:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a configurable volume control step parameter while also reducing the volume update delay for improved usability, and it refines the handling of the initial volume option.

  • Adds the new --volume-steps option and its associated description and parsing logic.
  • Reduces the volume update delay in the spirc module from 2 seconds to 500ms.
  • Simplifies the branching for initial volume handling by eliminating an unnecessary pattern match.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/main.rs Introduces --volume-steps option and streamlines initial volume handling.
connect/src/spirc.rs Reduces volume update delay from 2s to 500ms.

@photovoltex photovoltex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the submission :)

Some points as context why they are like they are and potential change suggestions.

Comment thread connect/src/spirc.rs
Comment thread src/main.rs
Comment thread src/main.rs

@photovoltex photovoltex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for your patience on the topic.

And in addition to the comment, please add a changelog entry for your changes :)

Comment thread connect/src/state.rs Outdated
Comment thread connect/src/state.rs Outdated
@ssmccoy

ssmccoy commented May 1, 2025

Copy link
Copy Markdown
Contributor Author

If you merge this, make sure it is a squash merge you don't want those fixup commits lingering.

Otherwise I can squash them together into a single commit and merge that.

@photovoltex

Copy link
Copy Markdown
Member

I will just link to what was said in this #1495 (comment). But yeah no worries, will do :)

@photovoltex photovoltex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, will wait a bit if someone wants to add something to the topic

@photovoltex photovoltex merged commit 6bdc0eb into librespot-org:dev May 1, 2025
13 checks passed
@photovoltex

Copy link
Copy Markdown
Member

@ssmccoy Just as info, nothing to worry about :)

Something doesn't seem to line up in your git config. If you look at the squashed commit 6bdc0eb there are seemingly two people who worked at the change, but I would assume it was all you.

paulfariello pushed a commit to paulfariello/librespot that referenced this pull request Sep 23, 2025
* spirc: Configurable volume control steps

Allow the volume control steps to be configured via the `--volume-steps`
command line parameter. The author personally found the default volume
steps of `1024` to be completely unusable, and is presently using `128`
as his configuration. Perhaps consider this as a more reasonable
default.

Additionally, reduce the delay in volume update from a wopping two
seconds to 500ms, again for usability.

Also clean up the seemingly unnecessary use of a pattern match on
whether or not `--initial-volume` was supplied.

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

---------

Co-authored-by: Scott S. McCoy <scott.s.mccoy@acm.org>
CreatorMetaSky pushed a commit to StreamMediaSpace/librespot-zig that referenced this pull request Feb 9, 2026
* spirc: Configurable volume control steps

Allow the volume control steps to be configured via the `--volume-steps`
command line parameter. The author personally found the default volume
steps of `1024` to be completely unusable, and is presently using `128`
as his configuration. Perhaps consider this as a more reasonable
default.

Additionally, reduce the delay in volume update from a wopping two
seconds to 500ms, again for usability.

Also clean up the seemingly unnecessary use of a pattern match on
whether or not `--initial-volume` was supplied.

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

* fixup! spirc: Configurable volume control steps

---------

Co-authored-by: Scott S. McCoy <scott.s.mccoy@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants