Version: 1.4-SVN (using KDE KDE 3.4.3) Installed from: Ubuntu Packages Compiler: gcc 4.0.2 OS: Linux if you have a few tracks, whose artist-, album- and title tags are the same, but the tracknumber is different, they won't be transferred to an ipod. i don't know how it is with other media players. it seems to me, that amarok only checks against the artist, the album and the title of a track, to verfiy, that it doesn't exist already on an ipod. i've looked into the code and wrote a little patch, which i'll append to this bug report. but i think the method how remote (tracks on ipod) and local tracks are compared is not very well. to compare only a few tags seems a little buggy to me. wouldn't it be better, to generate something like a checksum of each track and take that afterwards for comparison? elias
Created attachment 13693 [details] add the track number for comparison, if a track exists on ipod or not
As soon as recoding songs before transferring them to the iPod is implemented, the only way to check for the presence of a song on the iPod is comparing the tags. So we will stick to this scheme for now. But comparing additional tags makes sense.
SVN commit 484472 by aumuell: also compare track number when checking if track is already present on ipod BUG: 117380 M +2 -0 ChangeLog M +18 -9 src/gpodmediadevice/gpodmediadevice.cpp M +1 -1 src/gpodmediadevice/gpodmediadevice.h M +8 -4 src/mediabrowser.cpp M +1 -1 src/mediabrowser.h --- trunk/extragear/multimedia/amarok/ChangeLog #484471:484472 @@ -59,6 +59,8 @@ compatibility with various iPod models. BUGFIXES: + * Also take track number into account when comparing tags for checking + if a track is already present on iPod. (BR 117380) * "Show Fullsize" now works for ID3 embedded cover images. (BR 114517) (BACKPORT?) * Fix possible bug when saving unencoded podcasts to strange file systems. --- trunk/extragear/multimedia/amarok/src/gpodmediadevice/gpodmediadevice.cpp #484471:484472 @@ -309,9 +309,10 @@ MediaItem * GpodMediaDevice::trackExists( const MetaBundle& bundle ) { - GpodMediaItem *item = getTitle( bundle.artist(), + GpodMediaItem *item = getTrack( bundle.artist(), bundle.album().isEmpty() ? i18n( "Unknown" ) : bundle.album(), - bundle.title()); + bundle.title(), + bundle.track() ); return item; } @@ -1074,14 +1075,18 @@ } GpodMediaItem * -GpodMediaDevice::getTitle(const QString &artist, const QString &album, const QString &title) +GpodMediaDevice::getTrack(const QString &artist, const QString &album, const QString &title, int trackNumber) { GpodMediaItem *item = getAlbum(artist, album); if(item) { - GpodMediaItem *track = dynamic_cast<GpodMediaItem *>(item->findItem(title)); - if(track) - return track; + for( GpodMediaItem *track = dynamic_cast<GpodMediaItem *>(item->findItem(title)); + track; + track = dynamic_cast<GpodMediaItem *>(item->findItem(title, track)) ) + { + if( trackNumber==-1 || track->bundle()->track() == trackNumber ) + return track; + } } if(m_podcastItem) @@ -1089,9 +1094,13 @@ item = dynamic_cast<GpodMediaItem *>(m_podcastItem->findItem(album)); if(item) { - GpodMediaItem *track = dynamic_cast<GpodMediaItem *>(item->findItem(title)); - if(track) - return track; + for( GpodMediaItem *track = dynamic_cast<GpodMediaItem *>(item->findItem(title)); + track; + track = dynamic_cast<GpodMediaItem *>(item->findItem(title, track)) ) + { + if( trackNumber==-1 || track->bundle()->track() == trackNumber ) + return track; + } } } --- trunk/extragear/multimedia/amarok/src/gpodmediadevice/gpodmediadevice.h #484471:484472 @@ -89,7 +89,7 @@ GpodMediaItem *getArtist(const QString &artist); GpodMediaItem *getAlbum(const QString &artist, const QString &album); - GpodMediaItem *getTitle(const QString &artist, const QString &album, const QString &title); + GpodMediaItem *getTrack(const QString &artist, const QString &album, const QString &title, int trackNumber=-1); bool removeDBTrack(Itdb_Track *track); --- trunk/extragear/multimedia/amarok/src/mediabrowser.cpp #484471:484472 @@ -462,11 +462,15 @@ } MediaItem * -MediaItem::findItem( const QString &key ) const +MediaItem::findItem( const QString &key, const MediaItem *after ) const { - for(MediaItem *it = dynamic_cast<MediaItem *>(firstChild()); - it; - it = dynamic_cast<MediaItem *>(it->nextSibling())) + MediaItem *it = 0; + if( after ) + it = dynamic_cast<MediaItem *>( after->nextSibling() ); + else + it = dynamic_cast<MediaItem *>( firstChild() ); + + for( ; it; it = dynamic_cast<MediaItem *>(it->nextSibling())) { if(key == it->text(0)) return it; --- trunk/extragear/multimedia/amarok/src/mediabrowser.h #484471:484472 @@ -48,7 +48,7 @@ void setType( Type type ); Type type() const { return m_type; } - MediaItem *findItem(const QString &key) const; + MediaItem *findItem(const QString &key, const MediaItem *after=0) const; virtual bool isLeafItem() const; // A leaf node of the tree virtual bool isFileBacked() const; // Should the file be deleted of the device when removed