[Amarok] Clean up the EngineController logic.
Seb Ruiz
ruiz at kde.org
Sat Aug 22 02:27:44 CEST 2009
Hi Alex,
Not sure if this is the commit behind this break, but it seems that
Amarok doesn't register automatic track changes anymore.
Anybody else have a clue if it isn't?
Seb
2009/8/19 Alex Merry <kde at randomguy3.me.uk>:
> commit 28b3e64d1dc5c7170d7b5d09e6cb4c8b3cdd4e02
> Author: Alex Merry <kde at randomguy3.me.uk>
> AuthorDate: Fri Aug 7 21:26:34 2009 +0100
> Commit: Alex Merry <kde at randomguy3.me.uk>
> CommitDate: Tue Aug 18 19:49:42 2009 +0100
>
> Clean up the EngineController logic.
>
> This commit has been sitting on my computer for too long.
>
> Leo: this does the fadeout logic the "right way" (and in doing so makes
> it work properly on GStreamer), at the expense of not working on
> CoreAudio (as I recall, there were some issues with keeping a
> VolumeFaderEffect in the audio path on CoreAudio). You might want to
> try re-enabling it and seeing if Phonon on CoreAudio is still broken.
>
> CCMAIL: lfranchi at kde.org
>
> diff --git a/src/EngineController.cpp b/src/EngineController.cpp
> index 4911d65..f8b93e1 100644
> --- a/src/EngineController.cpp
> +++ b/src/EngineController.cpp
> @@ -101,6 +101,14 @@ EngineController::~EngineController()
> }
>
> void
> +EngineController::createFadeoutEffect()
> +{
> + m_fader = new Phonon::VolumeFaderEffect( this );
> + m_path.insertEffect( m_fader );
> + m_fader->setFadeCurve( Phonon::VolumeFaderEffect::Fade9Decibel );
> +}
> +
> +void
> EngineController::initializePhonon()
> {
> DEBUG_BLOCK
> @@ -109,6 +117,7 @@ EngineController::initializePhonon()
> delete m_controller;
> delete m_audio;
> delete m_preamp;
> + delete m_fader;
>
> PERF_LOG( "EngineController: loading phonon objects" )
> m_media = new Phonon::MediaObject( this );
> @@ -118,19 +127,27 @@ EngineController::initializePhonon()
>
> m_controller = new Phonon::MediaController( m_media );
>
> - // HACK we turn off replaygain manually on OSX, until the phonon coreaudio backend is fixed.
> - // as the default is specified in the .cfg file, we can't just tell it to be a different default on OSX
> + // HACK we turn off replaygain and fader manually on OSX, until the phonon
> + // coreaudio backend is fixed as the default is specified in the .cfg file,
> + // we can't just tell it to be a different default on OSX
> #ifdef Q_WS_MAC
> AmarokConfig::setReplayGainMode( AmarokConfig::EnumReplayGainMode::Off );
> + AmarokConfig::setFadeout( false );
> #endif
>
> - // only create pre-amp if we have replaygain on, preamp can cause phonon issues
> + // only create pre-amp if we have replaygain on, VolumeFaderEffect can cause phonon issues
> if( AmarokConfig::replayGainMode() != AmarokConfig::EnumReplayGainMode::Off )
> {
> m_preamp = new Phonon::VolumeFaderEffect( this );
> m_path.insertEffect( m_preamp );
> }
>
> + // only create fader if we have fadeout on, VolumeFaderEffect can cause phonon issues
> + if( AmarokConfig::fadeout() && AmarokConfig::fadeoutLength() )
> + {
> + createFadeoutEffect();
> + }
> +
> m_media->setTickInterval( 100 );
> debug() << "Tick Interval (actual): " << m_media->tickInterval();
> PERF_LOG( "EngineController: loaded phonon objects" )
> @@ -149,13 +166,10 @@ EngineController::initializePhonon()
> connect( m_controller, SIGNAL( titleChanged( int ) ), SLOT( slotTitleChanged( int ) ) );
>
>
> - //TODO: The xine engine does not support crossfading. Cannot get the gstreamer engine to work, will test this once I do.
> -#if 0
> if( AmarokConfig::trackDelayLength() > -1 )
> m_media->setTransitionTime( AmarokConfig::trackDelayLength() ); // Also Handles gapless.
> else if( AmarokConfig::crossfadeLength() > 0 ) // TODO: Handle the possible options on when to crossfade.. the values are not documented anywhere however
> m_media->setTransitionTime( -AmarokConfig::crossfadeLength() );
> -#endif
> }
>
>
> @@ -271,11 +285,15 @@ EngineController::play() //SLOT
> {
> DEBUG_BLOCK
>
> + // FIXME: what should we do in BufferingState?
> if( m_media->state() == Phonon::PlayingState )
> return;
>
> - if( m_fader )
> - m_fader->deleteLater();
> + if ( m_fader )
> + {
> + m_fadeoutTimer->stop();
> + m_fader->setVolume(1.0);
> + }
>
> if ( m_media->state() == Phonon::PausedState )
> {
> @@ -343,7 +361,6 @@ EngineController::playUrl( const KUrl &url, uint offset )
> slotStopFadeout();
>
> debug() << "URL: " << url.url();
> - /// TODO: commented out since audiocd needs porting to new devicelib framework, should not affect other urls
>
> if ( url.url().startsWith( "audiocd:/" ) )
> {
> @@ -440,22 +457,23 @@ EngineController::stop( bool forceInstant ) //SLOT
> trackChangedNotify( Meta::TrackPtr( 0 ) );
> }
>
> - // Stop instantly if fadeout is already running, or the media is paused (i.e. pressing Stop twice)
> - if( m_fader || m_media->state() == Phonon::PausedState )
> + // Stop instantly if fadeout is already running, or if the media is not actually playing
> + if( m_fadeoutTimer->isActive() || m_media->state() != Phonon::PlayingState )
> {
> forceInstant = true;
> }
>
> if( AmarokConfig::fadeout() && AmarokConfig::fadeoutLength() && !forceInstant )
> {
> - stateChangedNotify( Phonon::StoppedState, Phonon::PlayingState ); //immediately disable Stop action
> + // WARNING: this can cause a gap in playback on GStreamer...
> + if ( !m_fader )
> + createFadeoutEffect();
>
> - m_fader = new Phonon::VolumeFaderEffect( this );
> - m_path.insertEffect( m_fader );
> - m_fader->setFadeCurve( Phonon::VolumeFaderEffect::Fade9Decibel );
> m_fader->fadeOut( AmarokConfig::fadeoutLength() );
>
> m_fadeoutTimer->start( AmarokConfig::fadeoutLength() + 1000 ); //add 1s for good measure, otherwise seems to cut off early (buffering..)
> +
> + stateChangedNotify( Phonon::StoppedState, m_media->state() );
> }
> else
> m_media->stop();
> @@ -477,12 +495,17 @@ EngineController::playPause() //SLOT
> //this is used by the TrayIcon, PlayPauseAction and DBus
> debug() << "PlayPause: phonon state" << m_media->state();
>
> - if( m_media->state() == Phonon::PausedState ||
> - m_media->state() == Phonon::StoppedState ||
> - m_media->state() == Phonon::LoadingState )
> - play();
> - else
> - pause();
> + switch ( m_media->state() )
> + {
> + case Phonon::PausedState:
> + case Phonon::StoppedState:
> + case Phonon::LoadingState:
> + play();
> + break;
> + default:
> + pause();
> + break;
> + }
> }
>
> void
> @@ -499,9 +522,10 @@ EngineController::seek( int ms ) //SLOT
> if ( m_boundedPlayback )
> seekTo = m_boundedPlayback->startPosition() + ms;
> else
> - seekTo = ms;
> + seekTo = ms;
>
> m_media->seek( static_cast<qint64>( seekTo ) );
> + // FIXME: is this correct for bounded playback?
> trackPositionChangedNotify( seekTo, true ); /* User seek */
> }
> else
> @@ -632,6 +656,15 @@ EngineController::setNextTrack( Meta::TrackPtr track )
> }
> }
>
> +Phonon::State
> +EngineController::state() const
> +{
> + if ( m_fadeoutTimer->isActive() )
> + return Phonon::StoppedState;
> + else
> + return phononMediaObject()->state();
> +}
> +
> bool
> EngineController::isStream()
> {
> @@ -816,9 +849,6 @@ EngineController::slotNewTrackPlaying( const Phonon::MediaSource &source )
> else if( m_preamp )
> m_preamp->setVolumeDecibel( 0.0 );
>
> - // state never changes if tracks are queued, but we need this to update the caption
> - stateChangedNotify( m_media->state(), m_media->state() );
> -
> trackChangedNotify( m_currentTrack );
> newTrackPlaying();
> }
> @@ -828,10 +858,6 @@ EngineController::slotStateChanged( Phonon::State newState, Phonon::State oldSta
> {
> DEBUG_BLOCK
>
> - // Sanity checks:
> - if( newState == oldState )
> - return;
> -
> // HACK:
> // The following check is an attempt to fix http://bugs.kde.org/show_bug.cgi?id=180339
> // ("amarok stops playing tracks") and other issues with Phonon.
> @@ -848,6 +874,10 @@ EngineController::slotStateChanged( Phonon::State newState, Phonon::State oldSta
> newState = Phonon::ErrorState; // Indicate error
> }
>
> + // Sanity checks:
> + if( newState == oldState )
> + return;
> +
> if( newState == Phonon::ErrorState ) // If media is borked, skip to next track
> {
> warning() << "Phonon failed to play this URL. Error: " << m_media->errorString();
> @@ -881,6 +911,16 @@ EngineController::slotStateChanged( Phonon::State newState, Phonon::State oldSta
> The::playlistActions()->requestNextTrack();
> }
>
> + if ( m_fadeoutTimer->isActive() )
> + {
> + // We've stopped already as far as the rest of Amarok is concerned
> + if ( oldState == Phonon::PlayingState )
> + oldState = Phonon::StoppedState;
> +
> + if ( oldState == newState )
> + return;
> + }
> +
> stateChangedNotify( newState, oldState );
> }
>
> @@ -989,8 +1029,8 @@ EngineController::slotStopFadeout() //SLOT
> m_fadeoutTimer->stop();
>
> if ( m_fader ) {
> - m_fader->deleteLater();
> m_media->stop();
> + m_fader->setVolume( 1.0 );
> }
> }
>
> diff --git a/src/EngineController.h b/src/EngineController.h
> index db91da0..d7e32c1 100644
> --- a/src/EngineController.h
> +++ b/src/EngineController.h
> @@ -101,7 +101,7 @@ public:
> /**
> * The state of the engine
> */
> - Phonon::State state() const { return phononMediaObject()->state(); }
> + Phonon::State state() const;
>
> /*enum Filetype { MP3 };*/ //assuming MP3 for time being
> /*AMAROK_EXPORT*/ static bool installDistroCodec();
> @@ -276,6 +276,8 @@ private:
> */
> void playUrl( const KUrl &url, uint offset );
>
> + void createFadeoutEffect();
> +
> static EngineController* s_instance;
> EngineController();
> ~EngineController();
>
>
>
--
Seb Ruiz
http://www.sebruiz.net/
http://amarok.kde.org/
More information about the Amarok-devel
mailing list