Ticket #9913 (new defect)

Opened 5 years ago

Last modified 9 months ago

sugar frame speaker icon does not reflect volume hot key usage

Reported by: Andy Pei Owned by: godiard
Priority: normal Milestone: Future Release
Component: sugar Version: 1.5 Software Build os64 aka 10.1.0
Keywords: Cc: pgf, TMarques, godiard, dsd, maximilian, mtd
Action Needed: design Verified: no
Deployments affected: Blocked By:
Blocking: #10229

Description (last modified by Quozl) (diff)

Change to volume using keyboard keys are effective but are not reflected in the frame speaker icon.

Environment: XO-1.5 B1 B2, Q3A24 Q3A25 firmware, OS62 OS64.

Attachments

alsamixernotify.c (3.1 kB) - added by Quozl 5 years ago.
C implementation of an ALSA mixer change notifier, demonstrating the minimum use of the ALSA API for the purpose of detecting mixer changes without enumerating them.
9913-12-1-0-os7-testcase-setup.png (54.2 kB) - added by Quozl 2 years ago.
test case setup example
9913-data-flow.png (104.3 kB) - added by Quozl 2 years ago.
proposed data flow diagram, made using Graphviz

Change History

  Changed 5 years ago by Quozl

  • status changed from new to closed
  • resolution set to worksforme

Attempted to reproduce on XO-1.5 B2, Q3A25, OS64, but symptom does not occur. Tested on two units.

Also tested that alsamixer shows both volume keys are responded to.

Also tested that OpenFirmware handled both volume keys (by repeated use of sound command).

Please retest on later hardware model and versions, thanks.

If this is thought to be a hardware unit failure, please test with OpenFirmware test /keyboard and verify the function of the keyboard matrix.

  Changed 5 years ago by Andy Pei

  • status changed from closed to reopened
  • resolution deleted

So sorry for replying not clearly. I mains the volume which in the horn icon doesn't increase or decrease if you change volume size. Test on OS64 Q3A25 OFW.

  Changed 5 years ago by Quozl

  • next_action changed from never set to diagnose
  • component changed from not assigned to sugar
  • version changed from not specified to 1.5 Software Build os64 aka 10.1.0

Ah, yes, now I understand.

You're right.

  • pressing the volume keys on the keyboard does result in a change to the actual volume setting, and the volume levels of the Master control shown by alsamixer,
  • these changes are not shown by the frame icon My Speakers, or the palette,
  • changing the level using the palette horizontal scroll bar does change the icon status.

So the problem is that the keyboard directed volume changes are not being displayed by the Sugar shell.

  Changed 5 years ago by Quozl

  • description modified (diff)

  Changed 5 years ago by Quozl

  • summary changed from Volume hot key doesn't work. to sugar frame speaker icon does not reflect volume hot key usage

  Changed 5 years ago by dsd

  • owner set to sayamindu
  • status changed from reopened to new

follow-up: ↓ 10   Changed 5 years ago by sayamindu

  • cc pgf added
  • status changed from new to assigned

I think this is being caused due to olpc-kbdshim handling the volume keys directly (we switched to this behaviour to allow users to set volume in GNOME).

Paul, is there any way to disable this when sugar starts up ?

With the current code in Sugar, I see no way of notifying the trayicon that the volume has changed (apart from polling the volume level).

follow-up: ↓ 15   Changed 5 years ago by Quozl

On notifying the tray icon that the levels have changed ... ALSA has an API for notification of mixer changes, see snd_mixer_poll_descriptors. The file descriptor can then be monitored by the main event loop.

Both pyalsaaudio and pyglet contain code that reference this part of the ALSA API.

  • pyalsaaudio : alsaaudio.c
  • pyglet: media/drivers/alsa/asound.py

Changed 5 years ago by Quozl

C implementation of an ALSA mixer change notifier, demonstrating the minimum use of the ALSA API for the purpose of detecting mixer changes without enumerating them.

  Changed 5 years ago by Quozl

sugar-toolkit.git/src/sugar/acme-volume-alsa.c is a C implementation of ALSA mixer read and write used by Sugar.

This would be a logical place to integrate detection of mixer changes.

in reply to: ↑ 7   Changed 5 years ago by pgf

Replying to sayamindu:

I think this is being caused due to olpc-kbdshim handling the volume keys directly (we switched to this behaviour to allow users to set volume in GNOME). Paul, is there any way to disable this when sugar starts up ?

oops. forgot to reply to this earlier. there's no way currently to disable this just while sugar is running. until recently the /usr/bin/olpc-volume script (which does the work for kbdshim) could have signalled the daemon that it should pass the volume keys along by returning failure, but we took this out to appease the screen-rotation gods. we could put that back, and change olpc-rotate to always return true, to compensate. or something.

but i sort of thouht the longer-term goal was for sugar to get out of the hardware-control business. does SoaS control the volume on a non-XO? what will happen in the next version of sugar?

  Changed 5 years ago by Quozl

Yes, as far as I can see SoaS tries to control the volume on a non-XO ... the code remains present in git. It uses XF86AudioLowerVolume, XF86AudioRaiseVolume and XF86AudioMute or the XO F11 and F12 keys.

See also a thread exactly a year ago on devel@ especially the followups by you, Carol Farlow Lerche and David Lang,

September 2009 testing of soas63xo and soas70xo by Mikus showed volume is still manipulated by Sugar, although the default (muted) was not initially helpful.

See also SL 737 (sugar shouldn't handle XO keyboard keys directly) for the 0.88 directions.

My opinion is that since all other mixer applications and applets select for mixer updates and update their display when another program makes a mixer change ... then so should Sugar.

If we only need olpc-kbdshim's action on this because GNOME doesn't have mixer shortcuts or an applet ... perhaps finding an applet will fix that.

  Changed 4 years ago by Quozl

  • cc TMarques added

  Changed 4 years ago by Quozl

Failed factory test os206 component Audio test item Volume Control Function Test (alsamixer, amixer).

  Changed 4 years ago by Quozl

  • blocking 10229 added

in reply to: ↑ 8   Changed 4 years ago by Quozl

Replying to Quozl:

Both pyalsaaudio and pyglet contain code that reference this part of the ALSA API.

pyalsaaudio is packaged as python-alsaaudio and is included from build os850 of 10.1.2.

  Changed 4 years ago by Quozl

Also occurs 10.1.3 os359, per #10551.

  Changed 3 years ago by Quozl

  • blocking 11155 added

(In #11155) Yes, that's due to #10831, missing software controls for audio mixer. Added the other tickets as blocking this ticket.

  Changed 3 years ago by martin.langhoff

  • blocking 11155 removed

  Changed 2 years ago by Quozl

  • blocking 11780 added

(See also #11780).

  Changed 2 years ago by godiard

Ok, probably this is solved now in sugar (os7) but we need solve the key mappings.

|TestCase|

Show the frame, with Alt+Shift+F In a xo with rubber or mechanical keyboard, press Fn+VolumeUp or Fn+VolumeDown. You need press 3 times until the icon is changed.

Maybe in the rubber keyboard we should not use Fn key? Is possible?

follow-up: ↓ 39   Changed 2 years ago by Quozl

  • next_action changed from diagnose to design

Your test case is wrong, sorry. It is not a problem of key mapping.

When you use Fn+F11 and Fn+F12 these keys are handled by Sugar instead of olpc-kbdshim. When you use F11 and F12 these keys are handled by olpc-kbdshim. We want the keys to be handled by olpc-kbdshim, since olpc-kbdshim will work in text console, Sugar, or GNOME. Fn+F11 and Fn+F12 will only work in Sugar.

The test case should be: Start Sugar. Start Terminal actvitiy, run alsamixer, so that the Master mixer is visible on the left. Show the frame. Right click on the speaker icon to show the speaker volume. See the attached screenshot. Note how the Master mixer control is displayed now in three positions on screen; (1) the Sugar Frame, (2) alsamixer coloured vertical bar, (3) alsamixer selected control yellow text "dB gain".

Then, press F11 (VolumeDown) and the volume should visibly decrease on all three positions, press F12 (VolumeUp) and the volume should visibly increase on all three positions.

Currently this test case fails. Only the alsamixer controls change. The Sugar control does not.

Also, manual changes made in Terminal to the alsamixer control using up or down arrow keys should be reflected in the Sugar control. Currently this does not occur either.

Changed 2 years ago by Quozl

test case setup example

  Changed 2 years ago by Quozl

  • milestone changed from 1.5-software-later to 12.1.0

  Changed 2 years ago by Quozl

I shall attach a diagram to explain. In the diagram, the grey node with dotted lines "mixer notify" does not exist. That is the proposed solution I've discussed earlier on this ticket.

An alternate solution is to repeatedly call get volume in extensions/deviceicon/speaker.py, on a timer, say once a second, but only when the GTK+ slider widget is visible or just before it is made visible. This solution consumes more power.

Changed 2 years ago by Quozl

proposed data flow diagram, made using Graphviz

  Changed 2 years ago by Quozl

  • blocking 11780 removed

(diagram colour legend; blue is kernel, cyan is alsa-utils and x11, green is olpc, red is sugar, orange is sugar-toolkit).

  Changed 2 years ago by godiard

  • cc godiard added

May be we can do olpc-volume set the gconf variable /desktop/sugar/sound/volume and add code in sugar to detect this change?

We don't solve the problem if a user set the volume with alsamixer, but solve the issue with olpc-kbdshim / olpc-volume

  Changed 2 years ago by Quozl

No. In my opinion gconf should not be used as an interprocess communication mechanism. Your suggestion also won't work with activities that change the volume. What's wrong with using snd_mixer_poll_descriptors as I suggested? I've already proved this API works, and provided an example program ... all you have to do is port it to sugar-toolkit.git and sugar.git.

  Changed 2 years ago by Quozl

http://dev.laptop.org/~quozl/z/1SKO9d.txt is an example of a change toward the alternate solution of repeatedly updating the slider. however, it does not function correctly; sound.get_volume() is not showing any change as a result of alsamixer or keyboard changes.

i also found that the acme code has a four second timeout, after which it closes the alsa device. if the palette is popdown, followed by a five second pause, followed by popup palette, then the displayed value is correct.

therefore i conclude that the alternate solution is impractical, and nd_mixer_poll_descriptors is the better solution. unfortunately i don't know how to build the sugarext components for testing.

  Changed 2 years ago by godiard

I don't think having a timeout polling for the value is good because consume resources.

I am not talking about using gconf as interprocess communication, but as a common config place.

Also, this does not work with activities if they change alsa volume, what activity is doing it?

Finally, we are trying of move from alsa to pulseaudio....

follow-up: ↓ 30   Changed 2 years ago by Quozl

the resources are only consumed when the speaker palette is displayed. this is rare. therefore this should not be a concern. however, as this doesn't yet work and would require invasive changes to the acme-volume-alsa program in the sugar-toolkit, it remains less practical than the use of snd_mixer_poll_descriptors.

your solution of using gconf is effectively interprocess communication between olpc-volume and sugar. it would require disk access for every change, shortening the life of the microSD or eMMC. you would still need to re-read the gconf setting in a timed polling in order to display the up to date value.

the use of snd_mixer_poll_descriptors will work with activities when they change volume. measure and turtleart contain volume control manipulation code.

pulseaudio uses alsa underneath, so i don't see how you can use that as an excuse to avoid the problem. but it does bring up complexity. olpc-volume uses alsa. does the sugar team's intention to move to pulseaudio include changes for olpc-volume? where's the design plan?

in reply to: ↑ 29   Changed 2 years ago by godiard

Replying to Quozl:

the resources are only consumed when the speaker palette is displayed. this is rare.

Ok.

therefore this should not be a concern. however, as this doesn't yet work and would require invasive changes to the acme-volume-alsa program in the sugar-toolkit, it remains less practical than the use of snd_mixer_poll_descriptors.

your solution of using gconf is effectively interprocess communication between olpc-volume and sugar. it would require disk access for every change, shortening the life of the microSD or eMMC. you would still need to re-read the gconf setting in a timed polling in order to display the up to date value.

Yes. I understand your point now.

the use of snd_mixer_poll_descriptors will work with activities when they change volume. measure and turtleart contain volume control manipulation code. pulseaudio uses alsa underneath, so i don't see how you can use that as an excuse to avoid the problem.

Hey!, I am not using this as a excuse :)

but it does bring up complexity. olpc-volume uses alsa. does the sugar team's intention to move to pulseaudio include changes for olpc-volume? where's the design plan?

It's only in evaluation (#11499)

  Changed 2 years ago by erikos

  • owner changed from sayamindu to godiard
  • status changed from assigned to new

Gonzalo, asigning to you as Sayamindu is not around anymore.

  Changed 2 years ago by dsd

  • cc dsd added
  • milestone changed from 12.1.0 to Future Release

  Changed 2 years ago by Quozl

  • blocking 12237 added

(In #12237) See #9913, scheduled for a future release.

  Changed 2 years ago by dsd

  • blocking 12237 removed

(In #12237) Lets use this ticket to track inability to control volume on any level (due to mixer naming), rather than the visual feedback bug described in #9913.

  Changed 22 months ago by Quozl

  • blocking 12359 added

(In #12359) This is not a defect with CL4, but this is a known problem in Sugar, ticket #9913 still not fixed. It has been three years, so I doubt it will be fixed soon.

  Changed 22 months ago by Quozl

http://bugs.sugarlabs.org/ticket/1677 is an upstream ticket.

  Changed 22 months ago by dsd

  • blocking 12359 removed

(In #12359) I'll just go ahead and do that, please reopen if there are objections.

  Changed 21 months ago by maximilian

  • cc maximilian added

in reply to: ↑ 21   Changed 9 months ago by mtd

Replying to Quozl:

[Test case]: Start Sugar. Start Terminal actvitiy, run alsamixer, so that the Master mixer is visible on the left. Show the frame. Right click on the speaker icon to show the speaker volume. See the attached screenshot. Note how the Master mixer control is displayed now in three positions on screen; (1) the Sugar Frame, (2) alsamixer coloured vertical bar, (3) alsamixer selected control yellow text "dB gain".

If we separate (1) the Sugar Frame into (1a) the Sugar frame speaker icon; and (1b) the Sugar frame speaker icon palette slider bar, then the problem with the polling solution is that in order for the speaker icon to be accurate, it has to poll all the time it's visible. ISTR this is in general bad, but 1) could be acceptable, given the fact it's just once a second when the frame is visible; and 2) could be acceptable, given the fact there are already lots of wakeups (though this is IMHO a poor justification in theory) #2778-comment7.

Is polling all the time the speaker icon (or palette) is visible acceptable?

follow-up: ↓ 41   Changed 9 months ago by Quozl

  • cc mtd added

Thanks. Yes. That was the alternate solution that I mentioned above. Have you reviewed the change that I tested? I didn't figure out why it didn't work.

in reply to: ↑ 40 ; follow-up: ↓ 42   Changed 9 months ago by mtd

Replying to Quozl:

Thanks. Yes.

Great, thanks.

That was the alternate solution that I mentioned above. Have you reviewed the change that I tested? I didn't figure out why it didn't work.

Not in detail yet, sorry.

Just to confirm: has the snd_mixer_poll_descriptors change (aka "proposed solution", as opposed to the "alternate solution") of yours from comment #9 been rejected as too close to the hardware? I don't want to sidetrack us onto the "alternate solution" if the "proposed solution" is better / preferred.

in reply to: ↑ 41   Changed 9 months ago by Quozl

Replying to mtd:

Just to confirm: has the snd_mixer_poll_descriptors change (aka "proposed solution", as opposed to the "alternate solution") of yours from comment #9 been rejected as too close to the hardware? I don't want to sidetrack us onto the "alternate solution" if the "proposed solution" is better / preferred.

No, I don't think it has been rejected. Perhaps I missed that. Where has it been rejected? Both solutions use ALSA, which Sugar already uses. There was a mention of PulseAudio, but that is just another layer on top of ALSA, and the OLPC builds did not use PulseAudio. So switching to PulseAudio would be a far larger task with no apparent benefit.

Note: See TracTickets for help on using tickets.