Review Request 127540: [plasma-pa] Don't show virtual streams which aren't displayed correctly

Wed Apr 6 10:59:13 UTC 2016

> On avr. 5, 2016, 5:43 après-midi, David Rosca wrote:
> > src/pulseaudio.h, line 64
> > <https://git.reviewboard.kde.org/r/127540/diff/2/?file=455590#file455590line64>
> >
> >     I think this change (adding AbstractStreamModel) is not needed.
> 
> Yoann Laissus wrote:
>     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)
>     
>     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 ?
> 
> David Rosca wrote:
>     Yes, my original snippet doesn't work because mapping works only when there is sorting enabled. So it should be like this:
>     
>     ```
>     filterCallback: function(source_row, value) {
>                         var idx = mapRowFromSource(source_row);
>                         var data = get(idx < 0 ? source_row : idx);
>                         return !data["VirtualStream"];
>                     }
>     ```
>     
>     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.

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/))

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)

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 ?

- Yoann

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127540/#review94305
-----------------------------------------------------------

On avr. 5, 2016, 8:08 après-midi, Yoann Laissus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127540/
> -----------------------------------------------------------
> 
> (Updated avr. 5, 2016, 8:08 après-midi)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-pa
> 
> 
> Description
> -------
> 
> 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.
> 
> Screens attached.
> 
> 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)
> 
> The branch is here : https://quickgit.kde.org/?p=clones%2Fplasma-pa%2Flaissus%2Fplasma-pa.git&a=shortlog&h=014d6925b0911f9db2dc5f55cec5e5a9a84df8c9
> 
> PS : Other review requests are coming (sink selection for streams) but needs some rebasing with David Rosca's latest patches.
> 
> 
> Diffs
> -----
> 
>   applet/contents/ui/StreamListItem.qml a47be1cb65d634e49bb9d97f11853ff667f9aa7d 
>   applet/contents/ui/main.qml b5c9540487255fff567b89aab4c3344ee533618a 
>   src/kcm/package/contents/ui/StreamListItem.qml d1d72bde7f205e6858a8344927f2c34085db5589 
>   src/kcm/package/contents/ui/main.qml 0700910b13089bc515199debee9151990aee1bfc 
>   src/pulseaudio.h d534fa7e7067d660de608e43cd94470281af1ed4 
>   src/pulseaudio.cpp 958451566ac3160bf9f2033c9a471677338f5495 
>   src/qml/CMakeLists.txt 0c2edb305d2745fd857a552c058cf1b7e652f3eb 
>   src/qml/ClientIcon.qml 83545a1a446c6c07611cb59cdc8a7f11e81407e3 
>   src/qml/NonVirtualStreamFilterModel.qml PRE-CREATION 
>   src/qml/plugin.cpp 4e52203a25c19fabe5a097b06e8f6d5e522a8246 
>   src/qml/qmldir 88715cac57d6b8c9c6854caa54b0e3d3f773014a 
>   src/stream.h b5e09ebaaa0b812f3db81e269671640c783e7c87 
>   src/stream.cpp d8f390805e96bbc2dc69ca02520c049b2ca0a4b6 
> 
> Diff: https://git.reviewboard.kde.org/r/127540/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> KCM
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/fd74b564-2e56-45e2-88b9-6bf526f98311__Screenshot_20160331_225655.png
> Applet
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/44b3c277-ee01-4dbf-99f7-dc27edc592e9__Screenshot_20160331_225554.png
> pavucontrol
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/5a872915-dd32-4558-9e4e-947f92f75a70__Screenshot_20160331_231548.png
> KCM
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/cdf983ea-032f-4246-95b8-ba9a4d228ccd__Screenshot_20160331_232502.png
> Applet
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/03/31/1a3d3e3a-6078-445d-84c4-8929cb6243a8__Screenshot_20160331_232444.png
> 
> 
> Thanks,
> 
> Yoann Laissus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160406/87521878/attachment.html>