extragear/multimedia/amarok/src
Mark Kretschmann
kretschmann at kde.org
Tue Jun 17 16:13:20 CEST 2008
SVN commit 821449 by markey:
Refactor Meta::Observer so that we automatically unsubscribe all Meta subscriptions in the dtor. This fixes many issues, e.g. you are now actually able to listen to more than 5 tracks in a row without crashes ;) Also, it makes the code simpler, since we don't have to do manual bookkeeping.
The important API change is that you now subscribe/unsubscribe to Meta objects with subscribeTo(MetaPtr) / unsubscribeTo(MetaPtr); these two methods are built into the Observer base.
This was quite a bit of work, but I think it was worth it :)
CCMAIL: amarok-devel at kde.org
implement subscribeTo/unsubscribeTo
M +6 -6 Osd.cpp
M +4 -21 context/engines/current/CurrentEngine.cpp
M +2 -2 covermanager/CoverManager.cpp
M +157 -1 meta/Meta.cpp
M +38 -12 meta/Meta.h
M +1 -1 meta/proxy/MetaProxy_p.h
M +4 -4 playlist/PlaylistModel.cpp
M +3 -4 servicebrowser/lastfm/meta/MultiPlayableCapabilityImpl_p.h
M +6 -3 servicebrowser/magnatunestore/MagnatunePurchaseDialog.cpp
M +6 -7 widgets/TrackTooltip.cpp
--- trunk/extragear/multimedia/amarok/src/Osd.cpp #821448:821449
@@ -600,11 +600,11 @@
DEBUG_BLOCK
if( m_track && m_track->artist() )
- m_track->artist()->unsubscribe( this );
+ unsubscribeTo( m_track->artist() );
if( m_track && m_track->album() )
- m_track->album()->unsubscribe( this );
+ unsubscribeTo( m_track->album() );
if( m_track )
- m_track->unsubscribe( this );
+ unsubscribeTo( m_track );
m_track = track;
@@ -639,11 +639,11 @@
image = track->album()->image( 100 ).toImage();
if( m_track ) {
- m_track->subscribe( this );
+ subscribeTo( m_track );
if( m_track->artist() )
- m_track->artist()->subscribe( this );
+ subscribeTo( m_track->artist() );
if( m_track->album() )
- m_track->album()->subscribe( this );
+ subscribeTo( m_track->album() );
}
OSDWidget::show( text, image );
--- trunk/extragear/multimedia/amarok/src/context/engines/current/CurrentEngine.cpp #821448:821449
@@ -43,14 +43,6 @@
CurrentEngine::~CurrentEngine()
{
DEBUG_BLOCK
- if( m_currentTrack )
- {
- m_currentTrack->unsubscribe( this );
- if( m_currentTrack->album() )
- {
- m_currentTrack->album()->unsubscribe( this );
- }
- }
}
QStringList CurrentEngine::sources() const
@@ -83,11 +75,9 @@
if( m_currentTrack )
{
debug() << "2";
- m_currentTrack->unsubscribe( this );
+ unsubscribeTo( m_currentTrack );
if( m_currentTrack->album() )
- {
- m_currentTrack->album()->unsubscribe( this );
- }
+ unsubscribeTo( m_currentTrack->album() );
}
update();
}
@@ -110,24 +100,17 @@
void CurrentEngine::update()
{
DEBUG_BLOCK
-
- if( m_currentTrack ) {
- debug() << "Unsubscribing m_currentTrack.";
- m_currentTrack->unsubscribe( this );
- if( m_currentTrack->album() )
- m_currentTrack->album()->unsubscribe( this );
- }
m_currentTrack = The::engineController()->currentTrack();
if( !m_currentTrack )
return;
- m_currentTrack->subscribe( this );
+ subscribeTo( m_currentTrack );
QVariantMap trackInfo = Meta::Field::mapFromTrack( m_currentTrack.data() );
int width = coverWidth();
if( m_currentTrack->album() )
- m_currentTrack->album()->subscribe( this );
+ subscribeTo( m_currentTrack->album() );
removeAllData( "current" );
if( m_currentTrack->album() ) {
--- trunk/extragear/multimedia/amarok/src/covermanager/CoverManager.cpp #821448:821449
@@ -922,7 +922,7 @@
m_artist = i18n( "No Artist" );
setText( album->prettyName() );
setIcon( album->image( 100 ) );
- album->subscribe( qobject_cast<CoverManager*>(parent->parent()->parent()) );
+ qobject_cast<CoverManager*>(parent->parent()->parent())->subscribeTo( album ); // Man this looks sick
// setDragEnabled( true );
// setDropEnabled( true );
calcRect();
@@ -930,7 +930,7 @@
CoverViewItem::~CoverViewItem()
{
- m_albumPtr->unsubscribe( qobject_cast<CoverManager*>( m_parent->parent()->parent()) );
+ qobject_cast<CoverManager*>(m_parent->parent()->parent())->subscribeTo( m_albumPtr );
}
bool CoverViewItem::hasCover() const
{
--- trunk/extragear/multimedia/amarok/src/meta/Meta.cpp #821448:821449
@@ -1,6 +1,7 @@
/* This file is part of the KDE project
Copyright (C) 2007 Maximilian Kossick <maximilian.kossick at googlemail.com>
Copyright (C) 2007 Ian Monroe <ian at monroe.nu>
+ Copyright (C) 2008 Mark Kretschmann <kretschmann at kde.org>
This program is free software; you can redistribute it and/or
modify it under the terms of the GNU General Public License
@@ -24,6 +25,7 @@
#include "Collection.h"
#include "Debug.h"
#include "QueryMaker.h"
+#include "../servicebrowser/magnatunestore/MagnatuneMeta.h"
#include <QDir>
#include <QImage>
@@ -34,10 +36,158 @@
Meta::Observer::~Observer()
{
- //nothing to do
+ // Unsubscribe all stray Meta subscriptions:
+
+ foreach( TrackPtr ptr, m_trackSubscriptions )
+ if( ptr )
+ ptr->unsubscribe( this );
+ foreach( ArtistPtr ptr, m_artistSubscriptions )
+ if( ptr )
+ ptr->unsubscribe( this );
+ foreach( AlbumPtr ptr, m_albumSubscriptions )
+ if( ptr )
+ ptr->unsubscribe( this );
+ foreach( GenrePtr ptr, m_genreSubscriptions )
+ if( ptr )
+ ptr->unsubscribe( this );
+ foreach( ComposerPtr ptr, m_composerSubscriptions )
+ if( ptr )
+ ptr->unsubscribe( this );
+ foreach( YearPtr ptr, m_yearSubscriptions )
+ if( ptr )
+ ptr->unsubscribe( this );
+ foreach( MagnatuneAlbum* ptr, m_magnatuneAlbumSubscriptions )
+ if( ptr )
+ ptr->unsubscribe( this );
}
void
+Meta::Observer::subscribeTo( TrackPtr ptr )
+{
+ if( ptr ) {
+ ptr->subscribe( this );
+ m_trackSubscriptions.append( ptr );
+ }
+}
+
+void
+Meta::Observer::unsubscribeTo( TrackPtr ptr )
+{
+ if( ptr ) {
+ ptr->unsubscribe( this );
+ m_trackSubscriptions.remove( ptr );
+ }
+}
+
+void
+Meta::Observer::subscribeTo( ArtistPtr ptr )
+{
+ if( ptr ) {
+ ptr->subscribe( this );
+ m_artistSubscriptions.append( ptr );
+ }
+}
+
+void
+Meta::Observer::unsubscribeTo( ArtistPtr ptr )
+{
+ if( ptr ) {
+ ptr->unsubscribe( this );
+ m_artistSubscriptions.remove( ptr );
+ }
+}
+
+void
+Meta::Observer::subscribeTo( AlbumPtr ptr )
+{
+ if( ptr ) {
+ ptr->subscribe( this );
+ m_albumSubscriptions.append( ptr );
+ }
+}
+
+void
+Meta::Observer::unsubscribeTo( AlbumPtr ptr )
+{
+ if( ptr ) {
+ ptr->unsubscribe( this );
+ m_albumSubscriptions.remove( ptr );
+ }
+}
+
+void
+Meta::Observer::subscribeTo( ComposerPtr ptr )
+{
+ if( ptr ) {
+ ptr->subscribe( this );
+ m_composerSubscriptions.append( ptr );
+ }
+}
+
+void
+Meta::Observer::unsubscribeTo( ComposerPtr ptr )
+{
+ if( ptr ) {
+ ptr->unsubscribe( this );
+ m_composerSubscriptions.remove( ptr );
+ }
+}
+
+void
+Meta::Observer::subscribeTo( GenrePtr ptr )
+{
+ if( ptr ) {
+ ptr->subscribe( this );
+ m_genreSubscriptions.append( ptr );
+ }
+}
+
+void
+Meta::Observer::unsubscribeTo( GenrePtr ptr )
+{
+ if( ptr ) {
+ ptr->unsubscribe( this );
+ m_genreSubscriptions.remove( ptr );
+ }
+}
+
+void
+Meta::Observer::subscribeTo( YearPtr ptr )
+{
+ if( ptr ) {
+ ptr->subscribe( this );
+ m_yearSubscriptions.append( ptr );
+ }
+}
+
+void
+Meta::Observer::unsubscribeTo( YearPtr ptr )
+{
+ if( ptr ) {
+ ptr->unsubscribe( this );
+ m_yearSubscriptions.remove( ptr );
+ }
+}
+
+void
+Meta::Observer::subscribeTo( MagnatuneAlbum* ptr )
+{
+ if( ptr ) {
+ ptr->subscribe( this );
+ m_magnatuneAlbumSubscriptions.append( ptr );
+ }
+}
+
+void
+Meta::Observer::unsubscribeTo( MagnatuneAlbum* ptr )
+{
+ if( ptr ) {
+ ptr->unsubscribe( this );
+ m_magnatuneAlbumSubscriptions.remove( ptr );
+ }
+}
+
+void
Meta::Observer::metadataChanged( Track *track )
{
Q_UNUSED( track );
@@ -144,6 +294,8 @@
void
Meta::Track::notifyObservers() const
{
+ DEBUG_BLOCK
+
foreach( Observer *observer, m_observers )
{
if( m_observers.contains( observer ) ) // guard against observers removing themselves in destructors
@@ -174,6 +326,8 @@
void
Meta::Artist::notifyObservers() const
{
+ DEBUG_BLOCK
+
foreach( Observer *observer, m_observers )
{
if( m_observers.contains( observer ) ) // guard against observers removing themselves in destructors
@@ -198,6 +352,8 @@
void
Meta::Album::notifyObservers() const
{
+ DEBUG_BLOCK
+
foreach( Observer *observer, m_observers )
{
if( m_observers.contains( observer ) ) // guard against observers removing themselves in destructors
--- trunk/extragear/multimedia/amarok/src/meta/Meta.h #821448:821449
@@ -1,5 +1,6 @@
/* This file is part of the KDE project
Copyright (C) 2007 Maximilian Kossick <maximilian.kossick at googlemail.com>
+ Copyright (C) 2008 Mark Kretschmann <kretschmann at kde.org>
This program is free software; you can redistribute it and/or
modify it under the terms of the GNU General Public License
@@ -45,27 +46,42 @@
class Genre;
class Composer;
class Year;
+ class MagnatuneAlbum;
-
typedef KSharedPtr<MetaBase> DataPtr;
- typedef QList<DataPtr > DataList;
-
+ typedef QList<DataPtr> DataList;
typedef KSharedPtr<Track> TrackPtr;
- typedef QList<TrackPtr > TrackList;
+ typedef QList<TrackPtr> TrackList;
typedef KSharedPtr<Artist> ArtistPtr;
- typedef QList<ArtistPtr > ArtistList;
+ typedef QList<ArtistPtr> ArtistList;
typedef KSharedPtr<Album> AlbumPtr;
- typedef QList<AlbumPtr > AlbumList;
+ typedef QList<AlbumPtr> AlbumList;
typedef KSharedPtr<Composer> ComposerPtr;
typedef QList<ComposerPtr> ComposerList;
typedef KSharedPtr<Genre> GenrePtr;
- typedef QList<GenrePtr > GenreList;
+ typedef QList<GenrePtr> GenreList;
typedef KSharedPtr<Year> YearPtr;
- typedef QList<YearPtr > YearList;
+ typedef QList<YearPtr> YearList;
+ typedef QList<MagnatuneAlbum*> MagnatuneAlbumList;
class AMAROK_EXPORT Observer
{
public:
+ void subscribeTo( TrackPtr );
+ void unsubscribeTo( TrackPtr );
+ void subscribeTo( ArtistPtr );
+ void unsubscribeTo( ArtistPtr );
+ void subscribeTo( AlbumPtr );
+ void unsubscribeTo( AlbumPtr );
+ void subscribeTo( ComposerPtr );
+ void unsubscribeTo( ComposerPtr );
+ void subscribeTo( GenrePtr );
+ void unsubscribeTo( GenrePtr );
+ void subscribeTo( YearPtr );
+ void unsubscribeTo( YearPtr );
+ void subscribeTo( MagnatuneAlbum* );
+ void unsubscribeTo( MagnatuneAlbum* );
+
/** This method is called when the metadata of a track has changed.
The called class may not cache the pointer */
virtual void metadataChanged( Track *track );
@@ -75,10 +91,21 @@
virtual void metadataChanged( Composer *composer );
virtual void metadataChanged( Year *year );
virtual ~Observer();
+
+ private:
+ TrackList m_trackSubscriptions;
+ ArtistList m_artistSubscriptions;
+ AlbumList m_albumSubscriptions;
+ ComposerList m_composerSubscriptions;
+ GenreList m_genreSubscriptions;
+ YearList m_yearSubscriptions;
+ MagnatuneAlbumList m_magnatuneAlbumSubscriptions;
};
class AMAROK_EXPORT MetaBase : public QSharedData
{
+ friend class Observer;
+
public:
MetaBase() {}
virtual ~MetaBase() {}
@@ -94,9 +121,6 @@
virtual void addMatchTo( QueryMaker *qm ) = 0;
- virtual void subscribe( Observer *observer );
- virtual void unsubscribe( Observer *observer );
-
virtual bool hasCapabilityInterface( Meta::Capability::Type type ) const;
virtual Meta::Capability* asCapabilityInterface( Meta::Capability::Type type );
@@ -125,9 +149,11 @@
}
protected:
+ virtual void subscribe( Observer *observer );
+ virtual void unsubscribe( Observer *observer );
+
virtual void notifyObservers() const = 0;
- protected:
QSet<Meta::Observer*> m_observers;
private: // no copy allowed, since it's not safe with observer list
--- trunk/extragear/multimedia/amarok/src/meta/proxy/MetaProxy_p.h #821448:821449
@@ -93,7 +93,7 @@
Meta::TrackPtr track = newTrackProvider->trackForUrl( url );
if( track )
{
- track->subscribe( this );
+ subscribeTo( track );
realTrack = track;
notifyObservers();
disconnect( CollectionManager::instance(), SIGNAL( trackProviderAdded( TrackProvider* ) ), this, SLOT( slotNewTrackProvider( TrackProvider* ) ) );
--- trunk/extragear/multimedia/amarok/src/playlist/PlaylistModel.cpp #821448:821449
@@ -878,9 +878,9 @@
{
if( track )
{
- track->subscribe( this );
+ subscribeTo( track );
if( track->album() )
- track->album()->subscribe( this );
+ subscribeTo( track->album() );
m_items.insert( row + i, new Item( track ) );
i++;
@@ -920,9 +920,9 @@
for( int i = position; i < position + rows; i++ )
{
Item* item = m_items.takeAt( position ); //take at position, row times
- item->track()->unsubscribe( this );
+ unsubscribeTo( item->track() );
if( item->track()->album() )
- item->track()->album()->unsubscribe( this );
+ unsubscribeTo( item->track()->album() );
ret.push_back( item->track() );
delete item;
}
--- trunk/extragear/multimedia/amarok/src/servicebrowser/lastfm/meta/MultiPlayableCapabilityImpl_p.h #821448:821449
@@ -30,13 +30,12 @@
, m_url( track->playableUrl() )
, m_track( track )
{
- m_track->subscribe( this );
+ //FIXME
+ //subscribeTo( m_track );
}
virtual ~MultiPlayableCapabilityImpl()
- {
- m_track->unsubscribe( this );
- }
+ {}
virtual void fetchFirst() { m_track->playCurrent(); }
virtual void fetchNext() { m_track->playNext(); }
--- trunk/extragear/multimedia/amarok/src/servicebrowser/magnatunestore/MagnatunePurchaseDialog.cpp #821448:821449
@@ -38,6 +38,8 @@
: QDialog( parent, fl )
{
Q_UNUSED( modal );
+ DEBUG_BLOCK
+
setObjectName( name );
setupUi( this );
@@ -48,7 +50,9 @@
}
MagnatunePurchaseDialog::~MagnatunePurchaseDialog()
-{}
+{
+ DEBUG_BLOCK
+}
void MagnatunePurchaseDialog::setAlbum( MagnatuneAlbum * album )
@@ -62,10 +66,9 @@
m_albumCode = album->albumCode();
- album->subscribe( this );
+ subscribeTo( album );
coverPixmapLabel->setPixmap( album->image( 200, false ) );
-
}
void MagnatunePurchaseDialog::purchase( )
--- trunk/extragear/multimedia/amarok/src/widgets/TrackTooltip.cpp #821448:821449
@@ -93,10 +93,10 @@
return;
if( m_track->artist() )
- m_track->artist()->unsubscribe( this );
+ unsubscribeTo( m_track->artist() );
if( m_track->album() )
- m_track->album()->unsubscribe( this );
- m_track->unsubscribe( this );
+ unsubscribeTo( m_track->album() );
+ unsubscribeTo( m_track );
if( force || m_track != track )
{
@@ -185,12 +185,11 @@
updateWidgets();
- m_track->subscribe( this );
+ subscribeTo( m_track );
if( m_track->artist() )
- m_track->artist()->subscribe( this );
+ subscribeTo( m_track->artist() );
if( m_track->album() )
- m_track->album()->subscribe( this );
-
+ subscribeTo( m_track->album() );
}
}
More information about the Amarok-devel
mailing list