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

[Yoann Laissus]

> On avr. 5, 2016, 5:43 après-midi, David Rosca wrote:
> > Also can you please rebase the review?

Rebased


> On avr. 5, 2016, 5:43 après-midi, David Rosca wrote:
> > applet/contents/ui/StreamListItem.qml, line 28
> > <https://git.reviewboard.kde.org/r/127540/diff/2/?file=455585#file455585line28>
> >
> >     This should check for client not being null instead.

Yes you're right, I have seen some weird cases where client was null for real streams too.


> On avr. 5, 2016, 5:43 après-midi, David Rosca wrote:
> > src/kcm/package/contents/ui/main.qml, line 38
> > <https://git.reviewboard.kde.org/r/127540/diff/2/?file=455589#file455589line38>
> >
> >     I think this is also not needed, you should be able to do:
> >     
> >     ```
> >     model: NonVirtualStreamFilterModel {
> >         sourceModel: SinkInputModel {}
> >     }
> >     ```

I agree, it's better to move it outside of the ListView.


> 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.

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 ?


> On avr. 5, 2016, 5:43 après-midi, David Rosca wrote:
> > src/qml/plugin.cpp, line 34
> > <https://git.reviewboard.kde.org/r/127540/diff/2/?file=455595#file455595line34>
> >
> >     This should not be registered.

See my comment above.


> On avr. 5, 2016, 5:43 après-midi, David Rosca wrote:
> > src/stream.h, line 38
> > <https://git.reviewboard.kde.org/r/127540/diff/2/?file=455597#file455597line38>
> >
> >     Unrelated

Right, deleted in the upcoming patch.


- Yoann


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


On avr. 5, 2016, 5:23 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, 5:23 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 cbe63ced145cd6e755511d025df84a536976eac4 
>   src/kcm/package/contents/ui/StreamListItem.qml c9ec36c31ef55dbc0281d9807f45f0e48bb4cc41 
>   src/kcm/package/contents/ui/StreamView.qml b08c87a2452cadd13bae5b0b94b57c3d8f531b3b 
>   src/kcm/package/contents/ui/main.qml 3360201dd8e2a4b5ab03c82c94e5c79d8af9ea4c 
>   src/pulseaudio.h 9fc9656e84ee4761d9e8dd9d2a7b8c9d0af9e517 
>   src/pulseaudio.cpp 14887849a04ff2fb3ded5d0d5e60ec3ce4c40745 
>   src/qml/CMakeLists.txt 0c2edb305d2745fd857a552c058cf1b7e652f3eb 
>   src/qml/ClientIcon.qml 83545a1a446c6c07611cb59cdc8a7f11e81407e3 
>   src/qml/NonVirtualStreamFilterModel.qml PRE-CREATION 
>   src/qml/plugin.cpp 5743253f9a7b8283d2bcbeeb081d0706ba4c7b12 
>   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/20160405/c44bf7cf/attachment-0001.html>