<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/105524/">http://git.reviewboard.kde.org/r/105524/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been submitted with commit 38497cca7a10d5858e2df14c1f1711fcac9dfd39 by Matěj Laitl to branch master.</pre>
<br />
<p>- Commit</p>
<br />
<p>On July 19th, 2012, 1:38 p.m., Matěj Laitl wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok, Bart Cerneels, Ralf Engels, and Sven Krohlas.</div>
<div>By Matěj Laitl.</div>
<p style="color: grey;"><i>Updated July 19, 2012, 1:38 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Works, fixes testm3uplaylist, should fix bug 303253. (waiting for original reporter, I cannot reproduce)</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=303253">303253</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>ChangeLog <span style="color: grey">(1d4f1e92fb5a033500c0f18d3dca257a89f1a139)</span></li>
<li>src/EngineController.h <span style="color: grey">(ad2e0c4a5e7c80c79bf448bf74cd6b52cd1f0ed3)</span></li>
<li>src/EngineController.cpp <span style="color: grey">(83f0a6ed0a92ae992e1809800cee65d9349dc680)</span></li>
<li>src/browsers/filebrowser/FileBrowser.cpp <span style="color: grey">(567ff799df7ef8bfcd93de73ee120bfc5be634b7)</span></li>
<li>src/browsers/filebrowser/FileView.cpp <span style="color: grey">(8eae4b6731ebfcb7310d3d719687989be125de92)</span></li>
<li>src/core-impl/collections/support/CollectionManager.cpp <span style="color: grey">(9085b5a27d9a7ce94d2325d94bec5fce8d126abe)</span></li>
<li>src/core-impl/meta/file/File.h <span style="color: grey">(caa7102fd901ccdac7c9f25ee0caeb554a4ee518)</span></li>
<li>src/core-impl/meta/file/File.cpp <span style="color: grey">(231c98521c3d6b1c609abc5799af47b88f048aee)</span></li>
<li>tests/CMakeLists.txt <span style="color: grey">(901c716a600bca63639939f4e62dbd89d9db707f)</span></li>
<li>tests/TestEngineController.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/TestEngineController.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105524/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>