D13931: [KCM] Add speaker placement test

[David Rosca]

drosca requested changes to this revision.
drosca added a comment.
This revision now requires changes to proceed.


  And the whole ComboBox/SinkModel code needs to be changed. You should be able to get the PulseObject from the model just with QML code (see invokables in https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/itemmodels/qabstractitemmodel.h).
  Also I don't see purpose of that "changed" property.

INLINE COMMENTS

> volumefeedback.cpp:36
>  {
> -    if (m_context) {
> -        ca_context_destroy(m_context);
> +    if (QPulseAudio::CanberraContext::instance()->canberra()) {
> +        ca_context_destroy(QPulseAudio::CanberraContext::instance()->canberra());

What if there is multiple VolumeFeedback instances? It will crash then.
You should either move canberra context to Context class (should probably be fine) or implement the refcounting of CanberraContext similar to Context.

> drosca wrote in sink.cpp:89
>   if (name == QLatin1String("Front Left")) {
>       // ...
>   } else if (...) {
>       //
>   }

Braces and "else if" still missing.

> sink.h:35
>      Sink(QObject *parent);
> +    virtual ~Sink();
>  

No reason to add empty destructor.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D13931

To: nicolasfella, drosca
Cc: ngraham, #vdg, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180711/4073d050/attachment-0001.html>