Bug 176170 - starts to fetch album covers automatically when starting the cover manager
Summary: starts to fetch album covers automatically when starting the cover manager
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 2.0-SVN
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-26 21:12 UTC by Pascal d'Hermilly
Modified: 2009-12-09 11:29 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pascal d'Hermilly 2008-11-26 21:12:16 UTC
Version:           1.98 (using KDE 4.1.3)
OS:                Linux
Installed from:    Ubuntu Packages

When I open the cover manager it automatically starts to fetch the covers for all my albums. I didn't press the "fetch missing covers" button
Comment 1 Seb Ruiz 2008-11-26 22:58:58 UTC
Yes, I've seen this behaviour. It doesn't make sense to automatically retrieve cover art when opening the cover manager as this causes lock downs when we try fetch more than a few images at once.
Comment 2 Dan Meltzer 2008-12-05 07:30:46 UTC
The simple fix would be to change CoverManager::loadCover() so that it only calls m_albumPtr->image() if hasImage() returns true.  If !hasImage() we'd want to manually set the album cover to our nocover image.
Comment 3 Dan Meltzer 2008-12-05 07:32:24 UTC
That'd be CoverViewItem, obviously.

Also would need to change the ctor to do the same thing.
Comment 4 Seb Ruiz 2008-12-05 08:06:17 UTC
Yes, this is a good workaround that I never considered. Good idea.
Comment 5 Seb Ruiz 2008-12-05 09:04:44 UTC
Actually on further investigation this won't work since hasImage() simply calls checks whether image() returns a valid QPixmap.
Comment 6 Dan Meltzer 2008-12-06 06:18:45 UTC
Well, the other solution that seems valid would be to not show the "Fetch missing covers" button if AmarokConfig::autoGetCoverArt() is set.  It seems acceptable to me to do this, the user did configure automatic fetching, after all.
Comment 7 Seb Ruiz 2009-04-13 02:07:39 UTC
After consideration, we should _never_ auto fetch all covers by using the inbuild SqlAlbum methods, because it does no queuing and does not know about any other fetch operations which are happening.
Comment 8 Dan Meltzer 2009-04-13 02:10:43 UTC
SqlMeta _does_ use queueing...

From the source:
  if( !m_name.isEmpty() && AmarokConfig::autoGetCoverArt() )
        CoverFetcher::instance()->queueAlbum( AlbumPtr(this) );
Comment 9 Seb Ruiz 2009-04-13 02:15:56 UTC
I guess it does. Regardless, I don't think that we should start an automatic global fetch for 1000 albums, for example.

It's fine to do it in the normal UI.
Comment 10 Seb Ruiz 2009-04-13 02:22:42 UTC
SVN commit 952966 by seb:

Don't automatically fetch all covers when opening the Cover Manager
BUG: 176170

 M  +2 -0      ChangeLog  
 M  +2 -1      src/collection/sqlcollection/SqlMeta.cpp  
 M  +3 -0      src/collection/sqlcollection/SqlMeta.h  
 M  +9 -1      src/covermanager/CoverManager.cpp  
 M  +6 -1      src/meta/Meta.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=952966