[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: [Amarok] Use MetaFile::Track for getting information (which
From:       Seb Ruiz <ruiz () kde ! org>
Date:       2009-08-03 23:23:36
Message-ID: 60ebdd0b0908031623r1ac57e3dw328fe58c6a6c304c () mail ! gmail ! com
[Download RAW message or body]

This change has some big side effects:

If you load a track with no metadata from the file system into the
playlist, we don't show anything in the playlist. We used to at least
show the url.


2009/8/4 Alejandro Wainzinger <aikawarazuni@gmail.com>:
> commit 20f4b4a6422790938b59e8b09240b18f722b5904
> Author:     Alejandro Wainzinger <aikawarazuni@gmail.com>
> AuthorDate: Mon Aug 3 16:01:34 2009 +0200
> Commit:     Alejandro Wainzinger <aikawarazuni@gmail.com>
> CommitDate: Mon Aug 3 16:04:13 2009 +0200
> 
> Use MetaFile::Track for getting information (which also uses TagLib) to reduce code \
> duplication.  Modify MetaFile::Track to not return modified strings if it has blank \
> strings for certain metadata, since this should be the responsibility of the user \
> of MetaFile::Track. 
> diff --git a/src/collection/ipodcollection/handler/IpodHandler.cpp \
> b/src/collection/ipodcollection/handler/IpodHandler.cpp index f9ecb60..1be1c26 \
>                 100644
> --- a/src/collection/ipodcollection/handler/IpodHandler.cpp
> +++ b/src/collection/ipodcollection/handler/IpodHandler.cpp
> @@ -429,7 +429,7 @@ IpodHandler::slotStaleOrphaned()
> 
> if( init )
> {
> -        ThreadWeaver::Weaver::instance()->enqueue( new StaleWorkerThread( this ) \
> ); +        ThreadWeaver::Weaver::instance()->enqueue( new OrphanedWorkerThread( \
> this ) ); }
> 
> 
> @@ -454,15 +454,15 @@ IpodHandler::findOrphaned()
> bool
> IpodHandler::addNextOrphaned()
> {
> +    DEBUG_BLOCK
> QString realPath;
> QString path = m_orphanedPaths.takeFirst();
> -    pathExists( path, &realPath );
> -    const AttributeHash attributes = readTags( realPath );
> -
> -    if( attributes.empty() )
> +    if( !pathExists( path, &realPath ) )
> return false;
> 
> -    debug() << "Found: " << attributes["artist"] << " - " << attributes["title"];
> +    // Create track based on URL
> +
> +    Meta::TrackPtr filetrack( new MetaFile::Track( realPath ) );
> 
> // Create new track
> 
> @@ -472,34 +472,9 @@ IpodHandler::addNextOrphaned()
> 
> libCreateTrack( destTrack );
> 
> -    // Fill the track struct of the destTrack with info from the track parameter \
>                 as source
> -
> -    libSetTitle( destTrack, attributes["title"] );
> -    libSetArtist( destTrack, attributes["artist"] );
> -    libSetAlbum( destTrack, attributes["album"] );
> -    libSetComment( destTrack, attributes["comment"] );
> -    libSetGenre( destTrack, attributes["genre"] );
> -    libSetYear( destTrack, attributes["year"] );
> -    libSetTrackNumber( destTrack, attributes["track"].toInt() );
> +    // Fill the track struct of the destTrack with info from the filetrack as \
> source 
> -    if( attributes.contains("composer" ) )
> -        libSetComposer( destTrack, attributes["composer"] );
> -    if( attributes.contains("discnumber" ) )
> -        libSetDiscNumber( destTrack, attributes["discnumber"].toInt() );
> -
> -    //libSetBpm( destTrack, attributes["bpm"] );
> -    if( attributes.contains("filesize" ) )
> -        libSetFileSize( destTrack, attributes["filesize"].toInt() );
> -
> -    libSetType( destTrack, attributes["filetype"] );
> -    //libSetPlayableUrl( destTrack, srcTrack );
> -
> -    if( attributes["audioproperties"] == "true" )
> -    {
> -        libSetBitrate( destTrack, attributes["bitrate"].toInt() );
> -        libSetLength( destTrack, attributes["length"].toInt() );
> -        libSetSamplerate( destTrack, attributes["samplerate"].toInt() );
> -    }
> +    setBasicMediaDeviceTrackInfo( filetrack, destTrack );
> 
> // set up the play url
> 
> @@ -509,7 +484,7 @@ IpodHandler::addNextOrphaned()
> 
> addTrackInDB( destTrack );
> 
> -    // Inform subclass that a track has been added to the db
> +    // A track has been added to the db
> 
> databaseChanged();
> 
> diff --git a/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp \
> b/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp index \
>                 35fedbe..28c9c48 100644
> --- a/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp
> +++ b/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp
> @@ -107,7 +107,11 @@ MediaDeviceHandler::getBasicMediaDeviceTrackInfo( const \
> Meta::MediaDeviceTrackPt void
> MediaDeviceHandler::setBasicMediaDeviceTrackInfo( const Meta::TrackPtr& srcTrack, \
> MediaDeviceTrackPtr destTrack ) {
> -    DEBUG_BLOCK
> +    setupWriteCapability();
> +
> +    if( !m_wc )
> +        return;
> +
> m_wc->libSetTitle( destTrack, srcTrack->name() );
> m_wc->libSetAlbum( destTrack, srcTrack->album()->name() );
> m_wc->libSetArtist( destTrack, srcTrack->artist()->name() );
> @@ -127,6 +131,7 @@ MediaDeviceHandler::setBasicMediaDeviceTrackInfo( const \
> Meta::TrackPtr& srcTrack m_wc->libSetRating( destTrack, srcTrack->rating() );
> m_wc->libSetType( destTrack, srcTrack->type() );
> //libSetPlayableUrl( destTrack, srcTrack );
> +
> //if( srcTrack->album()->hasImage() )
> //    libSetCoverArt( destTrack, srcTrack->album()->image() );
> }
> @@ -264,18 +269,10 @@ MediaDeviceHandler::copyTrackListToDevice(const \
> Meta::TrackList tracklist) 
> DEBUG_BLOCK
> 
> +    setupWriteCapability();
> +
> if( !m_wc )
> -    {
> -        if( this->hasCapabilityInterface( Handler::Capability::Writable ) )
> -        {
> -            m_wc = this->create<Handler::WriteCapability>();
> -            if( !m_wc )
> -            {
> -                debug() << "Handler does not have \
>                 MediaDeviceHandler::WriteCapability. Aborting copy.";
> -                return;
> -            }
> -        }
> -    }
> +        return;
> 
> m_isCopying = true;
> 
> @@ -1073,15 +1070,9 @@ MediaDeviceHandler::setupReadCapability()
> }
> }
> 
> -/** Observer Methods **/
> void
> -MediaDeviceHandler::metadataChanged( TrackPtr track )
> +MediaDeviceHandler::setupWriteCapability()
> {
> -    DEBUG_BLOCK
> -
> -    Meta::MediaDeviceTrackPtr trackPtr = Meta::MediaDeviceTrackPtr::staticCast( \
>                 track );
> -    KUrl trackUrl = KUrl::fromPath( trackPtr->uidUrl() );
> -
> if( !m_wc )
> {
> if( this->hasCapabilityInterface( Handler::Capability::Writable ) )
> @@ -1089,11 +1080,26 @@ MediaDeviceHandler::metadataChanged( TrackPtr track )
> m_wc = this->create<Handler::WriteCapability>();
> if( !m_wc )
> {
> -                debug() << "Handler does not have \
> MediaDeviceHandler::WriteCapability. Aborting metadata change."; +                \
> debug() << "Handler does not have MediaDeviceHandler::WriteCapability. Aborting."; \
> return; }
> }
> }
> +}
> +
> +/** Observer Methods **/
> +void
> +MediaDeviceHandler::metadataChanged( TrackPtr track )
> +{
> +    DEBUG_BLOCK
> +
> +    Meta::MediaDeviceTrackPtr trackPtr = Meta::MediaDeviceTrackPtr::staticCast( \
> track ); +    KUrl trackUrl = KUrl::fromPath( trackPtr->uidUrl() );
> +
> +    setupWriteCapability();
> +
> +    if( !m_wc )
> +        return;
> 
> setBasicMediaDeviceTrackInfo( track, trackPtr );
> 
> diff --git a/src/collection/mediadevicecollection/handler/MediaDeviceHandler.h \
> b/src/collection/mediadevicecollection/handler/MediaDeviceHandler.h index \
>                 2dcf116..9fa8ebd 100644
> --- a/src/collection/mediadevicecollection/handler/MediaDeviceHandler.h
> +++ b/src/collection/mediadevicecollection/handler/MediaDeviceHandler.h
> @@ -245,6 +245,14 @@ protected:
> 
> void addMediaDeviceTrackToCollection( Meta::MediaDeviceTrackPtr &track );
> 
> +    /** Uses wrapped libSet methods to fill a track struct of the particular \
> library +    *  with information from a Meta::Track
> +    *  @param srcTrack The track that has the source information
> +    *  @param destTrack The track whose associated struct we want to fill with \
> information +    */
> +
> +    void setBasicMediaDeviceTrackInfo( const Meta::TrackPtr &srcTrack, \
> Meta::MediaDeviceTrackPtr destTrack ); +
> MediaDeviceCollection   *m_memColl; /// Associated collection
> ProgressBar      *m_statusbar; /// A progressbar to show progress of an operation
> 
> @@ -283,14 +291,6 @@ private:
> */
> void getBasicMediaDeviceTrackInfo( const Meta::MediaDeviceTrackPtr& track, \
> Meta::MediaDeviceTrackPtr destTrack ); 
> -    /** Uses wrapped libSet methods to fill a track struct of the particular \
>                 library
> -    *  with information from a Meta::Track
> -    *  @param srcTrack The track that has the source information
> -    *  @param destTrack The track whose associated struct we want to fill with \
>                 information
> -    */
> -
> -    void setBasicMediaDeviceTrackInfo( const Meta::TrackPtr &srcTrack, \
>                 Meta::MediaDeviceTrackPtr destTrack );
> -
> /**
> * Pulls out meta information (e.g. artist string)
> * from track struct, inserts into appropriate map
> @@ -312,6 +312,7 @@ private:
> // Misc. Helper Methods
> 
> void setupReadCapability();
> +    void setupWriteCapability();
> 
> /**
> *  @return free space on the device
> diff --git a/src/meta/file/File.cpp b/src/meta/file/File.cpp
> index aee21ce..ef826bf 100644
> --- a/src/meta/file/File.cpp
> +++ b/src/meta/file/File.cpp
> @@ -160,10 +160,7 @@ Track::name() const
> if( d )
> {
> const QString trackName = d->m_data.title;
> -        if( !trackName.isEmpty() )
> -            return trackName;
> -        //lets use the filename, or it will look really dull in the playlist
> -        return d->url.fileName();
> +        return trackName;
> }
> return "This is a bug!";
> }
> @@ -222,7 +219,7 @@ Track::isEditable() const
> const bool editable = ( p & QFile::WriteUser ) || ( p & QFile::WriteGroup ) || ( p \
> & QFile::WriteOther ); 
> debug() << d->url.path() << " editable: " << editable;
> -    return editable;
> +    return editable;
> }
> 
> Meta::AlbumPtr
> @@ -566,10 +563,10 @@ Track::collection() const
> bool
> Track::hasCapabilityInterface( Meta::Capability::Type type ) const
> {
> -    return type == Meta::Capability::Editable ||
> -           type == Meta::Capability::Importable ||
> -           type == Meta::Capability::CurrentTrackActions ||
> -           type == Meta::Capability::WriteTimecode ||
> +    return type == Meta::Capability::Editable ||
> +           type == Meta::Capability::Importable ||
> +           type == Meta::Capability::CurrentTrackActions ||
> +           type == Meta::Capability::WriteTimecode ||
> type == Meta::Capability::LoadTimecode;
> }
> 
> diff --git a/src/meta/file/File_p.h b/src/meta/file/File_p.h
> index 237499a..27df463 100644
> --- a/src/meta/file/File_p.h
> +++ b/src/meta/file/File_p.h
> @@ -312,9 +312,7 @@ public:
> QString name() const
> {
> const QString artist = d->m_data.artist;
> -        if( !artist.isEmpty() )
> -            return artist;
> -        return i18nc( "The value is not known", "Unknown" );
> +        return artist;
> }
> 
> QString prettyName() const
> @@ -358,13 +356,10 @@ public:
> if( d )
> {
> const QString albumName = d->m_data.album;
> -            if( !albumName.isEmpty() )
> -                return albumName;
> -            else
> -                return i18nc( "The value is not known", "Unknown" );
> +            return albumName;
> }
> else
> -            return i18nc( "The value is not known", "Unknown" );
> +            return QString();
> }
> 
> QString prettyName() const
> @@ -400,9 +395,7 @@ public:
> QString name() const
> {
> const QString genreName = d->m_data.genre;
> -        if( !genreName.isEmpty() )
> -            return genreName;
> -        return i18nc( "The value is not known", "Unknown" );
> +        return genreName;
> }
> 
> QString prettyName() const
> @@ -429,10 +422,8 @@ public:
> QString name() const
> {
> const QString composer = d->m_data.composer;
> -        if( !composer.isEmpty() )
> -            return composer;
> -        return i18nc( "The value is not known", "Unknown" );
> -    }
> +        return composer;
> +     }
> 
> QString prettyName() const
> {
> @@ -458,9 +449,7 @@ public:
> QString name() const
> {
> const QString year = QString::number( d->m_data.year );
> -        if( !year.isEmpty()  )
> -            return year;
> -        return i18nc( "The value is not known", "Unknown" );
> +        return year;
> }
> 
> QString prettyName() const
> 
> 
> 



-- 
Seb Ruiz

http://www.sebruiz.net/
http://amarok.kde.org/


[prev in list] [next in list] [prev in thread] [next in thread]