Bug 308014 - Information via DBus does not update properly
Summary: Information via DBus does not update properly
Status: RESOLVED FIXED
Alias: None
Product: kmix
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: VHI major (vote)
Target Milestone: ---
Assignee: Christian Esken
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-07 10:51 UTC by Igor Poboiko
Modified: 2012-12-06 00:17 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 4.10


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Igor Poboiko 2012-10-07 10:51:22 UTC
When I cange volume using KMix, the DBus wrapper does not get these updates so it cannot populate new volume level via DBus. The problem is that when DBusMixerWrapper is created, it adds itself as a listener for mixer (e.g PulseAudio::Playback_Devices:0), and then mixers ID changes (using recreateId() method, e.g to PulseAudio::Playback_Devices:1). And this leads to this bug (DBusMixerWrapper does not listen to changes related to new mixer, which is actually the old one but with new id).

I can propose following solutions:
1) When mixerId changes, reconnect all the listeners from old mixerId to the new one (needs changes in ControlManager)
2) Or just reconnect it manually on recreateId (by calling removeListener and addListener from DBusMixerWrapper).

Reproducible: Always




This bug is in latest git snapshot, the version shipped with distro is not affected.
Comment 1 Christian Esken 2012-10-07 21:42:23 UTC
Thanks for the info, Igor. I'll try to fix it. My current idea is to create the DBusMixerWrapper later, when the device has the correct ID.

This must really really be fixed.

I am also wondering why the device ID changes at all. In my understanding there should only be just PulseAudio::Playback_Devices:0, never PulseAudio::Playback_Devices:1
Comment 2 Christian Esken 2012-10-07 23:04:12 UTC
(In reply to comment #1)
> Thanks for the info, Igor. I'll try to fix it. My current idea is to create
> the DBusMixerWrapper later, when the device has the correct ID.
> 
> This must really really be fixed.
> 
> I am also wondering why the device ID changes at all. In my understanding
> there should only be just PulseAudio::Playback_Devices:0, never
> PulseAudio::Playback_Devices:1

OK, PulseAudio::Playback_Devices:1 is correct (cards start with instanceNumber() 1, while controls start with number 0).
I also found an issue with the experimental mulit-driver mode: The DBUS API is broken with it, as there are multiple cards "0", and they seem to map the same DBUS object. So for example Mixers->2 contains a driverName == "PulseAudio", but also contains ALSA controls. I also need to fix it for KDE 4.10.
Comment 3 Christian Esken 2012-11-13 00:01:47 UTC
Git commit cb7e8e787a77fec416b3cdc853e515de89034b42 by Christian Esken.
Committed on 13/11/2012 at 01:00.
Pushed by esken into branch 'master'.

Starting to fix the DBUS interface. It still does not work, but the issue is understood.
mixer->openIfValid() requires the cardId, and then everything should be fine.

M  +0    -8    backends/kmix-backends.cpp
M  +1    -1    backends/mixer_backend.cpp
M  +11   -0    backends/mixer_backend.h
M  +15   -6    core/mixer.cpp
M  +17   -6    core/mixertoolbox.cpp
M  +1    -0    dbus/dbusmixerwrapper.cpp

http://commits.kde.org/kmix/cb7e8e787a77fec416b3cdc853e515de89034b42
Comment 4 Christian Esken 2012-11-13 00:10:37 UTC
@Igor We cannot use this simple numbering scheme for DBusMixerWrapper::DBusMixerWrapper(Mixer* parent, const QString& path). path used to be "/Mixers/0", "Mixers/1" and so forth. But these numbers are not really unique - I propose to use fully qualified names like "ALSA::Creative_X-Fi:1" instead.
Is that OK for you, Igor?
Comment 5 Christian Esken 2012-11-14 20:24:57 UTC
Git commit aaf1ef9224441d05b937e77180380238b386f887 by Christian Esken.
Committed on 14/11/2012 at 21:21.
Pushed by esken into branch 'master'.

Fix DBus interface (which was broken in trunk by my communication architecture upgrade.
The Mixer names are also now full qualified Mixer ID's instead of simple numbers - this is required as the ID's conflict in a multi-backend setup.

M  +3    -3    CMakeLists.txt
M  +21   -15   core/mixer.cpp
M  +5    -8    core/mixer.h
M  +16   -14   core/mixertoolbox.cpp
M  +2    -8    dbus/dbusmixerwrapper.cpp
M  +1    -1    gui/kmixdockwidget.cpp

http://commits.kde.org/kmix/aaf1ef9224441d05b937e77180380238b386f887
Comment 6 Igor Poboiko 2012-11-16 13:53:53 UTC
There is still a problem: sometimes when controls are added, the "_id" property of the mixer is an empty string, so mixers dbus path is wrong (and so is control's path). The debug output is:

kmix(8401) Mixer::dbusPath: Late _id= ""
kmix(8401) Mixer::dbusPath: handMade= "/Mixers/PulseAudio_PlaybackxDevices"
QDBusConnection for control created "/Mixers//alsa_output_pci_0000_01_00_1_hdmi_stereo" 
kmix(8401) Mixer_PULSE::addDevice: Adding Pulse volume  "alsa_output.pci-0000_00_1b.0.analog-stereo" , isCapture=  false , devnum= 0
Comment 7 Igor Poboiko 2012-11-16 13:54:59 UTC
And, BTW, everything should work fine with new DBus paths.
Comment 8 Christian Esken 2012-11-18 12:24:51 UTC
Need to look into the "empty id" thing. Reopeni g.
Comment 9 Christian Esken 2012-12-04 23:27:04 UTC
The empty "_id" are for the MixDevice instances. Thanks for being so observant, Igor. Its a actually a design bug [1], but I worked around it for KDE 4.10. Additionally I applied the changes from https://git.reviewboard.kde.org/r/107439/ . According to the logs there are no empty _id values any longer, and I tested with qdbusviewer - as far as I see everything is fine now.

Igor, a confirmation from you would be appreciated.



[1] Parts of the ID of a Mixer(Backend) are constructed outside the Mixer(Backend), namely the cardInstance.
Comment 10 Christian Esken 2012-12-04 23:31:11 UTC
Git commit 8241b0e9776fbebb8c2d07abcd008eededc07bd8 by Christian Esken.
Committed on 05/12/2012 at 00:30.
Pushed by esken into branch 'master'.

Fix wrong DBus paths for the MixDevice instances

M  +22   -4    core/mixer.cpp

http://commits.kde.org/kmix/8241b0e9776fbebb8c2d07abcd008eededc07bd8
Comment 11 Igor Poboiko 2012-12-05 12:25:58 UTC
As for me everything seem to work fine now. :)