> On Nov. 27, 2012, 8:08 a.m., Bart Cerneels wrote: > > src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp, line 218 > > <http://git.reviewboard.kde.org/r/107473/diff/1/?file=96233#file96233line218> > > > > Here is the real reason why lazy loading is needed. Reading > > playlist-file contents from disk is too fast to bother, but getting the > > right Track objects from CollectionManager::trackForUrl() is problematic. > > It should either block until the track is ready (depends in TrackProviders) > > or using proxy loading like it is now. The proxy is temporary and gets > > initialized with metadata we know from the contents of this playlist file.
That is a totally wrong assumption, don't you remember that we introduced it because reading playlists on start-up is a PITA? Please don't assume that all users have their collection and playlists on an SSD. Playlists should never be read unless you actually load it in the play queue, not a second earlier. We reduced start up time by over a minute, and some users reported start up delays of several minutes because playlist were read on start. See also https://bugs.kde.org/show_bug.cgi?id=245654, you introduced that yourself :) - Myriam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review22621 ----------------------------------------------------------- On Nov. 26, 2012, 12:10 p.m., Tatjana Gornak wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107473/ > ----------------------------------------------------------- > > (Updated Nov. 26, 2012, 12:10 p.m.) > > > Review request for Amarok. > > > Description > ------- > > I've started my changes with an implementation of a ASXPlaylist class, > due this work I've found that implementation of different playlist file > types has different logic, as result I end up with a rewriting > playlist's implementations. > > As example of difference: > -- Constructor M3UPlaylist::M3UPlaylist( const KUrl &url ) sets a url, but > does not load playlist, therefore loading of playlist are > posponed till data from playlist are actualy needed (lazy loading) > On the other hand constructor XSPFPlaylist::XSPFPlaylist( const > KUrl &url, bool autoAppend ) loads playlist. > > The main idea of proposed changes is to create a unify code for > processing playlist files: > -- lazy loading through LazyLoadPlaylist > -- common functionality was moved to PlaylistFile. > > It is worth to be noticed that this patch changes structure of > playlist's classes, bit does not changes their behavior, even if this behavior > is inconsistent in some cases. > > In following patch-requests I plan to submit ASX Playlist parser and > organize playlists processing in more consistent way. > > > Diffs > ----- > > src/CMakeLists.txt b70147d > src/core-impl/playlists/types/file/LazyPlaylistFile.h PRE-CREATION > src/core-impl/playlists/types/file/LazyPlaylistFile.cpp PRE-CREATION > src/core-impl/playlists/types/file/PlaylistFile.h 4322da9 > src/core-impl/playlists/types/file/PlaylistFile.cpp 2730354 > src/core-impl/playlists/types/file/PlaylistFileSupport.cpp 2dcc0cd > src/core-impl/playlists/types/file/m3u/M3UPlaylist.h 3176c45 > src/core-impl/playlists/types/file/m3u/M3UPlaylist.cpp 2395260 > src/core-impl/playlists/types/file/pls/PLSPlaylist.h e974539 > src/core-impl/playlists/types/file/pls/PLSPlaylist.cpp 3ba154a > src/core-impl/playlists/types/file/xspf/XSPFPlaylist.h ebf3235 > src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp bbcecc3 > src/core/playlists/Playlist.h 5cf11f2 > src/playlistmanager/file/PlaylistFileProvider.cpp e4e7284 > tests/core-impl/playlists/types/file/CMakeLists.txt ef69236 > tests/core-impl/playlists/types/file/TestLazyPlaylistFile.h PRE-CREATION > tests/core-impl/playlists/types/file/TestLazyPlaylistFile.cpp PRE-CREATION > tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp 0f19e1b > > Diff: http://git.reviewboard.kde.org/r/107473/diff/ > > > Testing > ------- > > 1) All unit-tests were passed. > 2) For all playlists I've also checked loading and > saving through gui. > > > Thanks, > > Tatjana Gornak > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
