From kde-commits Wed Mar 21 23:09:15 2012 From: Christian Esken Date: Wed, 21 Mar 2012 23:09:15 +0000 To: kde-commits Subject: KDE/kdemultimedia/kmix Message-Id: <20120321230915.F2C68AC899 () svn ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-commits&m=133237144028626 SVN commit 1286624 by esken: More MixDevice reference cleanups, and fix some minor memory leaks. CCBUGS: 290742 M +2 -0 backends/mixer_alsa.h M +10 -2 backends/mixer_alsa9.cpp M +2 -0 backends/mixer_backend.cpp M +1 -2 backends/mixer_mpris2.cpp M +7 -4 core/mixdevice.cpp M +3 -0 core/mixdevice.h M +1 -1 core/mixer.cpp M +1 -1 core/mixer.h M +4 -0 core/mixset.cpp M +2 -0 core/mixset.h M +6 -20 gui/viewbase.cpp M +3 -2 gui/viewbase.h M +2 -2 gui/viewdockareapopup.cpp M +1 -1 gui/viewdockareapopup.h M +10 -8 gui/viewsliders.cpp --- trunk/KDE/kdemultimedia/kmix/backends/mixer_alsa.h #1286623:1286624 @@ -78,6 +78,8 @@ bool _initialUpdate; snd_mixer_t *_handle; + snd_ctl_t *ctl_handle; + QString devName; struct pollfd *m_fds; QList m_sns; --- trunk/KDE/kdemultimedia/kmix/backends/mixer_alsa9.cpp #1286623:1286624 @@ -65,11 +65,13 @@ m_fds = 0; // m_sns = 0; _handle = 0; + ctl_handle = 0; _initialUpdate = true; } Mixer_ALSA::~Mixer_ALSA() { + qDebug() << "Running Mixer_ALSA destructor"; close(); } @@ -215,7 +217,7 @@ if ( !enumList.isEmpty() ) mdNew->addEnums(enumList); shared_ptr md = mdNew->addToPool(); - m_mixDevices.append( md->addToPool() ); + m_mixDevices.append( md ); qDeleteAll(enumList); // clear temporary list @@ -275,7 +277,6 @@ int Mixer_ALSA::openAlsaDevice(const QString& devName) { int err; - snd_ctl_t *ctl_handle; QString probeMessage; probeMessage += "Trying ALSA Device '" + devName + "': "; @@ -483,6 +484,13 @@ { int ret=0; m_isOpen = false; + + if ( ctl_handle != 0) + { + //snd_ctl_close( ctl_handle ); + ctl_handle = 0; + } + if ( _handle != 0 ) { //kDebug() << "IN Mixer_ALSA::close()"; --- trunk/KDE/kdemultimedia/kmix/backends/mixer_backend.cpp #1286623:1286624 @@ -40,10 +40,12 @@ // like ::select() is possible (as in ALSA). _pollingTimer = new QTimer(); // will be started on open() and stopped on close() connect( _pollingTimer, SIGNAL(timeout()), this, SLOT(readSetFromHW()), Qt::QueuedConnection); + } Mixer_Backend::~Mixer_Backend() { + qDebug() << "Running Mixer_Backend destructor"; delete _pollingTimer; //qDeleteAll(m_mixDevices); // TODO cesken Leak check the removed qDeleteAll() m_mixDevices.clear(); --- trunk/KDE/kdemultimedia/kmix/backends/mixer_mpris2.cpp #1286623:1286624 @@ -285,8 +285,7 @@ mdNew->setApplicationStream(true); mdNew->addPlaybackVolume(*vol); - shared_ptr md = mdNew->addToPool(); - m_mixDevices.append( md->addToPool() ); + m_mixDevices.append( mdNew->addToPool() ); // conn.connect("", QString("/org/mpris/MediaPlayer2"), "org.freedesktop.DBus.Properties", "PropertiesChanged", mad, SLOT(volumeChangedIncoming(QString,QList)) ); conn.connect(busDestination, QString("/org/mpris/MediaPlayer2"), "org.freedesktop.DBus.Properties", "PropertiesChanged", mad, SLOT(volumeChangedIncoming(QString,QVariantMap,QStringList)) ); --- trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp #1286623:1286624 @@ -28,9 +28,9 @@ #include "core/ControlPool.h" #include "core/mixer.h" +#include "dbus/dbuscontrolwrapper.h" #include "gui/guiprofile.h" #include "core/volume.h" -#include "dbus/dbuscontrolwrapper.h" static const QString channelTypeToIconName( MixDevice::ChannelType type ) { @@ -115,6 +115,7 @@ { _artificial = false; _applicationStream = false; + _dbusControlWrapper = 0; // will be set in addToPool() _mixer = mixer; _id = id; mediaPlayControl = false; @@ -130,7 +131,7 @@ _iconName = iconName; _moveDestinationMixSet = moveDestinationMixSet; if ( _id.contains(' ') ) { - // The key is used in the config file. It MUST NOT contain spaces + // The key is used in the config file. IdbusControlWrappert MUST NOT contain spaces kError(67100) << "MixDevice::setId(\"" << id << "\") . Invalid key - it must not contain spaces" << endl; _id.replace(' ', '_'); } @@ -142,8 +143,9 @@ const QString& fullyQualifiedId = getFullyQualifiedId(); kDebug() << "MixDevice::init() id=" << fullyQualifiedId; - shared_ptr thisSharedPtr = ControlPool::instance()->add(fullyQualifiedId, this); - new DBusControlWrapper( thisSharedPtr, dbusPath() ); + shared_ptr thisSharedPtr(this); + //shared_ptr thisSharedPtr = ControlPool::instance()->add(fullyQualifiedId, this); + _dbusControlWrapper = new DBusControlWrapper( thisSharedPtr, dbusPath() ); return thisSharedPtr; } @@ -182,6 +184,7 @@ MixDevice::~MixDevice() { _enumValues.clear(); // The QString's inside will be auto-deleted, as they get unref'ed + delete _dbusControlWrapper; } Volume& MixDevice::playbackVolume() --- trunk/KDE/kdemultimedia/kmix/core/mixdevice.h #1286623:1286624 @@ -31,6 +31,7 @@ class MixSet; class ProfControl; #include "core/volume.h" +class DBusControlWrapper; // KDE #include @@ -227,6 +228,8 @@ int _enumCurrentId; QList _enumValues; // A MixDevice, that is an ENUM, has these _enumValues + DBusControlWrapper *_dbusControlWrapper; + // A virtual control. It will not be saved/restored and/or doesn't get shortcuts // Actually we discriminate those "virtual" controls in artificial controls and dynamic controls: // Type Shortcut Restore --- trunk/KDE/kdemultimedia/kmix/core/mixer.cpp #1286623:1286624 @@ -240,7 +240,7 @@ return md; } -MixSet Mixer::getMixSet() +MixSet& Mixer::getMixSet() { return _mixerBackend->m_mixDevices; } --- trunk/KDE/kdemultimedia/kmix/core/mixer.h #1286623:1286624 @@ -161,7 +161,7 @@ void setLocalMasterMD(QString&); /// get the actual MixSet - MixSet getMixSet(); + MixSet& getMixSet(); static float VOLUME_STEP_DIVISOR; // The divisor for defining volume control steps (for mouse-wheel, DBUS and Normal step for Sliders ) static float VOLUME_PAGESTEP_DIVISOR; // The divisor for defining volume control steps (page-step for sliders) --- trunk/KDE/kdemultimedia/kmix/core/mixset.cpp #1286623:1286624 @@ -33,6 +33,10 @@ #include +MixSet::~MixSet() +{ + clear(); +} void MixSet::read( KConfig *config, const QString& grp ) { --- trunk/KDE/kdemultimedia/kmix/core/mixset.h #1286623:1286624 @@ -28,6 +28,8 @@ class MixSet : public QList > { public: + ~MixSet(); + void read( KConfig *config, const QString& grp ); void write( KConfig *config, const QString& grp ); --- trunk/KDE/kdemultimedia/kmix/gui/viewbase.cpp #1286623:1286624 @@ -49,7 +49,6 @@ setObjectName(id); m_viewId = id; _mixer = mixer; - _mixSet = new MixSet(); // This must be populated now otherwise bad things happen (circular dependancies etc // This is due to the fact that setMixSet() calls isDynamic() which in turn needs a populated @@ -89,7 +88,6 @@ } ViewBase::~ViewBase() { - delete _mixSet; // Hint: The GUI profile will not be removed, as it is pooled and might be applied to a new View. } @@ -103,7 +101,7 @@ bool ViewBase::isValid() const { - return ( _mixSet->count() > 0 || isDynamic() ); + return ( !_mixSet.isEmpty() || isDynamic() ); } void ViewBase::setIcons (bool on) { KMixToolBox::setIcons (_mdws, on ); } @@ -119,25 +117,13 @@ */ void ViewBase::createDeviceWidgets() { - // create devices - foreach ( shared_ptr md, *_mixSet ) + foreach ( shared_ptr md, _mixSet ) { QWidget* mdw = add(md); // a) Let the View implementation do its work _mdws.append(mdw); // b) Add it to the local list } // allow view to "polish" itself constructionFinished(); - -// Moved the following up one Level to KMixerWidget -// kDebug() << "CONNECT ViewBase count " << _mixers.size(); -// foreach ( Mixer* mixer, _mixers ) -// { -// kDebug(67100) << "CONNECT ViewBase controlschanged" << mixer->id(); -// connect ( mixer, SIGNAL(controlChanged()), this, SLOT(refreshVolumeLevels()) ); -// connect ( mixer, SIGNAL(controlsReconfigured(QString)), this, SLOT(controlsReconfigured(QString)) ); -// } - - } /** @@ -214,9 +200,9 @@ if (isRelevantMixer) { - kDebug(67100) << "ViewBase::controlsReconfigured() " << mixer_ID << " is being redrawn (mixset contains: " << _mixSet->count() << ")"; + kDebug(67100) << "ViewBase::controlsReconfigured() " << mixer_ID << " is being redrawn (mixset contains: " << _mixSet.count() << ")"; setMixSet(); - kDebug(67100) << "ViewBase::controlsReconfigured() " << mixer_ID << ": Recreating widgets (mixset contains: " << _mixSet->count() << ")"; + kDebug(67100) << "ViewBase::controlsReconfigured() " << mixer_ID << ": Recreating widgets (mixset contains: " << _mixSet.count() << ")"; createDeviceWidgets(); } } @@ -249,13 +235,13 @@ while (!_mdws.isEmpty()) delete _mdws.takeFirst(); - _mixSet->clear(); // Clean up our _mixSet so we can reapply our GUIProfile + _mixSet.clear(); // Clean up our _mixSet so we can reapply our GUIProfile } _setMixSet(); _mixers.clear(); _mixers.insert(_mixer); - foreach ( shared_ptr md, *_mixSet ) + foreach ( shared_ptr md, _mixSet ) { // kDebug() << "VVV Add to " << md->mixer()->id(); // MixDeviceWidget* mdw = qobject_cast(qw); --- trunk/KDE/kdemultimedia/kmix/gui/viewbase.h #1286623:1286624 @@ -29,13 +29,14 @@ // KDE #include class KMenu; -class MixSet; + class Mixer; class MixDevice; // KMix class GUIProfile; #include "core/mixdevice.h" +#include "core/mixset.h" /** * The ViewBase is a virtual base class, to be used for subclassing the real Mixer Views. @@ -130,7 +131,7 @@ protected: - MixSet *_mixSet; + MixSet _mixSet; Mixer *_mixer; QSet _mixers; // this might deprecate _mixer in the future. Currently only in use by ViewDockAreaPopup KMenu *_popMenu; --- trunk/KDE/kdemultimedia/kmix/gui/viewdockareapopup.cpp #1286623:1286624 @@ -96,7 +96,7 @@ } } if ( dockMD != 0 ) { - _mixSet->append(dockMD); + _mixSet.append(dockMD); } foreach ( Mixer* mixer2 , Mixer::mixers() ) @@ -105,7 +105,7 @@ { if (md->isApplicationStream()) { - _mixSet->append(md); + _mixSet.append(md); kDebug(67100) << "Add to tray popup: " << md->id(); } } --- trunk/KDE/kdemultimedia/kmix/gui/viewdockareapopup.h #1286623:1286624 @@ -37,7 +37,7 @@ ViewDockAreaPopup(QWidget* parent, const char* name, Mixer* mixer, ViewBase::ViewFlags vflags, GUIProfile *guiprof, KMixWindow *dockW); virtual ~ViewDockAreaPopup(); - virtual QWidget* add(shared_ptr mdw); + virtual QWidget* add(shared_ptr md); virtual void constructionFinished(); virtual void refreshVolumeLevels(); virtual void showContextMenu(); --- trunk/KDE/kdemultimedia/kmix/gui/viewsliders.cpp #1286623:1286624 @@ -32,14 +32,15 @@ #endif #endif +#include "gui/viewsliders.h" + // KMix -#include "viewsliders.h" -#include "gui/guiprofile.h" -#include "mdwenum.h" -#include "mdwslider.h" #include "core/mixdevicecomposite.h" #include "core/mixer.h" -#include "verticaltext.h" +#include "gui/guiprofile.h" +#include "gui/mdwenum.h" +#include "gui/mdwslider.h" +#include "gui/verticaltext.h" // KDE #include @@ -116,6 +117,7 @@ QWidget* ViewSliders::add(shared_ptr md) + { MixDeviceWidget *mdw; Qt::Orientation orientation = (_vflags & ViewBase::Vertical) ? Qt::Horizontal : Qt::Vertical; @@ -209,7 +211,7 @@ if ( md->id().contains(idRegexp) ) { // Match found (by name) - if ( _mixSet->contains( md ) ) continue; // dup check + if ( _mixSet.contains( md ) ) continue; // dup check // Now check whether subcontrols match bool subcontrolPlaybackWanted = (control->useSubcontrolPlayback() && ( md->playbackVolume().hasVolume() || md->playbackVolume().hasSwitch()) ); @@ -230,7 +232,7 @@ else if ( control->getSwitchtype() == "Off" ) md->playbackVolume().setSwitchType(Volume::OffSwitch); } - _mixSet->append(md); + _mixSet.append(md); #ifdef TEST_MIXDEVICE_COMPOSITE if ( md->id() == "Front:0" || md->id() == "Surround:0") { mds.append(md); } // For temporary test @@ -247,7 +249,7 @@ } } // iteration over all controls from the Profile - emptyStreamHint->setVisible( _mixSet->isEmpty() && isDynamic() ); // show a hint why a tab is empty (dynamic controls!!!) + emptyStreamHint->setVisible( _mixSet.isEmpty() && isDynamic() ); // show a hint why a tab is empty (dynamic controls!!!) // visibleControls() == 0 could be used for the !isDynamic() case