-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107473/
-----------------------------------------------------------
(Updated March 2, 2013, 6:06 p.m.)
Review request for Amarok.
Changes
-------
== Description ==
PlaylistFile::triggerTrackLoad implements async loading.
In some cases triggerTrackLoad uses as if it is synchronous
(e.g. DirectoryLoader, PlaylistController, PlaylistBrowserModel)
Common use case is:
playlist->triggerTrackLoad();
playlist->track()->count();
If playlist is object of PlaylistFile, then behaviour of code above
will differ from expectation.
I see two solutions of this problem:
1) For now perform async loading in trigerTackLoad only on request.
Thereby loading will be async only if it was requested:
playlist->makeLoadingAsync();
In the future support of async loading can be added everywhere
and code for sync loading support can be removed.
2) Add async loading support to each place where triggerTrackLoad
is called.
I ?hose first solution. If someone thinks that second solution
is better, then I need some additional time to figure out how to
implement async loading support in remaining cases.
For now two classes support async loading:
- PlaylistAction (via Restorer)
- DirectoryLoader (this support was added because of bug 291934)
== Tests ==
Passed all unit tests
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.
This addresses bug 291934.
https://bugs.kde.org/show_bug.cgi?id=291934
Diffs (updated)
-----
src/CMakeLists.txt 92650c3
src/DirectoryLoader.h e53a30d
src/DirectoryLoader.cpp bd7fa34
src/core-impl/playlists/types/file/PlaylistFile.h 4322da9
src/core-impl/playlists/types/file/PlaylistFile.cpp 2730354
src/core-impl/playlists/types/file/PlaylistFileLoaderJob.h PRE-CREATION
src/core-impl/playlists/types/file/PlaylistFileLoaderJob.cpp PRE-CREATION
src/core-impl/playlists/types/file/PlaylistFileSupport.cpp 2dcc0cd
src/core-impl/playlists/types/file/TestPlaylistObserver.h PRE-CREATION
src/core-impl/playlists/types/file/TestPlaylistObserver.cpp PRE-CREATION
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 8fe4b51
src/core-impl/playlists/types/file/xspf/XSPFPlaylist.h 6cf33e9
src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp bbcecc3
src/core/playlists/Playlist.h b4e4f40
src/core/playlists/Playlist.cpp 2d57e6b
src/playlist/PlaylistActions.h e8e541b
src/playlist/PlaylistActions.cpp 00bf13a
src/playlist/PlaylistRestorer.h PRE-CREATION
src/playlist/PlaylistRestorer.cpp PRE-CREATION
src/playlistmanager/PlaylistManager.h cbf65b0
src/playlistmanager/PlaylistManager.cpp 89c754b
src/playlistmanager/file/PlaylistFileProvider.cpp e4e7284
tests/TestDirectoryLoader.h 0583a9e
tests/TestDirectoryLoader.cpp b98247d
tests/core-impl/meta/multi/TestMetaMultiTrack.cpp 8e9e47d
tests/core-impl/playlists/types/file/CMakeLists.txt ef69236
tests/core-impl/playlists/types/file/TestPlaylistObserver.h PRE-CREATION
tests/core-impl/playlists/types/file/TestPlaylistObserver.cpp PRE-CREATION
tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp b25d059
tests/core-impl/playlists/types/file/pls/TestPLSPlaylist.cpp 5eab2f3
tests/core-impl/playlists/types/file/xspf/TestXSPFPlaylist.cpp 5ea3a41
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
