<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127540/">https://git.reviewboard.kde.org/r/127540/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 5th, 2016, 5:43 p.m. UTC, <b>David Rosca</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="https://git.reviewboard.kde.org/r/127540/diff/2/?file=455590#file455590line64" style="color: black; font-weight: bold; text-decoration: underline;">src/pulseaudio.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">64</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">Q_DECL_EXPORT</span> <span class="nl">AbstractStreamModel</span> <span class="p">:</span> <span class="n">public</span> <span class="n">AbstractModel</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think this change (adding AbstractStreamModel) is not needed.</p></pre>
</blockquote>
<p>On April 5th, 2016, 8:01 p.m. UTC, <b>Yoann Laissus</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I had some issues with rbt which hasn't post the right diff from my branch...
It's actually used in NonVirtualStreamFilterModel to get the PulseObject role. (see the lastest patch)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's better than just retrieving it from SinkInputModel for both SinkInputs and SourceOutputs.
Do you have better ideas to avoid the use of AbstractStreamModel ?</p></pre>
</blockquote>
<p>On April 6th, 2016, 8:29 a.m. UTC, <b>David Rosca</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yes, my original snippet doesn't work because mapping works only when there is sorting enabled. So it should be like this:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">filterCallback<span style="color: #666666">:</span> <span style="color: #008000; font-weight: bold">function</span><span style="color: #666666">(</span>source_row<span style="color: #666666">,</span> value<span style="color: #666666">)</span> <span style="color: #666666">{</span>
var idx <span style="color: #666666">=</span> mapRowFromSource<span style="color: #666666">(</span>source_row<span style="color: #666666">);</span>
var data <span style="color: #666666">=</span> <span style="color: #008000; font-weight: bold">get</span><span style="color: #666666">(</span>idx <span style="color: #666666"><</span> <span style="color: #666666">0</span> <span style="color: #666666">?</span> source_row <span style="color: #666666">:</span> idx<span style="color: #666666">);</span>
<span style="color: #008000; font-weight: bold">return</span> <span style="color: #666666">!</span>data<span style="color: #666666">[</span><span style="color: #BA2121">"VirtualStream"</span><span style="color: #666666">];</span>
<span style="color: #666666">}</span>
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also can you please rename NonVirtualStreamFilterModel to just SortFilterModel (or maybe PaSortFilterModel so it doesn't have the same name as PlasmaCore.SortFilterModel)? I plan to use the sorting/filtering also with other properties, not just virtualstream and don't want to create new model for every property.</p></pre>
</blockquote>
<p>On April 6th, 2016, 10:59 a.m. UTC, <b>Yoann Laissus</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The get method don't return a valid object, even by using source_row (and your latest fixes (https://git.reviewboard.kde.org/r/127588/))</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Maybe I'm missing something but I can't understand how it could work.
Inside the get method, we have calls to the index and data methods of the filter model, not the source one.
It's not working as the filter model is still empty (until we fill it by returning true in the callback method)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For testing purposes, I created a method called getFromSource in SortFilterModel which uses the source model instead and it works like a charm.
Maybe we should introduce this kind of method in SortFilterModel ?</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hmm, that's true, sorry.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> return !sourceModel.data(idx, sourceModel.role("VirtualStream"));
</pre></div>
</p></pre>
<br />
<p>- David</p>
<br />
<p>On April 5th, 2016, 8:08 p.m. UTC, Yoann Laissus wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Yoann Laissus.</div>
<p style="color: grey;"><i>Updated April 5, 2016, 8:08 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-pa
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Virtual streams appear when virtual devices are defined in PulseAudio configuration (eg: combined output for classic analog and HDMI)
Those virtual streams aren't properly shown in both the KCM and the applet, producing ghost entries.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Screens attached.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I added a new property to the Stream object to identify if a stream is real or not.
Then, a proxy model takes care of the filtering in the KCM and the applet.
For now, only real devices are shown. But we could add a combobox later, at least in the KCM, to be able to display Virtual, Real or both kind of streams. (like pavucontrol)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The branch is here : https://quickgit.kde.org/?p=clones%2Fplasma-pa%2Flaissus%2Fplasma-pa.git&a=shortlog&h=014d6925b0911f9db2dc5f55cec5e5a9a84df8c9</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">PS : Other review requests are coming (sink selection for streams) but needs some rebasing with David Rosca's latest patches.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>applet/contents/ui/StreamListItem.qml <span style="color: grey">(a47be1cb65d634e49bb9d97f11853ff667f9aa7d)</span></li>
<li>applet/contents/ui/main.qml <span style="color: grey">(b5c9540487255fff567b89aab4c3344ee533618a)</span></li>
<li>src/kcm/package/contents/ui/StreamListItem.qml <span style="color: grey">(d1d72bde7f205e6858a8344927f2c34085db5589)</span></li>
<li>src/kcm/package/contents/ui/main.qml <span style="color: grey">(0700910b13089bc515199debee9151990aee1bfc)</span></li>
<li>src/pulseaudio.h <span style="color: grey">(d534fa7e7067d660de608e43cd94470281af1ed4)</span></li>
<li>src/pulseaudio.cpp <span style="color: grey">(958451566ac3160bf9f2033c9a471677338f5495)</span></li>
<li>src/qml/CMakeLists.txt <span style="color: grey">(0c2edb305d2745fd857a552c058cf1b7e652f3eb)</span></li>
<li>src/qml/ClientIcon.qml <span style="color: grey">(83545a1a446c6c07611cb59cdc8a7f11e81407e3)</span></li>
<li>src/qml/NonVirtualStreamFilterModel.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/qml/plugin.cpp <span style="color: grey">(4e52203a25c19fabe5a097b06e8f6d5e522a8246)</span></li>
<li>src/qml/qmldir <span style="color: grey">(88715cac57d6b8c9c6854caa54b0e3d3f773014a)</span></li>
<li>src/stream.h <span style="color: grey">(b5e09ebaaa0b812f3db81e269671640c783e7c87)</span></li>
<li>src/stream.cpp <span style="color: grey">(d8f390805e96bbc2dc69ca02520c049b2ca0a4b6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127540/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/fd74b564-2e56-45e2-88b9-6bf526f98311__Screenshot_20160331_225655.png">KCM</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/44b3c277-ee01-4dbf-99f7-dc27edc592e9__Screenshot_20160331_225554.png">Applet</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/5a872915-dd32-4558-9e4e-947f92f75a70__Screenshot_20160331_231548.png">pavucontrol</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/cdf983ea-032f-4246-95b8-ba9a4d228ccd__Screenshot_20160331_232502.png">KCM</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/1a3d3e3a-6078-445d-84c4-8929cb6243a8__Screenshot_20160331_232444.png">Applet</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>