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

When PNMixer card changes, the volume showed is wrong #94

Closed
elboulangero opened this issue Aug 8, 2015 · 9 comments
Closed

When PNMixer card changes, the volume showed is wrong #94

elboulangero opened this issue Aug 8, 2015 · 9 comments
Labels
Milestone

Comments

@elboulangero
Copy link
Collaborator

@landroni said:

Hmm... With Master volume at 55%, what I get is:

systray icon at full tilt (probably 100%)
systray tooltip at 100%
if I click on systray icon, I see volume at 55%

Moving it there to 57% adjusts both icon and tooltip to correct values...

@hasufell hasufell added this to the 0.6 milestone Aug 8, 2015
@hasufell
Copy link
Collaborator

hasufell commented Aug 8, 2015

I only have one card, so I cannot really test this. But my suspicion is that we need to replace this

void apply_prefs(gint alsa_change) {
[...]
  if (alsa_change)
    alsa_init();
[...]
}

With this:

void apply_prefs(gint alsa_change) {
[...]
  if (alsa_change)
    do_alsa_reinit();
[...]
}

@elboulangero
Copy link
Collaborator Author

Cool ! I didn't know about this do_alsa_reinit() thing... OK I'll test that right now

@elboulangero
Copy link
Collaborator Author

Pushed on branch https://github.com/nicklan/pnmixer/tree/fix-volume-on-card-change

Actually the code in apply_prefs() was OK, because basically the job that do_alsa_reinit() does was explicitly done afterward (ie, update icon and tooltip).

But the bugs came from the automatic card change when the current card is unplugged, in alsa.c.

@elboulangero
Copy link
Collaborator Author

@landroni Hey, you can also give a try to this branch, I tested it's OK, but two tests are always better than one

@landroni
Copy link

landroni commented Aug 9, 2015

Yup, this does it for me too.

@hasufell
Copy link
Collaborator

hasufell commented Aug 9, 2015

so can we merge this and then move on with the translation? I think we should be good to go then for the release

@landroni
Copy link

landroni commented Aug 9, 2015

@hasufell Is #87 hard to implement? Any chance it'll squeeze into the release?

@elboulangero
Copy link
Collaborator Author

I didn't look at it yet. I bet it's not a big thing, I'll give it a try asap

@elboulangero
Copy link
Collaborator Author

Merged

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

No branches or pull requests

3 participants