Bug 134604

Created attachment 18170 [details]
Proposed patch

Providing the "muteMaster", "toggleMuteMaster" and "setMuteMaster" DCOP calls
would also help making KMilo's mute function master-channel aware (see also
http://bugs.kde.org/show_bug.cgi?id=134820 ). The attached patch adds these
DCOP calls to kmix.

Nice patch. But I think the names should be "masterMute", "toggleMasterMute" and "setMasterMute", as in "volume" -> "masterVolume", etc.

Created attachment 18750 [details]
New proposed patch

By the way, how about this attached patch?
That way, we can avoid calling mixDeviceByType() when we already know the
device number.

Apollon, William, thanks for your work on this. This is really needed. Actually it might fix a whole bunch of issues.
My plan is to have this in KDE3.5.6. I would like to use Apollon's patch, but William's method names.
Apollon, could you please redo it like that and upload it here? Patch will then go in the KDE repository immediately.

Christian

Created attachment 18753 [details]
New patch using Wiliiam's method names

Ok, here is the patch with William's method names. Indeed, they do make more
sense this way and I shall also update the patch for kmilo accordingly. Thanks
for taking care of this feature!

Thanks Apollon, Christian.

I've just found out that my patch breaks :( (because I forgot to replace deviceidx in setMute() )
Apollon's patch is better, so I'm obsoleting my patch.

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);
 		}
 	}