Bug 134820 - Option for KMilo to change PCM instead of MASTER
Summary: Option for KMilo to change PCM instead of MASTER
Status: RESOLVED FIXED
Alias: None
Product: kmilo
Classification: Miscellaneous
Component: general (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR wishlist
Target Milestone: ---
Assignee: George Staikos
URL:
Keywords:
: 106251 128224 149487 157026 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-09-28 23:37 UTC by Tobias Niwi
Modified: 2009-07-17 21:31 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Proposed patch (4.51 KB, patch)
2006-10-18 11:47 UTC, Apollon Oikonomopoulos
Details
Proposed patch for mute/unmute events (3.28 KB, patch)
2006-12-02 09:54 UTC, William Poetra Yoga Hadisoeseno
Details
Updated patch for mute/unmute events (3.70 KB, patch)
2006-12-02 15:06 UTC, Apollon Oikonomopoulos
Details
Patch for kmix-3.5.8 to add new dcop call (masterDeviceIndex) (1.57 KB, patch)
2007-11-04 05:44 UTC, Kelvie Wong
Details
And the fix for kmilo, that uses that new DCop call. (2.72 KB, patch)
2007-11-04 06:11 UTC, Kelvie Wong
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Niwi 2006-09-28 23:37:36 UTC
Version:            (using KDE KDE 3.5.4)
Installed from:    Ubuntu Packages

I would like to use KMilo to change my PCM settings instead of the MASTER settings. This would have the advantage that KMilo would not only change the volume for the speakers but also for the headphones.

The KMix panel applet has a setting to decide which channel should be used as main channel. Maybe KMilo could use this setting as well.
Comment 1 Apollon Oikonomopoulos 2006-10-18 11:47:29 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?).
Comment 2 Luke Monahan 2006-10-24 06:04:44 UTC
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.
Comment 3 William Poetra Yoga Hadisoeseno 2006-12-02 09:54:36 UTC
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
Comment 4 Apollon Oikonomopoulos 2006-12-02 15:06:23 UTC
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
Comment 5 Apollon Oikonomopoulos 2006-12-02 15:08:34 UTC
Seems like my patch wasn't needed, William had already done the same work :-)
Comment 6 Christian Esken 2006-12-05 00:34:17 UTC
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.
Comment 7 Christian Esken 2006-12-05 00:53:36 UTC
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
Comment 8 Christian Esken 2006-12-05 20:25:51 UTC
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);
 		}
 	}
 
Comment 9 Apollon Oikonomopoulos 2007-06-16 00:23:39 UTC
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.
Comment 10 Olivier Vitrat 2007-06-20 15:54:28 UTC
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.
Comment 11 Gregorio Guidi 2007-08-14 23:52:48 UTC
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).
Comment 12 Kelvie Wong 2007-11-04 03:24:15 UTC
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.
Comment 13 Kelvie Wong 2007-11-04 05:44:55 UTC
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.
Comment 14 Kelvie Wong 2007-11-04 06:11:19 UTC
Created attachment 21996 [details]
And the fix for kmilo, that uses that new DCop call.

And the kmilo part of the patch.
Comment 15 Gregorio Guidi 2007-11-06 13:19:08 UTC
I tried the patches and I confirm that they solve the issue.
Comment 16 Kelvie Wong 2007-11-06 20:48:36 UTC
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
Comment 17 Christian Esken 2007-12-29 20:46:45 UTC
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
Comment 18 Christian Esken 2007-12-29 20:51:39 UTC
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
Comment 19 David Jarvie 2008-01-02 18:53:22 UTC
*** Bug 149487 has been marked as a duplicate of this bug. ***
Comment 20 David Jarvie 2008-01-02 18:55:29 UTC
*** Bug 128224 has been marked as a duplicate of this bug. ***
Comment 21 David Jarvie 2008-02-05 19:25:52 UTC
*** Bug 157026 has been marked as a duplicate of this bug. ***
Comment 22 Christian Esken 2009-07-17 21:31:39 UTC
*** Bug 106251 has been marked as a duplicate of this bug. ***