| Summary: | Option for KMilo to change PCM instead of MASTER | ||
|---|---|---|---|
| Product: | [Unmaintained] kmilo | Reporter: | Tobias Niwi <niwi-hh> |
| Component: | general | Assignee: | George Staikos <staikos> |
| Status: | RESOLVED FIXED | ||
| Severity: | wishlist | CC: | apoikos, EagleScreen, greg_g, rasasi78, slava, tdfischer |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Ubuntu | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
Proposed patch
Proposed patch for mute/unmute events Updated patch for mute/unmute events Patch for kmix-3.5.8 to add new dcop call (masterDeviceIndex) And the fix for kmilo, that uses that new DCop call. |
||
|
Description
Tobias Niwi
2006-09-28 23:37:36 UTC
Created attachment 18165 [details]
Proposed patch
I too think that kmilo should honour kmix's "Master channel" setting.
Personally I use a laptop and select either the Headphone channel, or the
Internal Speaker channel as the Master channel in kmix, and I would like kmilo
to honour my choice. By default kmilo calls kmix's "volume" and "setVolume" for
channel 0, which in many cases may not be desired. The attached patch fixes
this by using kmix's "masterVolume" and "setMasterVolume" DCOP calls, instead
of "volume" and "setVolume" respectivelly. Unfortunatelly I can't do the same
for muting, since kmix doesn't provide a "muteMaster" DCOP call (perhaps
another wishlist item?).
I noticed that kmilo now respects the selected master channel from kmix in KDE 3.5.5, which is great. However the kmilo mute/unmute still affects only the first channel rather than the selected master channel from kmix. Created attachment 18751 [details] Proposed patch for mute/unmute events This patch should be used in conjunction with https://bugs.kde.org/attachment.cgi?id=18750 which is part of bug https://bugs.kde.org/show_bug.cgi?id=134604 Created attachment 18754 [details] Updated patch for mute/unmute events This patch is for use in conjunction with attachment #18753 [details] for kmix (http://bugs.kde.org/attachment.cgi?id=18753). Please refer to http://bugs.kde.org/show_bug.cgi?id=134604 Seems like my patch wasn't needed, William had already done the same work :-) Willia, Apollon, thanks a lot. Your patches look very complete (including all KMilo "backends"). I'll check back with the KMilo maintainer, but I think this patch should be commited short term. SVN commit 610688 by esken:
Adding DCOP interface for muting master. It should have been added
earlier (when the masterVolume() DCOP calls were introduced).
Also neccesary, as it aids in fixing a KMilo/KMix interoperability
issue.
CCBUGS: 134820
CCBUGS: 134604
M +65 -5 mixer.cpp
M +3 -0 mixer.h
M +27 -0 mixerIface.h
--- branches/KDE/3.5/kdemultimedia/kmix/mixer.cpp #610687:610688
@@ -33,7 +33,12 @@
#include "kmix-platforms.cpp"
#include "volume.h"
+//#define MIXER_MASTER_DEBUG
+#ifdef MIXER_MASTER_DEBUG
+#warning MIXER_MASTER_DEBUG is enabled. DO NOT SHIP KMIX LIKE THIS !!!
+#endif
+
/**
* Some general design hints. Hierachy is Mixer->MixDevice->Volume
*/
@@ -264,6 +269,11 @@
*/
void Mixer::readSetFromHW()
{
+ if ( ! _mixerBackend->isOpen() ) {
+ // bail out immediately, if the mixer is not open.
+ // This can happen currently only, if the user executes the DCOP close() call.
+ return;
+ }
bool updated = _mixerBackend->prepareUpdateFromHW();
if ( (! updated) && (! _readSetFromHWforceUpdate) ) {
// Some drivers (ALSA) are smart. We don't need to run the following
@@ -363,12 +373,19 @@
Mixer* Mixer::masterCard()
{
Mixer *mixer = 0;
+ kdDebug(67100) << "Mixer::masterCard() searching for id=" << _masterCard << "\n";
for (mixer=Mixer::mixers().first(); mixer!=0; mixer=Mixer::mixers().next())
{
if ( mixer->id() == _masterCard ) {
+#ifdef MIXER_MASTER_DEBUG
+ kdDebug(67100) << "Mixer::masterCard() found id=" << mixer->id() << "\n";
+#endif
break;
}
}
+#ifdef MIXER_MASTER_DEBUG
+ if ( mixer == 0) kdDebug(67100) << "Mixer::masterCard() found no Mixer* mixer \n";
+#endif
return mixer;
}
@@ -379,6 +396,9 @@
// Also you can set the master at any time you like, e.g. after reading the KMix configuration file
// and before actually constructing the Mixer instances (hint: this mehtod is static!).
_masterCardDevice = ref_id;
+#ifdef MIXER_MASTER_DEBUG
+ kdDebug(67100) << "Mixer::setMasterCardDevice(\"" << ref_id << "\")\n";
+#endif
}
MixDevice* Mixer::masterCardDevice()
@@ -387,15 +407,24 @@
Mixer *mixer = masterCard();
if ( mixer != 0 ) {
for( md = mixer->_mixerBackend->m_mixDevices.first(); md != 0; md = mixer->_mixerBackend->m_mixDevices.next() ) {
-/*
- kdDebug(67100) << "Mixer::masterCardDevice() getPK()="
- << md->getPK() << " , _masterCardDevice="
- << _masterCardDevice << "\n";
-*/
+
+
if ( md->getPK() == _masterCardDevice )
+ {
+#ifdef MIXER_MASTER_DEBUG
+ kdDebug(67100) << "Mixer::masterCardDevice() getPK()="
+ << md->getPK() << " , _masterCardDevice="
+ << _masterCardDevice << "\n";
+#endif
break;
+ }
}
}
+
+#ifdef MIXER_MASTER_DEBUG
+ if ( md == 0) kdDebug(67100) << "Mixer::masterCardDevice() found no MixDevice* md" "\n";
+#endif
+
return md;
}
@@ -643,6 +672,16 @@
_mixerBackend->writeVolumeToHW(deviceidx, mixdev->getVolume() );
}
+// @dcop only
+void Mixer::setMasterMute( bool on )
+{
+ MixDevice *master = masterDevice();
+ if (master != 0 ) {
+ setMute( master->num(), on );
+ }
+}
+
+
// @dcop
void Mixer::toggleMute( int deviceidx )
{
@@ -656,6 +695,16 @@
_mixerBackend->writeVolumeToHW(deviceidx, mixdev->getVolume() );
}
+// @dcop only
+void Mixer::toggleMasterMute()
+{
+ MixDevice *master = masterDevice();
+ if (master != 0 ) {
+ toggleMute( master->num() );
+ }
+}
+
+
// @dcop
bool Mixer::mute( int deviceidx )
{
@@ -665,6 +714,17 @@
return mixdev->isMuted();
}
+// @dcop only
+bool Mixer::masterMute()
+{
+ MixDevice *master = masterDevice();
+ if (master != 0 ) {
+ return mute( master->num() );
+ }
+ return true;
+}
+
+
bool Mixer::isRecordSource( int deviceidx )
{
MixDevice *mixdev= mixDeviceByType( deviceidx );
--- branches/KDE/3.5/kdemultimedia/kmix/mixer.h #610687:610688
@@ -124,8 +124,11 @@
virtual int masterVolume();
virtual void setMute( int deviceidx, bool on );
+ virtual void setMasterMute( bool on );
virtual bool mute( int deviceidx );
+ virtual bool masterMute();
virtual void toggleMute( int deviceidx );
+ virtual void toggleMasterMute();
virtual bool isRecordSource( int deviceidx );
virtual bool isAvailableDevice( int deviceidx );
--- branches/KDE/3.5/kdemultimedia/kmix/mixerIface.h #610687:610688
@@ -62,14 +62,27 @@
*/
virtual void setMute( int deviceidx, bool on )=0;
/**
+ Mutes or unmutes the master device.
+ */
+ virtual void setMasterMute( bool on )=0;
+ /**
Toggles mute-state for the given device.
*/
virtual void toggleMute( int deviceidx )=0;
/**
+ Toggles mute-state for the master device.
+ */
+ virtual void toggleMasterMute()=0;
+ /**
Returns if the given device is muted or not. If the device is not
available in this mixer, it is reported as muted.
*/
virtual bool mute( int deviceidx )=0;
+ /**
+ Returns if the master device is muted or not. If the device is not
+ available in this mixer, it is reported as muted.
+ */
+ virtual bool masterMute()=0;
/**
Makes the given device a record source.
@@ -98,6 +111,20 @@
*/
virtual QString mixerName()=0;
+ /**
+ * Open/grab the mixer for further intraction
+ * You should use this method after a prior call of close(). See close() for usage directions.
+ */
+ virtual int open()=0;
+
+ /**
+ * Close/release the mixer
+ * This method SHOULD NOT be used via DCOP. You MAY use it if you really need to close the mixer device,
+ * for example when a software suspend action (ACPI) takes place, and the soundcard driver
+ * doesn't handle this situation gracefully.
+ */
+ virtual int close()=0;
+
};
#endif
SVN commit 610844 by esken:
KMilo now uses the new KMix DCOP interface for muting master. Aids in
fixing a KMilo/KMix interoperability issue. This fixes two bugs:
BUGS: 134820
BUGS: 134604
M +2 -2 delli8k/delli8k.cpp
M +4 -4 generic/generic_monitor.cpp
M +4 -4 kmilo_kvaio/kvaio.cpp
M +1 -1 thinkpad/thinkpad.cpp
--- branches/KDE/3.5/kdeutils/kmilo/delli8k/delli8k.cpp #610843:610844
@@ -195,7 +195,7 @@
{
bool kmix_error = false;
- DCOPReply reply = kmixClient->call( "mute", 0 );
+ DCOPReply reply = kmixClient->call( "masterMute" );
if( reply.isValid() )
{
@@ -234,7 +234,7 @@
void DellI8kMonitor::setMute( bool b )
{
m_mute = b;
- kmixClient->send( "setMute", 0, m_mute );
+ kmixClient->send( "setMasterMute", m_mute );
}
int DellI8kMonitor::fn_status( int fd )
--- branches/KDE/3.5/kdeutils/kmilo/generic/generic_monitor.cpp #610843:610844
@@ -169,7 +169,7 @@
if (m_mute)
{
m_mute = false;
- kmixClient->send("setMute", 0, m_mute);
+ kmixClient->send("setMasterMute", m_mute);
}
}
@@ -177,7 +177,7 @@
{
bool kmix_error = false;
- DCOPReply reply = kmixClient->call("mute", 0);
+ DCOPReply reply = kmixClient->call("masterMute");
if (reply.isValid())
m_mute = reply;
else
@@ -190,7 +190,7 @@
if (kapp->startServiceByDesktopName("kmix")==0) // trying to start kmix
{
// trying again
- reply = kmixClient->call("mute", 0);
+ reply = kmixClient->call("masterMute");
if (reply.isValid())
{
m_mute = reply;
@@ -234,7 +234,7 @@
muteText = i18n("Mute off");
}
- kmixClient->send("setMute", 0, m_mute);
+ kmixClient->send("setMasterMute", m_mute);
_interface->displayText(muteText);
}
--- branches/KDE/3.5/kdeutils/kmilo/kmilo_kvaio/kvaio.cpp #610843:610844
@@ -461,7 +461,7 @@
if (m_mute)
{
m_mute = false;
- kmixClient->send("setMute", 0, m_mute);
+ kmixClient->send("setMasterMute", m_mute);
}
}
@@ -535,7 +535,7 @@
{
bool kmix_error = false;
- DCOPReply reply = kmixClient->call("mute", 0);
+ DCOPReply reply = kmixClient->call("masterMute");
if (reply.isValid())
m_volume = reply;
else
@@ -549,7 +549,7 @@
if (kapp->startServiceByDesktopName("kmix")==0) // trying to start kmix
{
// trying again
- reply = kmixClient->call("mute", 0);
+ reply = kmixClient->call("masterMute");
if (reply.isValid())
{
m_mute = reply;
@@ -595,7 +595,7 @@
muteText = i18n("Mute off");
}
- kmixClient->send("setMute", 0, m_mute);
+ kmixClient->send("setMasterMute", m_mute);
//_interface->displayText(muteText);
showTextMsg(muteText);
--- branches/KDE/3.5/kdeutils/kmilo/thinkpad/thinkpad.cpp #610843:610844
@@ -89,7 +89,7 @@
showToggleMessage(i18n("Mute on"), i18n("Mute off"), thinkpad_state.mute_toggle == 1);
if (m_softwareVolume || m_volumeStep != defaultVolumeStep) {
- kmixClient->send("setMute", 0, thinkpad_state.mute_toggle == 1);
+ kmixClient->send("setMasterMute", thinkpad_state.mute_toggle == 1);
}
}
Unfortunately, it seems that the old behaviour is back under 3.5.7. kmilo is using the new DCOP functions "setAbsoluteVolume, absoluteVolume, absoluteVolumeMax and absoluteVolumeMin" provided by kmix, which are again hardcoded to device 0. So, the same process should be followed again to fix it: register the "master" versions of these DCOP functions in kmix and use them in kmilo. Reported from Debian BTS at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=427374 Volume up/down no longer changes the "Master Channel" as specified by kmix. Interestingly, the mute button now correctly selects the Master Channel. I suggest to reopen this bug, and maybe fix it by reverting the associated commit? Commit was number 624936, that was meant to solve rounding issues of volume steps (bug #140707), but has the problem of the hardcoded device and also changed the generic backend without changing the other backends. Note that this is also valid for KDE4 (brought by commit 644601). I think this warrants the filing of a new bug; is there not one already? Both of my new machines (running KDE 3.5.7 and 3.5.8) have this problem with their respective onboard cards. Created attachment 21995 [details]
Patch for kmix-3.5.8 to add new dcop call (masterDeviceIndex)
Might as well fix it myself, it wasn't tough.
I've tested this on x86_64/gentoo/kde-3.5.8 with alsa with a snd-intel-hda card
(though the changes are trivial).
This is the kmix portion of the fix.
Created attachment 21996 [details]
And the fix for kmilo, that uses that new DCop call.
And the kmilo part of the patch.
I tried the patches and I confirm that they solve the issue. Yep, I've filed a new bug for this, in hopes that they'll see this: https://bugs.kde.org/show_bug.cgi?id=151862 SVN commit 754430 by esken: Adding a patch for a KMilo issue - KMilo always uses index #0, which is not alway the master. This regression came into Kmilo with KDE3.5.7 or KDE3.5.8. The patch is applied as-is - to use it, KMilo must be changed as well (a corresponding patch is found in http://bugs.kde.org/show_bug.cgi?id=134820#c14 CCBUGS: 134820 M +5 -0 mixer.cpp M +1 -0 mixer.h M +5 -0 mixerIface.h WebSVN link: http://websvn.kde.org/?view=rev&revision=754430 Extra hints: a) This bug is now closed, as everything is now available for KMilo, and a bug report has been opened for KMilo. b) The state in KDE4 is undetermined c) KMilo is pretty much unmaintained, so it is not clear whether somebody will apply the patch from comment 14 *** Bug 149487 has been marked as a duplicate of this bug. *** *** Bug 128224 has been marked as a duplicate of this bug. *** *** Bug 157026 has been marked as a duplicate of this bug. *** *** Bug 106251 has been marked as a duplicate of this bug. *** |