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

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to