From 2e25be8f717f73281201e3d466b47865238b8c4d Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Wed, 21 Mar 2018 18:51:18 +0200 Subject: [PATCH 1/3] Eliminate redundant casts to/from void* in Ffmpeg player --- ffmpegaudio.cc | 4 ++-- ffmpegaudio.hh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ffmpegaudio.cc b/ffmpegaudio.cc index 04c35d85..c1d9aaac 100644 --- a/ffmpegaudio.cc +++ b/ffmpegaudio.cc @@ -61,10 +61,10 @@ AudioPlayer::~AudioPlayer() ao_shutdown(); } -void AudioPlayer::playMemory( const void * ptr, int size ) +void AudioPlayer::playMemory( const char * ptr, int size ) { emit cancelPlaying( false ); - QByteArray audioData( ( char * )ptr, size ); + QByteArray audioData( ptr, size ); DecoderThread * thread = new DecoderThread( audioData, this ); connect( thread, SIGNAL( error( QString ) ), this, SIGNAL( error( QString ) ) ); diff --git a/ffmpegaudio.hh b/ffmpegaudio.hh index 866edf8d..dd10cd4b 100644 --- a/ffmpegaudio.hh +++ b/ffmpegaudio.hh @@ -18,7 +18,7 @@ class AudioPlayer : public QObject public: static AudioPlayer & instance(); - void playMemory( const void * ptr, int size ); + void playMemory( const char * ptr, int size ); void stop(); signals: From e5045860ef7102a664d314bb87a7c2e9d6d47a52 Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Wed, 21 Mar 2018 19:49:34 +0200 Subject: [PATCH 2/3] Make adding new audio player implementations easy * add a new interface class AudioPlayerInterface; * inherit a new proxy class Ffmpeg::AudioPlayer from it; * partially switch ArlticleView to using the interface; * expose MainWindow's AudioPlayerInterface instance to all ArticleView instances; * add a new AudioPlayerFactory class responsible for creating instances of concrete classes derived from AudioPlayerInterface depending on relevant Config::Preferences values; * increase minimum supported Qt version from 4.5 to 4.6 in README in order to use QScopedPointer introduced in Qt 4.6. --- README.md | 2 +- articleview.cc | 22 +++++++++++----------- articleview.hh | 3 +++ audioplayerfactory.cc | 32 ++++++++++++++++++++++++++++++++ audioplayerfactory.hh | 24 ++++++++++++++++++++++++ audioplayerinterface.hh | 24 ++++++++++++++++++++++++ ffmpegaudio.cc | 12 ++++++------ ffmpegaudio.hh | 10 ++++------ ffmpegaudioplayer.hh | 40 ++++++++++++++++++++++++++++++++++++++++ goldendict.pro | 4 ++++ mainwindow.cc | 11 +++++++---- mainwindow.hh | 2 ++ scanpopup.cc | 5 +++-- scanpopup.hh | 1 + 14 files changed, 162 insertions(+), 30 deletions(-) create mode 100644 audioplayerfactory.cc create mode 100644 audioplayerfactory.hh create mode 100644 audioplayerinterface.hh create mode 100644 ffmpegaudioplayer.hh diff --git a/README.md b/README.md index 47ac2644..a27175f2 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ This code has been run and tested on Windows XP/Vista/7, Ubuntu Linux, Mac OS X. ### External Deps * Make, GCC, Git -* Qt framework. Minimum required version is 4.6 for Windows, 4.5 for all other platforms. But Qt 4.7 or 4.8 is recommended. +* Qt framework. Minimum required version is 4.6. But Qt 4.7 or 4.8 is recommended. * If you want to use Qt 5.x then use branch qt4x5 * Qt Creator IDE is recommended for development * Various libraries on Linux (png, zlib, etc) diff --git a/articleview.cc b/articleview.cc index a3a6c808..aa7d006d 100644 --- a/articleview.cc +++ b/articleview.cc @@ -17,7 +17,6 @@ #include "webmultimediadownload.hh" #include "programs.hh" #include "gddebug.hh" -#include "ffmpegaudio.hh" #include #include #include "gestures.hh" @@ -110,6 +109,7 @@ static QVariant evaluateJavaScriptVariableSafe( QWebFrame * frame, const QString } ArticleView::ArticleView( QWidget * parent, ArticleNetworkAccessManager & nm, + AudioPlayerPtr const & audioPlayer_, std::vector< sptr< Dictionary::Class > > const & allDictionaries_, Instances::Groups const & groups_, bool popupView_, Config::Class const & cfg_, @@ -118,6 +118,7 @@ ArticleView::ArticleView( QWidget * parent, ArticleNetworkAccessManager & nm, GroupComboBox const * groupComboBox_ ): QFrame( parent ), articleNetMgr( nm ), + audioPlayer( audioPlayer_ ), allDictionaries( allDictionaries_ ), groups( groups_ ), popupView( popupView_ ), @@ -270,8 +271,8 @@ ArticleView::~ArticleView() cleanupTemp(); #ifndef DISABLE_INTERNAL_PLAYER - if ( cfg.preferences.useInternalPlayer ) - Ffmpeg::AudioPlayer::instance().stop(); + if ( audioPlayer ) + audioPlayer->stop(); #endif #if QT_VERSION >= QT_VERSION_CHECK(4, 6, 0) @@ -287,8 +288,8 @@ void ArticleView::showDefinition( QString const & word, unsigned group, #ifndef DISABLE_INTERNAL_PLAYER // first, let's stop the player - if ( cfg.preferences.useInternalPlayer ) - Ffmpeg::AudioPlayer::instance().stop(); + if ( audioPlayer ) + audioPlayer->stop(); #endif QUrl req; @@ -355,8 +356,8 @@ void ArticleView::showDefinition( QString const & word, QStringList const & dict #ifndef DISABLE_INTERNAL_PLAYER // first, let's stop the player - if ( cfg.preferences.useInternalPlayer ) - Ffmpeg::AudioPlayer::instance().stop(); + if ( audioPlayer ) + audioPlayer->stop(); #endif QUrl req; @@ -1910,11 +1911,10 @@ void ArticleView::resourceDownloadFinished() { // Audio data #ifndef DISABLE_INTERNAL_PLAYER - if ( cfg.preferences.useInternalPlayer ) + if ( audioPlayer ) { - Ffmpeg::AudioPlayer & player = Ffmpeg::AudioPlayer::instance(); - connect( &player, SIGNAL( error( QString ) ), this, SLOT( audioPlayerError( QString ) ), Qt::UniqueConnection ); - player.playMemory( data.data(), data.size() ); + connect( audioPlayer.data(), SIGNAL( error( QString ) ), this, SLOT( audioPlayerError( QString ) ), Qt::UniqueConnection ); + audioPlayer->play( data.data(), data.size() ); } else #endif diff --git a/articleview.hh b/articleview.hh index 3f61ae3d..36cebbcf 100644 --- a/articleview.hh +++ b/articleview.hh @@ -10,6 +10,7 @@ #include #include #include "article_netmgr.hh" +#include "audioplayerinterface.hh" #include "instances.hh" #include "groupcombobox.hh" #include "ui_articleview.h" @@ -23,6 +24,7 @@ class ArticleView: public QFrame Q_OBJECT ArticleNetworkAccessManager & articleNetMgr; + AudioPlayerPtr const & audioPlayer; std::vector< sptr< Dictionary::Class > > const & allDictionaries; Instances::Groups const & groups; bool popupView; @@ -68,6 +70,7 @@ public: /// The groups aren't copied -- rather than that, the reference is kept ArticleView( QWidget * parent, ArticleNetworkAccessManager &, + AudioPlayerPtr const &, std::vector< sptr< Dictionary::Class > > const & allDictionaries, Instances::Groups const &, bool popupView, diff --git a/audioplayerfactory.cc b/audioplayerfactory.cc new file mode 100644 index 00000000..cf1d5e80 --- /dev/null +++ b/audioplayerfactory.cc @@ -0,0 +1,32 @@ +/* This file is (c) 2018 Igor Kushnir + * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ + +#include "audioplayerfactory.hh" +#include "ffmpegaudioplayer.hh" + +AudioPlayerFactory::AudioPlayerFactory( Config::Preferences const & p ) : + useInternalPlayer( p.useInternalPlayer ) +{ + reset(); +} + +void AudioPlayerFactory::setPreferences( Config::Preferences const & p ) +{ + if( p.useInternalPlayer != useInternalPlayer ) + { + useInternalPlayer = p.useInternalPlayer; + reset(); + } +} + +void AudioPlayerFactory::reset() +{ +#ifndef DISABLE_INTERNAL_PLAYER + if( useInternalPlayer ) + playerPtr.reset( new Ffmpeg::AudioPlayer ); + else +#endif + { + playerPtr.reset(); + } +} diff --git a/audioplayerfactory.hh b/audioplayerfactory.hh new file mode 100644 index 00000000..482f385e --- /dev/null +++ b/audioplayerfactory.hh @@ -0,0 +1,24 @@ +/* This file is (c) 2018 Igor Kushnir + * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ + +#ifndef AUDIOPLAYERFACTORY_HH_INCLUDED +#define AUDIOPLAYERFACTORY_HH_INCLUDED + +#include "audioplayerinterface.hh" +#include "config.hh" + +class AudioPlayerFactory +{ + Q_DISABLE_COPY( AudioPlayerFactory ) +public: + explicit AudioPlayerFactory( Config::Preferences const & ); + void setPreferences( Config::Preferences const & ); + AudioPlayerPtr const & player() const { return playerPtr; } + +private: + void reset(); + bool useInternalPlayer; + AudioPlayerPtr playerPtr; +}; + +#endif // AUDIOPLAYERFACTORY_HH_INCLUDED diff --git a/audioplayerinterface.hh b/audioplayerinterface.hh new file mode 100644 index 00000000..66b055bf --- /dev/null +++ b/audioplayerinterface.hh @@ -0,0 +1,24 @@ +/* This file is (c) 2018 Igor Kushnir + * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ + +#ifndef AUDIOPLAYERINTERFACE_HH_INCLUDED +#define AUDIOPLAYERINTERFACE_HH_INCLUDED + +#include +#include +#include + +class AudioPlayerInterface : public QObject +{ + Q_OBJECT +public: + virtual void play( const char * data, int size ) = 0; + virtual void stop() = 0; + +signals: + void error( QString message ); +}; + +typedef QScopedPointer< AudioPlayerInterface > AudioPlayerPtr; + +#endif // AUDIOPLAYERINTERFACE_HH_INCLUDED diff --git a/ffmpegaudio.cc b/ffmpegaudio.cc index c1d9aaac..bc37822f 100644 --- a/ffmpegaudio.cc +++ b/ffmpegaudio.cc @@ -43,25 +43,25 @@ static inline QString avErrorString( int errnum ) return QString::fromLatin1( buf ); } -AudioPlayer & AudioPlayer::instance() +AudioService & AudioService::instance() { - static AudioPlayer a; + static AudioService a; return a; } -AudioPlayer::AudioPlayer() +AudioService::AudioService() { av_register_all(); ao_initialize(); } -AudioPlayer::~AudioPlayer() +AudioService::~AudioService() { emit cancelPlaying( true ); ao_shutdown(); } -void AudioPlayer::playMemory( const char * ptr, int size ) +void AudioService::playMemory( const char * ptr, int size ) { emit cancelPlaying( false ); QByteArray audioData( ptr, size ); @@ -74,7 +74,7 @@ void AudioPlayer::playMemory( const char * ptr, int size ) thread->start(); } -void AudioPlayer::stop() +void AudioService::stop() { emit cancelPlaying( false ); } diff --git a/ffmpegaudio.hh b/ffmpegaudio.hh index dd10cd4b..2dbda7bb 100644 --- a/ffmpegaudio.hh +++ b/ffmpegaudio.hh @@ -12,12 +12,12 @@ namespace Ffmpeg { -class AudioPlayer : public QObject +class AudioService : public QObject { Q_OBJECT public: - static AudioPlayer & instance(); + static AudioService & instance(); void playMemory( const char * ptr, int size ); void stop(); @@ -26,10 +26,8 @@ signals: void error( QString const & message ); private: - AudioPlayer(); - ~AudioPlayer(); - AudioPlayer( AudioPlayer const & ); - AudioPlayer & operator=( AudioPlayer const & ); + AudioService(); + ~AudioService(); }; class DecoderThread: public QThread diff --git a/ffmpegaudioplayer.hh b/ffmpegaudioplayer.hh new file mode 100644 index 00000000..7de992f2 --- /dev/null +++ b/ffmpegaudioplayer.hh @@ -0,0 +1,40 @@ +/* This file is (c) 2018 Igor Kushnir + * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ + +#ifndef FFMPEGAUDIOPLAYER_HH_INCLUDED +#define FFMPEGAUDIOPLAYER_HH_INCLUDED + +#include "audioplayerinterface.hh" +#include "ffmpegaudio.hh" + +#ifndef DISABLE_INTERNAL_PLAYER + +namespace Ffmpeg +{ + +class AudioPlayer : public AudioPlayerInterface +{ + Q_OBJECT +public: + AudioPlayer() + { + connect( &AudioService::instance(), SIGNAL( error( QString ) ), + this, SIGNAL( error( QString ) ) ); + } + + virtual void play( const char * data, int size ) + { + AudioService::instance().playMemory( data, size ); + } + + virtual void stop() + { + AudioService::instance().stop(); + } +}; + +} + +#endif // DISABLE_INTERNAL_PLAYER + +#endif // FFMPEGAUDIOPLAYER_HH_INCLUDED diff --git a/goldendict.pro b/goldendict.pro index d2c05a6f..0dd6cd5b 100644 --- a/goldendict.pro +++ b/goldendict.pro @@ -264,6 +264,9 @@ HEADERS += folding.hh \ article_maker.hh \ scanpopup.hh \ articleview.hh \ + audioplayerinterface.hh \ + audioplayerfactory.hh \ + ffmpegaudioplayer.hh \ externalviewer.hh \ wordfinder.hh \ groupcombobox.hh \ @@ -394,6 +397,7 @@ SOURCES += folding.cc \ article_maker.cc \ scanpopup.cc \ articleview.cc \ + audioplayerfactory.cc \ externalviewer.cc \ wordfinder.cc \ groupcombobox.cc \ diff --git a/mainwindow.cc b/mainwindow.cc index f721becf..218a1bbe 100644 --- a/mainwindow.cc +++ b/mainwindow.cc @@ -118,6 +118,7 @@ MainWindow::MainWindow( Config::Class & cfg_ ): articleNetMgr( this, dictionaries, articleMaker, cfg.preferences.disallowContentFromOtherSites, cfg.preferences.hideGoldenDictHeader ), dictNetMgr( this ), + audioPlayerFactory( cfg.preferences ), wordFinder( this ), newReleaseCheckTimer( this ), latestReleaseReply( 0 ), @@ -1373,8 +1374,8 @@ void MainWindow::makeScanPopup() !cfg.preferences.enableClipboardHotkey ) return; - scanPopup = new ScanPopup( 0, cfg, articleNetMgr, dictionaries, groupInstances, - history ); + scanPopup = new ScanPopup( 0, cfg, articleNetMgr, audioPlayerFactory.player(), + dictionaries, groupInstances, history ); scanPopup->setStyleSheet( styleSheet() ); @@ -1530,8 +1531,8 @@ void MainWindow::addNewTab() ArticleView * MainWindow::createNewTab( bool switchToIt, QString const & name ) { - ArticleView * view = new ArticleView( this, articleNetMgr, dictionaries, - groupInstances, false, cfg, + ArticleView * view = new ArticleView( this, articleNetMgr, audioPlayerFactory.player(), + dictionaries, groupInstances, false, cfg, *ui.searchInPageAction, dictionaryBar.toggleViewAction(), groupList ); @@ -2087,6 +2088,8 @@ void MainWindow::editPreferences() cfg.preferences = p; + audioPlayerFactory.setPreferences( cfg.preferences ); + beforeScanPopupSeparator->setVisible( cfg.preferences.enableScanPopup ); enableScanPopup->setVisible( cfg.preferences.enableScanPopup ); afterScanPopupSeparator->setVisible( cfg.preferences.enableScanPopup ); diff --git a/mainwindow.hh b/mainwindow.hh index b681f9fa..7543291e 100644 --- a/mainwindow.hh +++ b/mainwindow.hh @@ -15,6 +15,7 @@ #include "config.hh" #include "dictionary.hh" #include "article_netmgr.hh" +#include "audioplayerfactory.hh" #include "instances.hh" #include "article_maker.hh" #include "scanpopup.hh" @@ -151,6 +152,7 @@ private: QNetworkAccessManager dictNetMgr; // We give dictionaries a separate manager, // since their requests can be destroyed // in a separate thread + AudioPlayerFactory audioPlayerFactory; WordList * wordList; QLineEdit * translateLine; diff --git a/scanpopup.cc b/scanpopup.cc index aed999b0..dbd33c49 100644 --- a/scanpopup.cc +++ b/scanpopup.cc @@ -36,6 +36,7 @@ Qt::Popup ScanPopup::ScanPopup( QWidget * parent, Config::Class & cfg_, ArticleNetworkAccessManager & articleNetMgr, + AudioPlayerPtr const & audioPlayer_, std::vector< sptr< Dictionary::Class > > const & allDictionaries_, Instances::Groups const & groups_, History & history_ ): @@ -72,8 +73,8 @@ ScanPopup::ScanPopup( QWidget * parent, ui.queryError->hide(); - definition = new ArticleView( ui.outerFrame, articleNetMgr, allDictionaries, - groups, true, cfg, + definition = new ArticleView( ui.outerFrame, articleNetMgr, audioPlayer_, + allDictionaries, groups, true, cfg, openSearchAction, dictionaryBar.toggleViewAction() ); diff --git a/scanpopup.hh b/scanpopup.hh index 95e15011..7352bf85 100644 --- a/scanpopup.hh +++ b/scanpopup.hh @@ -30,6 +30,7 @@ public: ScanPopup( QWidget * parent, Config::Class & cfg, ArticleNetworkAccessManager &, + AudioPlayerPtr const &, std::vector< sptr< Dictionary::Class > > const & allDictionaries, Instances::Groups const &, History & ); From 278e05cbf3ac2d3952fc709e81b63439612730a6 Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Thu, 22 Mar 2018 19:07:10 +0200 Subject: [PATCH 3/3] Run a single external audio player process at a time External and internal audio players work similarly now. Fixes #950. * inherit a new ExternalAudioPlayer class from AudioPlayerInterface; * use an existing ExternalViewer class to implement ExternalAudioPlayer; * take (const char *, int) instead of std::vector in ExternalViewer constructor to fit into AudioPlayerInterface; * extend ExternalViewer API to let ExternalAudioPlayer stop superseded audio player processes; * make AudioPlayerInterface::play() return an error message string to allow reporting immediate failures from derived classes; * Document AudioPlayerInterface API; * Document AudioPlayerFactory::player(); * use the common audio interface exclusively in ArticleView. --- articleview.cc | 46 ++++--------------- audioplayerfactory.cc | 28 +++++++++++- audioplayerfactory.hh | 8 ++++ audioplayerinterface.hh | 8 +++- externalaudioplayer.cc | 99 +++++++++++++++++++++++++++++++++++++++++ externalaudioplayer.hh | 44 ++++++++++++++++++ externalviewer.cc | 37 ++++++++++++--- externalviewer.hh | 19 ++++++-- ffmpegaudioplayer.hh | 3 +- goldendict.pro | 2 + 10 files changed, 242 insertions(+), 52 deletions(-) create mode 100644 externalaudioplayer.cc create mode 100644 externalaudioplayer.hh diff --git a/articleview.cc b/articleview.cc index aa7d006d..b9f63ee9 100644 --- a/articleview.cc +++ b/articleview.cc @@ -2,7 +2,6 @@ * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ #include "articleview.hh" -#include "externalviewer.hh" #include #include #include @@ -269,11 +268,7 @@ void ArticleView::setGroupComboBox( GroupComboBox const * g ) ArticleView::~ArticleView() { cleanupTemp(); - -#ifndef DISABLE_INTERNAL_PLAYER - if ( audioPlayer ) - audioPlayer->stop(); -#endif + audioPlayer->stop(); #if QT_VERSION >= QT_VERSION_CHECK(4, 6, 0) ui.definition->ungrabGesture( Gestures::GDPinchGestureType ); @@ -285,12 +280,8 @@ void ArticleView::showDefinition( QString const & word, unsigned group, QString const & scrollTo, Contexts const & contexts_ ) { - -#ifndef DISABLE_INTERNAL_PLAYER // first, let's stop the player - if ( audioPlayer ) - audioPlayer->stop(); -#endif + audioPlayer->stop(); QUrl req; Contexts contexts( contexts_ ); @@ -354,11 +345,8 @@ void ArticleView::showDefinition( QString const & word, QStringList const & dict if( dictIDs.isEmpty() ) return; -#ifndef DISABLE_INTERNAL_PLAYER // first, let's stop the player - if ( audioPlayer ) - audioPlayer->stop(); -#endif + audioPlayer->stop(); QUrl req; @@ -1910,28 +1898,10 @@ void ArticleView::resourceDownloadFinished() Dictionary::WebMultimediaDownload::isAudioUrl( resourceDownloadUrl ) ) { // Audio data -#ifndef DISABLE_INTERNAL_PLAYER - if ( audioPlayer ) - { - connect( audioPlayer.data(), SIGNAL( error( QString ) ), this, SLOT( audioPlayerError( QString ) ), Qt::UniqueConnection ); - audioPlayer->play( data.data(), data.size() ); - } - else -#endif - { - // Use external viewer to play the file - try - { - ExternalViewer * viewer = new ExternalViewer( this, data, "wav", cfg.preferences.audioPlaybackProgram.trimmed() ); - - // Once started, it will erase itself - viewer->start(); - } - catch( ExternalViewer::Ex & e ) - { - QMessageBox::critical( this, "GoldenDict", tr( "Failed to run a player to play sound file: %1" ).arg( e.what() ) ); - } - } + connect( audioPlayer.data(), SIGNAL( error( QString ) ), this, SLOT( audioPlayerError( QString ) ), Qt::UniqueConnection ); + QString errorMessage = audioPlayer->play( data.data(), data.size() ); + if( !errorMessage.isEmpty() ) + QMessageBox::critical( this, "GoldenDict", tr( "Failed to play sound file: %1" ).arg( errorMessage ) ); } else { @@ -1987,7 +1957,7 @@ void ArticleView::resourceDownloadFinished() void ArticleView::audioPlayerError( QString const & message ) { - emit statusBarMessage( tr( "WARNING: FFmpeg Audio Player: %1" ).arg( message ), + emit statusBarMessage( tr( "WARNING: Audio Player: %1" ).arg( message ), 10000, QPixmap( ":/icons/error.png" ) ); } diff --git a/audioplayerfactory.cc b/audioplayerfactory.cc index cf1d5e80..68184b29 100644 --- a/audioplayerfactory.cc +++ b/audioplayerfactory.cc @@ -1,11 +1,16 @@ /* This file is (c) 2018 Igor Kushnir * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ +#include +#include #include "audioplayerfactory.hh" #include "ffmpegaudioplayer.hh" +#include "externalaudioplayer.hh" +#include "gddebug.hh" AudioPlayerFactory::AudioPlayerFactory( Config::Preferences const & p ) : - useInternalPlayer( p.useInternalPlayer ) + useInternalPlayer( p.useInternalPlayer ), + audioPlaybackProgram( p.audioPlaybackProgram ) { reset(); } @@ -15,8 +20,20 @@ void AudioPlayerFactory::setPreferences( Config::Preferences const & p ) if( p.useInternalPlayer != useInternalPlayer ) { useInternalPlayer = p.useInternalPlayer; + audioPlaybackProgram = p.audioPlaybackProgram; reset(); } + else + if( !useInternalPlayer && p.audioPlaybackProgram != audioPlaybackProgram ) + { + audioPlaybackProgram = p.audioPlaybackProgram; + ExternalAudioPlayer * const externalPlayer = + qobject_cast< ExternalAudioPlayer * >( playerPtr.data() ); + if( externalPlayer ) + setAudioPlaybackProgram( *externalPlayer ); + else + gdWarning( "External player was expected, but it does not exist.\n" ); + } } void AudioPlayerFactory::reset() @@ -27,6 +44,13 @@ void AudioPlayerFactory::reset() else #endif { - playerPtr.reset(); + QScopedPointer< ExternalAudioPlayer > externalPlayer( new ExternalAudioPlayer ); + setAudioPlaybackProgram( *externalPlayer ); + playerPtr.reset( externalPlayer.take() ); } } + +void AudioPlayerFactory::setAudioPlaybackProgram( ExternalAudioPlayer & externalPlayer ) +{ + externalPlayer.setPlayerCommandLine( audioPlaybackProgram.trimmed() ); +} diff --git a/audioplayerfactory.hh b/audioplayerfactory.hh index 482f385e..09fdf43a 100644 --- a/audioplayerfactory.hh +++ b/audioplayerfactory.hh @@ -7,17 +7,25 @@ #include "audioplayerinterface.hh" #include "config.hh" +class ExternalAudioPlayer; + class AudioPlayerFactory { Q_DISABLE_COPY( AudioPlayerFactory ) public: explicit AudioPlayerFactory( Config::Preferences const & ); void setPreferences( Config::Preferences const & ); + /// The returned reference to a smart pointer is valid as long as this object + /// exists. The pointer to the owned AudioPlayerInterface may change after the + /// call to setPreferences(), but it is guaranteed to never be null. AudioPlayerPtr const & player() const { return playerPtr; } private: void reset(); + void setAudioPlaybackProgram( ExternalAudioPlayer & externalPlayer ); + bool useInternalPlayer; + QString audioPlaybackProgram; AudioPlayerPtr playerPtr; }; diff --git a/audioplayerinterface.hh b/audioplayerinterface.hh index 66b055bf..d9cd22e6 100644 --- a/audioplayerinterface.hh +++ b/audioplayerinterface.hh @@ -12,10 +12,16 @@ class AudioPlayerInterface : public QObject { Q_OBJECT public: - virtual void play( const char * data, int size ) = 0; + /// Stops current playback if any, copies the audio buffer at [data, data + size), + /// then plays this buffer. It is safe to invalidate \p data after this function call. + /// Returns an error message in case of immediate failure; an empty string + /// in case of success. + virtual QString play( const char * data, int size ) = 0; + /// Stops current playback if any. virtual void stop() = 0; signals: + /// Notifies of asynchronous errors. void error( QString message ); }; diff --git a/externalaudioplayer.cc b/externalaudioplayer.cc new file mode 100644 index 00000000..7bda4164 --- /dev/null +++ b/externalaudioplayer.cc @@ -0,0 +1,99 @@ +/* This file is (c) 2018 Igor Kushnir + * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ + +#include "externalaudioplayer.hh" +#include "externalviewer.hh" + +ExternalAudioPlayer::ExternalAudioPlayer() : exitingViewer( 0 ) +{ +} + +ExternalAudioPlayer::~ExternalAudioPlayer() +{ + // Destroy viewers immediately to prevent memory and temporary file leaks + // at application exit. + + // Set viewer to null first and foremost to make sure that onViewerDestroyed() + // doesn't attempt to start viewer or mess the smart pointer up. + stopAndDestroySynchronously( viewer.take() ); + + stopAndDestroySynchronously( exitingViewer ); +} + +void ExternalAudioPlayer::setPlayerCommandLine( QString const & playerCommandLine_ ) +{ + playerCommandLine = playerCommandLine_; +} + +QString ExternalAudioPlayer::play( const char * data, int size ) +{ + stop(); + + Q_ASSERT( !viewer && "viewer must be null at this point for exception safety." ); + try + { + // Our destructor properly destroys viewers we remember about. + // In the unlikely case that we call viewer.reset() during the application + // exit, ~QObject() prevents leaks as this class is a parent of all viewers. + viewer.reset( new ExternalViewer( data, size, "wav", playerCommandLine, this ) ); + } + catch( const ExternalViewer::Ex & e ) + { + return e.what(); + } + + if( exitingViewer ) + return QString(); // Will start later. + return startViewer(); +} + +void ExternalAudioPlayer::stop() +{ + if( !exitingViewer && viewer && !viewer->stop() ) + { + // Give the previous viewer a chance to stop before starting a new one. + // This prevents overlapping audio and possible conflicts between + // external program instances. + // Graceful stopping is better than calling viewer.reset() because: + // 1) the process gets a chance to clean up and save its state; + // 2) there is no event loop blocking and consequently no (short) UI freeze + // while synchronously waiting for the external process to exit. + exitingViewer = viewer.take(); + } + else // viewer is either not started or already stopped -> simply destroy it. + viewer.reset(); +} + +void ExternalAudioPlayer::onViewerDestroyed( QObject * destroyedViewer ) +{ + if( exitingViewer == destroyedViewer ) + { + exitingViewer = 0; + if( viewer ) + { + QString errorMessage = startViewer(); + if( !errorMessage.isEmpty() ) + emit error( errorMessage ); + } + } + else + if( viewer.data() == destroyedViewer ) + viewer.take(); // viewer finished and died -> release ownership. +} + +QString ExternalAudioPlayer::startViewer() +{ + Q_ASSERT( !exitingViewer && viewer ); + connect( viewer.data(), SIGNAL( destroyed( QObject * ) ), + this, SLOT( onViewerDestroyed( QObject * ) ) ); + try + { + viewer->start(); + } + catch( const ExternalViewer::Ex & e ) + { + viewer.reset(); + return e.what(); + } + return QString(); +} diff --git a/externalaudioplayer.hh b/externalaudioplayer.hh new file mode 100644 index 00000000..c9a531aa --- /dev/null +++ b/externalaudioplayer.hh @@ -0,0 +1,44 @@ +/* This file is (c) 2018 Igor Kushnir + * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ + +#ifndef EXTERNALAUDIOPLAYER_HH_INCLUDED +#define EXTERNALAUDIOPLAYER_HH_INCLUDED + +#include +#include +#include "audioplayerinterface.hh" + +class ExternalViewer; + +class ExternalAudioPlayer : public AudioPlayerInterface +{ + Q_OBJECT +public: + ExternalAudioPlayer(); + ~ExternalAudioPlayer(); + /// \param playerCommandLine_ Will be used in future play() calls. + void setPlayerCommandLine( QString const & playerCommandLine_ ); + + virtual QString play( const char * data, int size ); + virtual void stop(); + +private slots: + void onViewerDestroyed( QObject * destroyedViewer ); + +private: + QString startViewer(); + + QString playerCommandLine; + ExternalViewer * exitingViewer; ///< If not null: points to the previous viewer, + ///< the current viewer (if any) is not started yet + ///< and waits for exitingViewer to be destroyed first. + + struct ScopedPointerDeleteLater + { + static void cleanup( QObject * p ) { if( p ) p->deleteLater(); } + }; + // deleteLater() is safer because viewer actively participates in the QEventLoop. + QScopedPointer< ExternalViewer, ScopedPointerDeleteLater > viewer; +}; + +#endif // EXTERNALAUDIOPLAYER_HH_INCLUDED diff --git a/externalviewer.cc b/externalviewer.cc index 5a56f0a7..b89043a6 100644 --- a/externalviewer.cc +++ b/externalviewer.cc @@ -2,22 +2,21 @@ * Part of GoldenDict. Licensed under GPLv3 or later, see the LICENSE file */ #include +#include #include "externalviewer.hh" #include "parsecmdline.hh" #include "gddebug.hh" -using std::vector; - -ExternalViewer::ExternalViewer( QObject * parent, vector< char > const & data, - QString const & extension, - QString const & viewerCmdLine_ ) +ExternalViewer::ExternalViewer( const char * data, int size, + QString const & extension, QString const & viewerCmdLine_, + QObject * parent) throw( exCantCreateTempFile ): QObject( parent ), tempFile( QDir::temp().filePath( QString( "gd-XXXXXXXX." ) + extension ) ), viewer( this ), viewerCmdLine( viewerCmdLine_ ) { - if ( !tempFile.open() || (size_t) tempFile.write( &data.front(), data.size() ) != data.size() ) + if ( !tempFile.open() || tempFile.write( data, size ) != size ) throw exCantCreateTempFile(); tempFileName = tempFile.fileName(); // For some reason it loses it after it was closed() @@ -53,3 +52,29 @@ void ExternalViewer::start() throw( exCantRunViewer ) else throw exCantRunViewer( tr( "the viewer program name is empty" ).toUtf8().data() ); } + +bool ExternalViewer::stop() +{ + if( viewer.state() == QProcess::NotRunning ) + return true; + viewer.terminate(); + QTimer::singleShot( 1000, &viewer, SLOT( kill() ) ); // In case terminate() fails. + return false; +} + +void ExternalViewer::stopSynchronously() +{ + // This implementation comes straight from QProcess::~QProcess(). + if( viewer.state() == QProcess::NotRunning ) + return; + viewer.kill(); + viewer.waitForFinished(); +} + +void stopAndDestroySynchronously( ExternalViewer * viewer ) +{ + if( !viewer ) + return; + viewer->stopSynchronously(); + delete viewer; +} diff --git a/externalviewer.hh b/externalviewer.hh index 832ed503..d2014d40 100644 --- a/externalviewer.hh +++ b/externalviewer.hh @@ -7,7 +7,6 @@ #include #include #include -#include #include "ex.hh" /// An external viewer, opens resources in other programs @@ -26,14 +25,26 @@ public: DEF_EX( exCantCreateTempFile, "Couldn't create temporary file.", Ex ) DEF_EX_STR( exCantRunViewer, "Couldn't run external viewer:", Ex ) - ExternalViewer( QObject * parent, std::vector< char > const & data, - QString const & extension, QString const & viewerCmdLine ) + ExternalViewer( const char * data, int size, + QString const & extension, QString const & viewerCmdLine, + QObject * parent = 0 ) throw( exCantCreateTempFile ); // Once this is called, the object will be deleted when it's done, even if // the function throws. void start() throw( exCantRunViewer ); + + /// If the external process is running, requests its termination and returns + /// false - expect the QObject::destroyed() signal to be emitted soon. + /// If the external process is not running, returns true, the object + /// destruction is not necessarily scheduled in this case. + bool stop(); + /// Kills the process if it is running and waits for it to finish. + void stopSynchronously(); }; -#endif +/// Call this function instead of simply deleting viewer to prevent the +/// "QProcess: Destroyed while process X is still running." warning in log. +void stopAndDestroySynchronously( ExternalViewer * viewer ); +#endif diff --git a/ffmpegaudioplayer.hh b/ffmpegaudioplayer.hh index 7de992f2..bfce56e6 100644 --- a/ffmpegaudioplayer.hh +++ b/ffmpegaudioplayer.hh @@ -22,9 +22,10 @@ public: this, SIGNAL( error( QString ) ) ); } - virtual void play( const char * data, int size ) + virtual QString play( const char * data, int size ) { AudioService::instance().playMemory( data, size ); + return QString(); } virtual void stop() diff --git a/goldendict.pro b/goldendict.pro index 0dd6cd5b..fa1505b2 100644 --- a/goldendict.pro +++ b/goldendict.pro @@ -267,6 +267,7 @@ HEADERS += folding.hh \ audioplayerinterface.hh \ audioplayerfactory.hh \ ffmpegaudioplayer.hh \ + externalaudioplayer.hh \ externalviewer.hh \ wordfinder.hh \ groupcombobox.hh \ @@ -398,6 +399,7 @@ SOURCES += folding.cc \ scanpopup.cc \ articleview.cc \ audioplayerfactory.cc \ + externalaudioplayer.cc \ externalviewer.cc \ wordfinder.cc \ groupcombobox.cc \