Review Request: EngineController: fixes to canDecode() and supportedMimeTypes(): make them non-static, thread-safe even on first call. (squached patches, recent on top)
Matěj Laitl
matej at laitl.cz
Thu Jul 12 13:38:48 UTC 2012
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105524/
-----------------------------------------------------------
(Updated July 12, 2012, 1:38 p.m.)
Review request for Amarok, Ralf Engels and Sven Krohlas.
Changes
-------
Rebase on master, include revert.
Description (updated)
-------
EngineController: use QFileInfo instead of apparently non-reentrant KFileItem
...remote files are not needed here, so QFileInfo does a good job. This
is the cause of last crashes in testm3uplaylist. (I have about 25%
probability of it crashing until this commit)
EngineController: make supportedMimeTypes() thread-safe even on first call
If supportedMimeTypes() is called for the first time in a thread,
Phonon::BackendCapabilities::availableMimeTypes() creates some QObject
subclasses in non-main thread and that causes problems later on, and is
the cause of testm3uplaylist failures. Add signal/slot/QSemaphore
trickery that causes Phonon::BackendCapabilities::availableMimeTypes()
is called from the main thread without performance penalty for 2nd and
next calls.
Test is added for EngineController::supportedMimeTypes() so that this
fragile code (hopefully) never breaks again.
This fixes (perhaps along with the next commit) 2 failing tests (
testm3uplaylist and apparently testplaylistmodels), so the fail count
is down to 4 here.
EngineController: make canDecode() and supportedMimeTypes() non-static
Static methods have no sense in a singleton class. Additionally, it was
very hacky (and impossible in corner-cases) to keep these thread-safe,
see testm3uplaylist failures. Making them non-static will allow us to
do some tricks so that the calls are more robust.
Never-used and confusing destory() method is removed, too.
Care is taken not to break existing tests.
Revert "Prevent hang in testmetamultitrack."
This reverts commit 115cb80f9bd94b23640ca9245c97d6c8e25d2c97.
Proper fix will follow, see next commits.
Diffs (updated)
-----
tests/playlistmanager/sql/TestSqlUserPlaylistProvider.cpp bb09515d617e8149e7e66e804ff46aa5ddb76045
tests/core/meta/TestMetaTrack.cpp 0adb7633538533202c50487b1eaff8e4ead150e9
tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp 4bbf29be01aef9043a64075a196ac04a544134cb
src/core-impl/collections/support/CollectionManager.cpp 9085b5a27d9a7ce94d2325d94bec5fce8d126abe
tests/CMakeLists.txt 901c716a600bca63639939f4e62dbd89d9db707f
tests/TestEngineController.h PRE-CREATION
tests/TestEngineController.cpp PRE-CREATION
src/browsers/filebrowser/FileBrowser.cpp 567ff799df7ef8bfcd93de73ee120bfc5be634b7
src/browsers/filebrowser/FileView.cpp 8eae4b6731ebfcb7310d3d719687989be125de92
src/EngineController.cpp 83f0a6ed0a92ae992e1809800cee65d9349dc680
src/EngineController.h ad2e0c4a5e7c80c79bf448bf74cd6b52cd1f0ed3
Diff: http://git.reviewboard.kde.org/r/105524/diff/
Testing
-------
Works, fixes testm3uplaylist
Thanks,
Matěj Laitl
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120712/95e636ae/attachment.html>
More information about the Amarok-devel
mailing list