Skip to content

Commit

Permalink
Fixes most of stuff found in Wallacoloo's code review for LMMS#1600
Browse files Browse the repository at this point in the history
Removal of a superfluous include in AudioAlsaSetupWidget.cpp

Removal of the function "bool hasCapabilities(char *device_name)" which
was not used anyway. It implemented a test for ALSA device capabilities
needed by LMMS (SND_PCM_ACCESS_RW_INTERLEAVED, SND_PCM_FORMAT_S16_LE,
etc.).

Corrected header name in AudioAlsaSetupWidget.h.

Created an implementation file for AudioDeviceSetupWidget to make more
clear that it's part of the GUI.

Fix build for builds that use Port Audio. The setup widget of
AudioPortAudio.h still inherited from AudioDevice::setupWidget instead
of the new AudioDeviceSetupWidget.
  • Loading branch information
michaelgregorius authored and ThomasJClark committed Sep 12, 2015
1 parent 31d35f4 commit 4a2536a
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 79 deletions.
4 changes: 2 additions & 2 deletions include/AudioAlsaSetupWidget.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* AudioDeviceSetupWidget.h - Implements a setup widget for ALSA-PCM-output
* AudioAlsaSetupWidget.h - Implements a setup widget for ALSA-PCM-output
*
* Copyright (c) 2004-2015 Tobias Doerffel <tobydox/at/users.sourceforge.net>
*
Expand Down Expand Up @@ -57,7 +57,7 @@ public slots:

int m_selectedDevice;
AudioAlsa::DeviceInfoCollection m_deviceInfos;
} ;
};

#endif

Expand Down
17 changes: 3 additions & 14 deletions include/AudioDeviceSetupWidget.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,13 @@
class AudioDeviceSetupWidget : public TabWidget
{
public:
AudioDeviceSetupWidget( const QString & _caption, QWidget * _parent ) :
TabWidget( TabWidget::tr( "Settings for %1" ).arg(
TabWidget::tr( _caption.toLatin1() ) ).
toUpper(), _parent )
{
}
AudioDeviceSetupWidget( const QString & _caption, QWidget * _parent );

virtual ~AudioDeviceSetupWidget()
{
}
virtual ~AudioDeviceSetupWidget();

virtual void saveSettings() = 0;

virtual void show()
{
parentWidget()->show();
QWidget::show();
}
virtual void show();
};


Expand Down
3 changes: 2 additions & 1 deletion include/AudioPortAudio.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public slots:
#endif

#include "AudioDevice.h"
#include "AudioDeviceSetupWidget.h"

#if defined paNeverDropInput || defined paNonInterleaved
# define PORTAUDIO_V19
Expand Down Expand Up @@ -81,7 +82,7 @@ class AudioPortAudio : public AudioDevice
unsigned long _framesPerBuffer );


class setupWidget : public AudioDevice::setupWidget
class setupWidget : public AudioDeviceSetupWidget
{
public:
setupWidget( QWidget * _parent );
Expand Down
59 changes: 0 additions & 59 deletions src/core/audio/AudioAlsa.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,63 +142,6 @@ QString AudioAlsa::probeDevice()



/**
* @brief Checks whether the ALSA device with the given name has the needed
* capabilities for LMMS.
* @param deviceName Name of the device that is checked.
* @return If the device is usable for LMMS <tt>true</tt> is returned.
*/
bool hasCapabilities(char *device_name)
{
snd_pcm_t *pcm; // PCM handle
snd_pcm_hw_params_t *hw_params;
int err;

// Implicit check for SND_PCM_STREAM_PLAYBACK
err = snd_pcm_open(&pcm, device_name, SND_PCM_STREAM_PLAYBACK, 0);
if (err < 0)
{
std::cerr << "Cannot open device '" << device_name << "': " << snd_strerror(err) << std::endl;
return false;
}

snd_pcm_hw_params_alloca(&hw_params);
err = snd_pcm_hw_params_any(pcm, hw_params);
if (err < 0)
{
std::cerr << "Cannot get hardware parameters: " << snd_strerror(err) << std::endl;
snd_pcm_close(pcm);
return false;
}

// Checks for SND_PCM_ACCESS_RW_INTERLEAVED
err = snd_pcm_hw_params_test_access(pcm, hw_params, SND_PCM_ACCESS_RW_INTERLEAVED);
if (err < 0)
{
std::cerr << "Interleaved access not possible for '" << device_name << "': " << snd_strerror(err) << std::endl;
snd_pcm_close(pcm);
return false;
}

// Check for SND_PCM_FORMAT_S16_LE or SND_PCM_FORMAT_S16_BE
bool validFormatFound = false;

validFormatFound |= !snd_pcm_hw_params_test_format(pcm, hw_params, SND_PCM_FORMAT_S16_LE);
validFormatFound |= !snd_pcm_hw_params_test_format(pcm, hw_params, SND_PCM_FORMAT_S16_BE);

if (!validFormatFound)
{
std::cerr << "Device " << device_name << " does not not support SND_PCM_FORMAT_S16_LE or SND_PCM_FORMAT_S16_BE!" << std::endl;
snd_pcm_close(pcm);
return false;
}

snd_pcm_close(pcm);

return true;
}


/**
* @brief Creates a list of all available devices.
*
Expand Down Expand Up @@ -232,8 +175,6 @@ AudioAlsa::DeviceInfoCollection AudioAlsa::getAvailableDevices()
char *name = snd_device_name_get_hint(*n, "NAME");
char *description = snd_device_name_get_hint(*n, "DESC");

// We could call hasCapabilities(name) here but this gives strange
// results
if (name != 0 && description != 0)
{
deviceInfos.push_back(DeviceInfo(QString(name), QString(description)));
Expand Down
2 changes: 1 addition & 1 deletion src/core/audio/AudioPortAudio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ void AudioPortAudioSetupUtil::updateChannels()


AudioPortAudio::setupWidget::setupWidget( QWidget * _parent ) :
AudioDevice::setupWidget( AudioPortAudio::name(), _parent )
AudioDeviceSetupWidget( AudioPortAudio::name(), _parent )
{
m_backend = new ComboBox( this, "BACKEND" );
m_backend->setGeometry( 64, 15, 260, 20 );
Expand Down
2 changes: 0 additions & 2 deletions src/gui/AudioAlsaSetupWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@

#ifdef LMMS_HAVE_ALSA

#include "AudioAlsa.h"

#include "ConfigManager.h"
#include "LcdSpinBox.h"
#include "gui_templates.h"
Expand Down
41 changes: 41 additions & 0 deletions src/gui/AudioDeviceSetupWidget.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* AudioDeviceSetupWidget.cpp - Base class for audio device setup widgets
*
* Copyright (c) 2004-2015 Tobias Doerffel <tobydox/at/users.sourceforge.net>
*
* This file is part of LMMS - http://lmms.io
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public
* License as published by the Free Software Foundation; either
* version 2 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* General Public License for more details.
*
* You should have received a copy of the GNU General Public
* License along with this program (see COPYING); if not, write to the
* Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301 USA.
*
*/

#include "AudioDeviceSetupWidget.h"

AudioDeviceSetupWidget::AudioDeviceSetupWidget( const QString & _caption, QWidget * _parent ) :
TabWidget( TabWidget::tr( "Settings for %1" ).arg(TabWidget::tr( _caption.toLatin1() ) ).toUpper(),
_parent )
{
}

AudioDeviceSetupWidget::~AudioDeviceSetupWidget()
{
}

void AudioDeviceSetupWidget::show()
{
parentWidget()->show();
QWidget::show();
}
1 change: 1 addition & 0 deletions src/gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ SET(LMMS_SRCS
gui/AboutDialog.cpp
gui/ActionGroup.cpp
gui/AudioAlsaSetupWidget.cpp
gui/AudioDeviceSetupWidget.cpp
gui/AutomatableModelView.cpp
gui/AutomationPatternView.cpp
gui/ControllerConnectionDialog.cpp
Expand Down

0 comments on commit 4a2536a

Please sign in to comment.