Bug 134604 - no dcop to mute master channel
Summary: no dcop to mute master channel
Status: RESOLVED FIXED
Alias: None
Product: kmix
Classification: Applications
Component: general (show other bugs)
Version: 2.6
Platform: Mandriva RPMs Linux
: NOR wishlist with 40 votes (vote)
Target Milestone: ---
Assignee: Christian Esken
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-24 21:15 UTC by Debajyoti Bera
Modified: 2006-12-05 20:25 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Proposed patch (2.60 KB, patch)
2006-10-18 12:38 UTC, Apollon Oikonomopoulos
Details
New proposed patch (3.11 KB, patch)
2006-12-02 09:50 UTC, William Poetra Yoga Hadisoeseno
Details
New patch using Wiliiam's method names (2.60 KB, patch)
2006-12-02 15:01 UTC, Apollon Oikonomopoulos
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Debajyoti Bera 2006-09-24 21:15:01 UTC
Version:           2.6 (using KDE KDE 3.5.4)
Installed from:    Mandriva RPMs
OS:                Linux

kmix has no dcop to mute the master channel. As a result, using dcop to mute (e.g. as done by i8k) mutes the first or some fixed channel but doesnt put a cross mark on the kmix tray icon.

Either provide a dcop call to mute master channel or cross the tray icon whenever any output channel is muted (I am not sure that is the right thing to do).
Comment 1 Apollon Oikonomopoulos 2006-10-18 12:38:25 UTC
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.
Comment 2 William Poetra Yoga Hadisoeseno 2006-12-02 09:21:31 UTC
Nice patch. But I think the names should be "masterMute", "toggleMasterMute" and "setMasterMute", as in "volume" -> "masterVolume", etc.
Comment 3 William Poetra Yoga Hadisoeseno 2006-12-02 09:50:21 UTC
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.
Comment 4 Christian Esken 2006-12-02 10:31:52 UTC
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
Comment 5 Apollon Oikonomopoulos 2006-12-02 15:01:15 UTC
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!
Comment 6 William Poetra Yoga Hadisoeseno 2006-12-02 15:59:15 UTC
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.
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:52 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);
 		}
 	}