Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: default to Qt Multimedia and delete default player shenanigans #1994

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/audio/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
Code to support GD's internal/external audio players.

Only `audioplayerinterface.hh` is supposed to be used outside this folder.
25 changes: 1 addition & 24 deletions src/audio/audioplayerfactory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
#include <QObject>
#include <utility>
#include "audioplayerfactory.hh"
#include "ffmpegaudioplayer.hh"
#include "multimediaaudioplayer.hh"
#include "externalaudioplayer.hh"

AudioPlayerFactory::AudioPlayerFactory( bool useInternalPlayer,
Expand Down Expand Up @@ -48,29 +46,8 @@ void AudioPlayerFactory::setPreferences( bool new_useInternalPlayer,
void AudioPlayerFactory::reset()
{
if ( useInternalPlayer ) {
// qobject_cast checks below account for the case when an unsupported backend
// is stored in config. After this backend is replaced with the default one
// upon preferences saving, the code below does not reset playerPtr with
// another object of the same type.

#ifdef MAKE_FFMPEG_PLAYER
Q_ASSERT( InternalPlayerBackend::defaultBackend().isFfmpeg()
&& "Adjust the code below after changing the default backend." );

if ( !internalPlayerBackend.isQtmultimedia() ) {
if ( !playerPtr || !qobject_cast< Ffmpeg::AudioPlayer * >( playerPtr.data() ) ) {
playerPtr.reset( new Ffmpeg::AudioPlayer );
}
return;
}
#endif

#ifdef MAKE_QTMULTIMEDIA_PLAYER
if ( !playerPtr || !qobject_cast< MultimediaAudioPlayer * >( playerPtr.data() ) ) {
playerPtr.reset( new MultimediaAudioPlayer );
}
playerPtr.reset( internalPlayerBackend.getActualPlayer() );
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reset will delete the current one and take over the ownership of incoming pointer.

return;
#endif
}

std::unique_ptr< ExternalAudioPlayer > externalPlayer( new ExternalAudioPlayer );
Expand Down
59 changes: 31 additions & 28 deletions src/audio/internalplayerbackend.cc
Original file line number Diff line number Diff line change
@@ -1,51 +1,54 @@
#include "internalplayerbackend.hh"
#include "ffmpegaudioplayer.hh"
#include "multimediaaudioplayer.hh"

#ifdef MAKE_FFMPEG_PLAYER
constexpr auto ffmpeg = "FFmpeg";
#endif

#ifdef MAKE_QTMULTIMEDIA_PLAYER
constexpr auto qtmultimedia = "Qt Multimedia";
#endif

bool InternalPlayerBackend::anyAvailable()
{
#if defined( MAKE_FFMPEG_PLAYER ) || defined( MAKE_QTMULTIMEDIA_PLAYER )
return true;
#else
static_assert( false, "No audio player backend. Please enable one." );
return false;
#endif
}

InternalPlayerBackend InternalPlayerBackend::defaultBackend()
{
#if defined( MAKE_FFMPEG_PLAYER )
return ffmpeg();
#elif defined( MAKE_QTMULTIMEDIA_PLAYER )
return qtmultimedia();
#else
return InternalPlayerBackend( QString() );
#endif
}

QStringList InternalPlayerBackend::nameList()
QStringList InternalPlayerBackend::availableBackends()
{
QStringList result;
#ifdef MAKE_FFMPEG_PLAYER
result.push_back( ffmpeg().uiName() );
#endif
#ifdef MAKE_QTMULTIMEDIA_PLAYER
result.push_back( qtmultimedia().uiName() );
result.push_back( qtmultimedia );
#endif
#ifdef MAKE_FFMPEG_PLAYER
result.push_back( ffmpeg );
#endif
return result;
}

bool InternalPlayerBackend::isFfmpeg() const
AudioPlayerInterface * InternalPlayerBackend::getActualPlayer()
{
// The one in user's config is not availiable,
// fall back to the default one
if ( name.isEmpty() || !availableBackends().contains( name ) ) {
name = availableBackends().constFirst();
}

#ifdef MAKE_FFMPEG_PLAYER
return *this == ffmpeg();
#else
return false;
if ( name == ffmpeg ) {
return new Ffmpeg::AudioPlayer();
};
#endif
}

bool InternalPlayerBackend::isQtmultimedia() const
{
#ifdef MAKE_QTMULTIMEDIA_PLAYER
return *this == qtmultimedia();
#else
return false;
if ( name == qtmultimedia ) {
return new MultimediaAudioPlayer();
};
#endif
qCritical( "Impossible situation. If ever reached, fix elsewhere. " );
return nullptr;
}
38 changes: 9 additions & 29 deletions src/audio/internalplayerbackend.hh
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#pragma once
#include "audioplayerinterface.hh"
#include "ffmpegaudioplayer.hh"
#include "multimediaaudioplayer.hh"
#include <QScopedPointer>
#include <QStringList>

/// Overly engineered dummy/helper/wrapper "backend", which is not, to manage backends.
Expand All @@ -7,22 +11,17 @@ class InternalPlayerBackend
public:
/// Returns true if at least one backend is available.
static bool anyAvailable();
/// Returns the default backend or null backend if none is available.
static InternalPlayerBackend defaultBackend();
AudioPlayerInterface * getActualPlayer();
/// Returns the name list of supported backends.
static QStringList nameList();
/// The first one willl be the default one
static QStringList availableBackends();

/// Returns true if built with FFmpeg player support and the name matches.
bool isFfmpeg() const;
/// Returns true if built with Qt Multimedia player support and the name matches.
bool isQtmultimedia() const;

QString const & uiName() const
QString const & getName() const
{
return name;
}

void setUiName( QString const & name_ )
void setName( QString const & name_ )
{
name = name_;
}
Expand All @@ -38,24 +37,5 @@ public:
}

private:
#ifdef MAKE_FFMPEG_PLAYER
static InternalPlayerBackend ffmpeg()
{
return InternalPlayerBackend( "FFmpeg" );
}
#endif

#ifdef MAKE_QTMULTIMEDIA_PLAYER
static InternalPlayerBackend qtmultimedia()
{
return InternalPlayerBackend( "Qt Multimedia" );
}
#endif

explicit InternalPlayerBackend( QString const & name_ ):
name( name_ )
{
}

QString name;
};
5 changes: 2 additions & 3 deletions src/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ Preferences::Preferences():
pronounceOnLoadMain( false ),
pronounceOnLoadPopup( false ),
useInternalPlayer( InternalPlayerBackend::anyAvailable() ),
internalPlayerBackend( InternalPlayerBackend::defaultBackend() ),
checkForNewReleases( true ),
disallowContentFromOtherSites( false ),
hideGoldenDictHeader( false ),
Expand Down Expand Up @@ -951,7 +950,7 @@ Class load()
}

if ( !preferences.namedItem( "internalPlayerBackend" ).isNull() ) {
c.preferences.internalPlayerBackend.setUiName(
c.preferences.internalPlayerBackend.setName(
preferences.namedItem( "internalPlayerBackend" ).toElement().text() );
}

Expand Down Expand Up @@ -1918,7 +1917,7 @@ void save( Class const & c )
preferences.appendChild( opt );

opt = dd.createElement( "internalPlayerBackend" );
opt.appendChild( dd.createTextNode( c.preferences.internalPlayerBackend.uiName() ) );
opt.appendChild( dd.createTextNode( c.preferences.internalPlayerBackend.getName() ) );
preferences.appendChild( opt );

opt = dd.createElement( "audioPlaybackProgram" );
Expand Down
2 changes: 1 addition & 1 deletion src/config.hh
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ struct Preferences
// Whether the word should be pronounced on page load, in main window/popup
bool pronounceOnLoadMain, pronounceOnLoadPopup;
bool useInternalPlayer;
InternalPlayerBackend internalPlayerBackend;
InternalPlayerBackend internalPlayerBackend{};
QString audioPlaybackProgram;

ProxyServer proxyServer;
Expand Down
15 changes: 8 additions & 7 deletions src/ui/preferences.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ Preferences::Preferences( QWidget * parent, Config::Class & cfg_ ):
ui.pronounceOnLoadMain->setChecked( p.pronounceOnLoadMain );
ui.pronounceOnLoadPopup->setChecked( p.pronounceOnLoadPopup );

ui.internalPlayerBackend->addItems( InternalPlayerBackend::nameList() );
ui.internalPlayerBackend->addItems( InternalPlayerBackend::availableBackends() );

// Make sure that exactly one radio button in the group is checked and that
// on_useExternalPlayer_toggled() is called.
Expand All @@ -304,12 +304,13 @@ Preferences::Preferences( QWidget * parent, Config::Class & cfg_ ):
// Checking ui.useInternalPlayer automatically unchecks ui.useExternalPlayer.
ui.useInternalPlayer->setChecked( p.useInternalPlayer );

int index = ui.internalPlayerBackend->findText( p.internalPlayerBackend.uiName() );
if ( index < 0 ) { // The specified backend is unavailable.
index = ui.internalPlayerBackend->findText( InternalPlayerBackend::defaultBackend().uiName() );
int index = ui.internalPlayerBackend->findText( p.internalPlayerBackend.getName() );
if ( index >= 0 ) {
ui.internalPlayerBackend->setCurrentIndex( index );
}
else {
// Find no backend, just do nothing and let just let Qt select the first one.
}
Q_ASSERT( index >= 0 && "Logic error: the default backend must be present in the backend name list." );
ui.internalPlayerBackend->setCurrentIndex( index );
}
else {
ui.useInternalPlayer->hide();
Expand Down Expand Up @@ -493,7 +494,7 @@ Config::Preferences Preferences::getPreferences()
p.pronounceOnLoadMain = ui.pronounceOnLoadMain->isChecked();
p.pronounceOnLoadPopup = ui.pronounceOnLoadPopup->isChecked();
p.useInternalPlayer = ui.useInternalPlayer->isChecked();
p.internalPlayerBackend.setUiName( ui.internalPlayerBackend->currentText() );
p.internalPlayerBackend.setName( ui.internalPlayerBackend->currentText() );
p.audioPlaybackProgram = ui.audioPlaybackProgram->text();

p.proxyServer.enabled = ui.useProxyServer->isChecked();
Expand Down
Loading