----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105524/#review16127 -----------------------------------------------------------
This review has been submitted with commit 38497cca7a10d5858e2df14c1f1711fcac9dfd39 by Matěj Laitl to branch master. - Commit Hook On July 19, 2012, 1:38 p.m., Matěj Laitl wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105524/ > ----------------------------------------------------------- > > (Updated July 19, 2012, 1:38 p.m.) > > > Review request for Amarok, Bart Cerneels, Ralf Engels, and Sven Krohlas. > > > Description > ------- > > 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). > > As a consequence, we show all audio and video files in file browser and > in other places, even if they wouldn't be playable by the current > phonon back-end. This is arguably a cleaner approach and at least lets > users discover where the error is. > > 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/ > > BUG: 303253 > FIXED-IN: 2.6 > > Revert "Prevent hang in testmetamultitrack." > > This reverts commit 115cb80f9bd94b23640ca9245c97d6c8e25d2c97. > > Proper fix will follow, see next commits. > > > This addresses bug 303253. > https://bugs.kde.org/show_bug.cgi?id=303253 > > > Diffs > ----- > > ChangeLog 1d4f1e92fb5a033500c0f18d3dca257a89f1a139 > 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 > > Diff: http://git.reviewboard.kde.org/r/105524/diff/ > > > Testing > ------- > > Works, fixes testm3uplaylist, should fix bug 303253. (waiting for original > reporter, I cannot reproduce) > > > Thanks, > > Matěj Laitl > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
