----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105524/ -----------------------------------------------------------
(Updated July 13, 2012, 11:53 a.m.) Review request for Amarok, Ralf Engels and Sven Krohlas. Changes ------- Significantly reworked the patch according to Ralf's and Bart's correct comments. The changes to tests we not needed at all as a result. Summary (updated) ----------------- EngineController: ditch canDecode(), fix supportedMimeTypes(): make it non-static, thread-safe even on first call. (squached patches, recent on top) Description (updated) ------- EngineController: always call Phonon::availableMimeTypes() from main thread 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 when they are deleted. 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 it (EngineController::supportedMimeTypes()) so that this fragile code (hopefully) never breaks again. EngineController: make supportedMimeTypes() non-static Static methods have no sense in a singleton class. Additionally, it was very hacky (and impossible in corner-cases) to keep it thread-safe. Making it non-static will allow us to do some tricks so that the calls are more robust. EngineController: null-pointer checking in destructor EngineController: remove unused and confusing destroy() EngineController: ditch canDecode() MetaFile::Track::isTrack() is partial replacement. Existing 3 calls to canDecode() were in fact all related to MetaFile classes, so move the method there, simplify it not to query phonon at all (and document it can return false positives). Works quite well for me and prevents failures in many tests. This change is propelled by Bart's and Ralf's legitimate comments on http://git.reviewboard.kde.org/r/105524/ Revert "Prevent hang in testmetamultitrack." This reverts commit 115cb80f9bd94b23640ca9245c97d6c8e25d2c97. Proper fix will follow, see next commits. Diffs (updated) ----- src/EngineController.h ad2e0c4a5e7c80c79bf448bf74cd6b52cd1f0ed3 src/EngineController.cpp 83f0a6ed0a92ae992e1809800cee65d9349dc680 src/browsers/filebrowser/FileBrowser.cpp 567ff799df7ef8bfcd93de73ee120bfc5be634b7 src/browsers/filebrowser/FileView.cpp 8eae4b6731ebfcb7310d3d719687989be125de92 src/core-impl/collections/support/CollectionManager.cpp 9085b5a27d9a7ce94d2325d94bec5fce8d126abe src/core-impl/meta/file/File.h caa7102fd901ccdac7c9f25ee0caeb554a4ee518 src/core-impl/meta/file/File.cpp 231c98521c3d6b1c609abc5799af47b88f048aee tests/CMakeLists.txt 901c716a600bca63639939f4e62dbd89d9db707f tests/TestEngineController.h PRE-CREATION tests/TestEngineController.cpp PRE-CREATION tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp 4bbf29be01aef9043a64075a196ac04a544134cb Diff: http://git.reviewboard.kde.org/r/105524/diff/ Testing ------- Works, fixes testm3uplaylist Thanks, Matěj Laitl
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
