<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/100065/">http://git.reviewboard.kde.org/r/100065/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/100065/diff/1/?file=948#file948line25" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/upnpcollection/UpnpCache.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">25</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">typedef</span> <span class="n">QMap</span><span class="o"><</span><span class="n">QString</span><span class="p">,</span> <span class="n">Meta</span><span class="o">::</span><span class="n">TrackPtr</span><span class="o">></span> <span class="n">TrackMap</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is the order of these types really important? This code would be faster with a QHash instead.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/100065/diff/1/?file=951#file951line60" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/upnpcollection/UpnpCollectionBase.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString UpnpCollectionBase::collectionId() const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">60</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">QString</span><span class="p">(</span><span class="s">"upnp-ms://"</span><span class="p">)</span> <span class="o">+</span> <span class="n">m_device</span><span class="p">.</span><span class="n">uuid</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">should be QLatin1Literal instead</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/100065/diff/1/?file=951#file951line70" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/upnpcollection/UpnpCollectionBase.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString UpnpCollectionBase::collectionId() const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">70</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">url</span><span class="p">.</span><span class="n">scheme</span><span class="p">()</span> <span class="o">==</span> <span class="s">"upnp-ms"</span> <span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">QLatin1Literal</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/100065/diff/1/?file=953#file953line58" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/upnpcollection/UpnpCollectionFactory.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void UpnpCollectionFactory::init()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">58</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">bus</span><span class="p">.</span><span class="n">connect</span><span class="p">(</span> <span class="s">"org.kde.Cagibi"</span><span class="p">,</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">QLatin1Litera</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/100065/diff/1/?file=953#file953line100" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/upnpcollection/UpnpCollectionFactory.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void UpnpCollectionFactory::init()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">100</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">type</span><span class="p">.</span><span class="n">startsWith</span><span class="p">(</span><span class="s">"urn:schemas-upnp-org:device:MediaServer"</span><span class="p">)</span> <span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">QLatin1Literal
(etc, etc, basically whenever you have a literal string being pased to a method that expects a QString, you should use i18n or QLatin1Literal. In this way, QString doesn't have to guess the text encoding at runtime. Occasionally QLatin1Literal doesn't work, then use QLatin1String.)</pre>
</div>
<br />
<p>- Ian</p>
<br />
<p>On October 17th, 2010, 11:30 a.m., Nikhil Shantanu Marathe wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/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.</div>
<div>By Nikhil Shantanu Marathe.</div>
<p style="color: grey;"><i>Updated 2010-10-17 11:30:11</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;">Adds UPnP collection support.
1) Depends on the Cagibi SSDP daemon for discovery and the UPnP KIO slave (http://gitorious.org/kio-upnp-ms) for browse
2) Each upnp device is represented as a collection.
3) Tries to use search based collection to reduce memory usage and push complexity to the server. Falls back to MemoryCollection and simple browse when search is not supported
4) BrowseCollection performs a full scan using KIO::listDir
5) SearchCollection uses the slave and queues up jobs as required. The slave API is documented in the slave (controlpointthread.h)
Other than code feedback, I am not sure about certain build parameters
1) How do I check if the upnp kio slave is installed? Right now I'm checking for hupnp, but that probably isn't the right way
What is left:
* In certain cases, the CPU usage goes very high when running a full scan using BrowseCollection
* Needs some profiling
* Fix warnings</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;">Tested with:
MediaTomb - browse based
Fuppes - search based
miniDLNA - search based (high quality)
There are issues with Windows Media Player's inbuilt upnp server. Some don't get accessed, and some don't stream their music at all. This may or may not be an upstream bug, or perhaps WMP's upnp is not standards compliant. Either way it requires investigation.</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=145683">145683</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>CMakeLists.txt <span style="color: grey">(baacef5)</span></li>
<li>cmake/modules/FindHUpnp.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/CMakeLists.txt <span style="color: grey">(1e29df7)</span></li>
<li>src/core-impl/collections/upnp/CMakeLists.txt <span style="color: grey">(441cd04)</span></li>
<li>src/core-impl/collections/upnp/amarok_collection-upnpcollection.desktop <span style="color: grey">(061b661)</span></li>
<li>src/core-impl/collections/upnpcollection/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpBrowseCollection.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpBrowseCollection.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpCache.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpCache.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpCollectionBase.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpCollectionBase.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpCollectionFactory.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpCollectionFactory.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpMemoryQueryMaker.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpMemoryQueryMaker.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpMeta.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpMeta.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpQuery.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpQuery.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpQueryMaker.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpQueryMaker.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpQueryMakerInternal.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpQueryMakerInternal.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpSearchCollection.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/UpnpSearchCollection.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/amarok_collection-upnpcollection.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/dbuscodec.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/dbuscodec.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core-impl/collections/upnpcollection/deviceinfo.h <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100065/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>