extragear/multimedia/amarok/src

Casey Link unnamedrambler at gmail.com
Fri Aug 15 16:59:55 CEST 2008


SVN commit 847525 by link:

This colossal commit enables async trackForUrl support for mp3tunes, however to achieve this MetaProxy was hacked to allow for selective updating of the proxied track. Due to the inheritance hierarchy in the design a chicken and egg problem developed where MetaProxy::name() would call itself causing infinite recursion. The solution was to add a "forwardToProxy" flag to MetaBase that by default does nothing, but is implemented in MetaProxy and ServiceMetaBase to allow for selective getting of cached or real metadata.

CCMAIL: maximilian.kossick at g­ooglemail.com
CCMAIL: amarok-devel at kde.org
BUG: 161840


 M  +30 -2     browsers/servicebrowser/ServiceMetaBase.cpp  
 M  +6 -0      browsers/servicebrowser/ServiceMetaBase.h  
 M  +2 -2      browsers/servicebrowser/mp3tunes/Mp3tunesServiceCollection.cpp  
 M  +9 -0      meta/Meta.h  
 M  +65 -33    meta/proxy/MetaProxy.cpp  
 M  +6 -1      meta/proxy/MetaProxy.h  
 M  +19 -2     meta/proxy/MetaProxy_p.h  


--- trunk/extragear/multimedia/amarok/src/browsers/servicebrowser/ServiceMetaBase.cpp #847524:847525
@@ -146,8 +146,9 @@
     , m_albumName( 0 )
     , m_artistId( 0 )
     , m_artistName( 0 )
+    , m_forwardToProxy( true )
 {
-    setName( name );
+    setTitle( name );
 }
 
 ServiceTrack::ServiceTrack( const QString & url )
@@ -169,6 +170,7 @@
     , m_albumName( 0 )
     , m_artistId( 0 )
     , m_artistName( 0 )
+    , m_forwardToProxy( true )
 {
 }
 
@@ -181,9 +183,10 @@
     , m_genre( 0 )
     , m_composer( 0 )
     , m_year( 0 )
+    , m_forwardToProxy( true )
 {
     m_id = resultRow[0].toInt();
-    setName( resultRow[1] );
+    setTitle( resultRow[1] );
     m_trackNumber = resultRow[2].toInt();
     m_length = resultRow[3].toInt();
     m_displayUrl = resultRow[4];
@@ -207,6 +210,14 @@
 }
 
 void
+ServiceTrack::update( Meta::TrackPtr track )
+{
+    DEBUG_BLOCK
+    updateTrack( track );
+}
+
+
+void
 ServiceTrack::setId(int id)
 {
     m_id = id;
@@ -243,6 +254,16 @@
 }
 
 QString
+ServiceTrack::name() const
+{
+    DEBUG_BLOCK
+    if( m_forwardToProxy )
+        return MetaProxy::Track::name();
+
+    return m_name;
+}
+
+QString
 ServiceTrack::prettyName() const
 {
     return name();
@@ -288,6 +309,11 @@
     m_downloadableUrl = url;
 }
 
+void ServiceTrack::setForwardToProxy( bool forward )
+{
+    m_forwardToProxy = forward;
+}
+
 bool
 ServiceTrack::isPlayable() const
 {
@@ -512,6 +538,8 @@
 void
 ServiceTrack::setTitle( const QString &title )
 {
+    DEBUG_BLOCK
+    m_name = title;
     setName( title );
 }
 
--- trunk/extragear/multimedia/amarok/src/browsers/servicebrowser/ServiceMetaBase.h #847524:847525
@@ -143,11 +143,13 @@
         ServiceTrack( const QStringList & resultRow );
         virtual ~ServiceTrack();
 
+        virtual QString name() const;
         virtual QString prettyName() const;
         virtual KUrl downloadableUrl() const;
         virtual KUrl playableUrl() const;
         virtual QString uidUrl() const;
         virtual QString prettyUrl() const;
+        virtual void  setForwardToProxy( bool forward );
 
         virtual bool isPlayable() const;
         virtual bool isEditable() const;
@@ -239,6 +241,7 @@
         void setUidUrl( const QString &url );
         void setDownloadableUrl( const QString &url );
         void refresh( TrackProvider *provider );
+        void update( Meta::TrackPtr track );
 
     private:
         ArtistPtr m_artist;
@@ -257,7 +260,10 @@
         QString m_albumName;
         int m_artistId;
         QString m_artistName;
+        QString m_name;
 
+        bool m_forwardToProxy;
+
 //         QString m_type;
 };
 
--- trunk/extragear/multimedia/amarok/src/browsers/servicebrowser/mp3tunes/Mp3tunesServiceCollection.cpp #847524:847525
@@ -115,7 +115,7 @@
     
     //Building a Meta::Track
     QString title = track.trackTitle().isEmpty() ? "Unknown" :  track.trackTitle();
-    serviceTrack->setName( title );
+    serviceTrack->setTitle( title );
     serviceTrack->setId( track.trackId() );
     serviceTrack->setUidUrl( track.playUrl() ); //was: setUrl
     serviceTrack->setDownloadableUrl( track.downloadUrl() );
@@ -148,7 +148,7 @@
     serviceTrack->setArtist( artistPtr );
     serviceAlbum->setArtistName( name );
     serviceAlbum->setAlbumArtist( artistPtr );
-    serviceTrack->refresh( service()->parent() );
+    serviceTrack->update( Meta::TrackPtr( serviceTrack ) );
 }
 
 Mp3tunesLocker* Mp3tunesServiceCollection::locker() const
--- trunk/extragear/multimedia/amarok/src/meta/Meta.h #847524:847525
@@ -110,6 +110,15 @@
             MetaBase() {}
             virtual ~MetaBase() {}
             virtual QString name() const = 0;
+
+            /**
+             * Warning HACK! HACK!
+             * This method is used by the MetaProxy class to ensure
+             * that the getter methods of the real Meta item (track, album, artist, etc.)
+             * does not call the corresponding method on the proxy (if there is one), because this
+             * would result in infinite recursion. Did that make sense?
+             */
+            virtual void    setForwardToProxy( bool forward ) { Q_UNUSED(forward) }; 
             virtual QString prettyName() const = 0;
             virtual QString fullPrettyName() const { return prettyName(); }
             virtual QString sortableName() const { return prettyName(); }
--- trunk/extragear/multimedia/amarok/src/meta/proxy/MetaProxy.cpp #847524:847525
@@ -107,10 +107,14 @@
 QString
 MetaProxy::Track::name() const
 {
-    if( d->realTrack )
-        return d->realTrack->name();
-    else
-        return d->cachedName;
+    DEBUG_BLOCK
+    if( d->realTrack ) {
+        d->realTrack->setForwardToProxy( false );
+        QString name = d->realTrack->name();
+        d->realTrack->setForwardToProxy( true );
+        return name;
+    } 
+    return d->cachedName;
 }
 
 void
@@ -122,65 +126,86 @@
 QString
 MetaProxy::Track::prettyName() const
 {
-    if( d->realTrack )
-        return d->realTrack->prettyName();
-    else
-        return d->cachedName;   //TODO maybe change this?
+    if( d->realTrack ) {
+        d->realTrack->setForwardToProxy( false );
+        QString prettyName = d->realTrack->prettyName();
+        d->realTrack->setForwardToProxy( true );
+        return prettyName;
+    } 
+    return d->cachedName;   //TODO maybe change this?
 }
 
 QString
 MetaProxy::Track::fullPrettyName() const
 {
-    if( d->realTrack )
-        return d->realTrack->fullPrettyName();
-    else
-        return d->cachedName;   //TODO maybe change this??
+    if( d->realTrack ) {
+        d->realTrack->setForwardToProxy( false );
+        QString fullPrettyName = d->realTrack->fullPrettyName();
+        d->realTrack->setForwardToProxy( true );
+        return fullPrettyName;
+    } 
+    return d->cachedName;   //TODO maybe change this??
 }
 
 QString
 MetaProxy::Track::sortableName() const
 {
-    if( d->realTrack )
-        return d->realTrack->sortableName();
-    else
-        return d->cachedName;   //TODO change this?
+    if( d->realTrack ) {
+        d->realTrack->setForwardToProxy( false );
+        QString sortableName = d->realTrack->sortableName();
+        d->realTrack->setForwardToProxy( true );
+        return sortableName;
+    }
+    return d->cachedName;   //TODO maybe change this??
 }
 
 KUrl
 MetaProxy::Track::playableUrl() const
 {
-    if( d->realTrack )
-        return d->realTrack->playableUrl();
-    else
-//         return KUrl();
-        return d->url; // Maybe?
+    if( d->realTrack ) {
+        d->realTrack->setForwardToProxy( false );
+        KUrl playableUrl = d->realTrack->playableUrl();
+        d->realTrack->setForwardToProxy( true );
+        return playableUrl;
+    }
+    //return KUrl();
+    return d->url; // Maybe?
 }
 
 QString
 MetaProxy::Track::prettyUrl() const
 {
-    if( d->realTrack )
-        return d->realTrack->prettyUrl();
-    else
-        return d->url.url();
+    if( d->realTrack ) {
+        d->realTrack->setForwardToProxy( false );
+        QString prettyUrl = d->realTrack->prettyUrl();
+        d->realTrack->setForwardToProxy( true );
+        return prettyUrl;
+    }
+    return d->url.url();
 }
 
 QString
 MetaProxy::Track::uidUrl() const
 {
-    if( d->realTrack )
-        return d->realTrack->uidUrl();
-    else
-        return d->url.url();
+    if( d->realTrack ) {
+        d->realTrack->setForwardToProxy( false );
+        QString uidUrl = d->realTrack->uidUrl();
+        d->realTrack->setForwardToProxy( true );
+        return uidUrl;
+    }
+    return d->url.url();
 }
 
 bool
 MetaProxy::Track::isPlayable() const
 {
-    if( d->realTrack )
-        return d->realTrack->isPlayable();
-    else
-        return false;
+    if( d->realTrack ) {
+        d->realTrack->setForwardToProxy( false );
+        bool isPlayable = d->realTrack->isPlayable();
+        d->realTrack->setForwardToProxy( true );
+        return isPlayable;
+    }
+    return false;
 }
 
 Meta::AlbumPtr
@@ -421,6 +446,13 @@
 	d->slotNewTrackProvider( provider );
 }
 
+void
+MetaProxy::Track::updateTrack( Meta::TrackPtr track )
+{
+    DEBUG_BLOCK
+    d->slotUpdateTrack( track );
+}
+
 bool
 MetaProxy::Track::hasCapabilityInterface( Meta::Capability::Type type ) const
 {
--- trunk/extragear/multimedia/amarok/src/meta/proxy/MetaProxy.h #847524:847525
@@ -103,10 +103,15 @@
 		 */
 		Track( const KUrl &url, bool awaitLookupNotification);
 		/**
-		 * MetaProxy wil check the given trackprovider if it can provide the track for the proxy's url.
+		 * MetaProxy will check the given trackprovider if it can provide the track for the proxy's url.
 		 */
 		void lookupTrack(TrackProvider *provider);
 
+        /**
+         * MetaProxy will update the proxy with the track.
+         */
+        void updateTrack( Meta::TrackPtr track );
+
         private:
 			void init( const KUrl &url, bool awaitLookupNotification );
             Private * const d;
--- trunk/extragear/multimedia/amarok/src/meta/proxy/MetaProxy_p.h #847524:847525
@@ -70,19 +70,26 @@
     public:
         void notifyObservers()
         {
-            foreach( Meta::Observer *observer, observers )
-                observer->metadataChanged( proxy );
+            DEBUG_BLOCK
+            if( proxy )
+            {
+                foreach( Meta::Observer *observer, observers ) {
+                    if( observer != this ) observer->metadataChanged( proxy );
+                }
+            }
         }
         using Observer::metadataChanged;
         void metadataChanged( Meta::Track *track )
         {
             Q_UNUSED( track )
+            DEBUG_BLOCK
             notifyObservers();
         }
 
     public slots:
         void slotNewTrackProvider( TrackProvider *newTrackProvider )
         {
+            DEBUG_BLOCK
             if ( !newTrackProvider )
             {
                 return;
@@ -101,6 +108,16 @@
             }
         }
 
+        void slotUpdateTrack( Meta::TrackPtr track )
+        {
+            DEBUG_BLOCK
+            if( track )
+            {
+                subscribeTo( track );
+                realTrack = track;
+                notifyObservers();
+            }
+        }
         void slotCheckCollectionManager()
         {
             Meta::TrackPtr track = CollectionManager::instance()->trackForUrl( url );


More information about the Amarok-devel mailing list