Re: Review Request 112208: KMix qml applet

2014-08-23 Thread Christian Esken


 On Aug. 23, 2014, 5:58 p.m., Mark Gaiser wrote:
  My suggestion: just push this if there are no major objections since doing 
  that now will have some benefits:
  - It is in time for plasma 5.1
  - You get some more exposure of users/devs testing it besides just you and 
  the few that run your diff.
  
  Sure, it might not be fully done right now, but it leaves enough time to 
  iron out the issues for 5.1 and makes it a lot easier for other developers 
  to jump in.
  
  It can always be reverted (or disabled) if it's too unstable for a plasma 
  5.1 release.
 
 Marco Martin wrote:
 hmm, this one is kde4, so no.
 but I would suggest getting an independent extragear release for the kde4 
 version of this applet.

I also suggested Extragear. But I am starting to wonder whether this approach 
makes sense. I mean, it is not active by default. So there should be no harm in 
activating it. Hmmm, I wish we could discuss this face to face. Ot at least on 
IRC, Jabber, ...


- Christian


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


On Aug. 22, 2014, 8:26 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 22, 2014, 8:26 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Repository: kmix
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/ButtonBar.qml 1467957 
   plasma/kmix-applet-qml/contents/ui/kmixapplet.qml 6c09359 
 
 Diff: https://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 Kmix, horizontal view
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/04/9d6b0ca4-5a75-45cc-ab8e-61b13d4079e6__kmix_horizontal_new.png
 Kmix applet, vertical view
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/04/7efb308a-c306-47c2-883f-64d1f32db5b5__kmix_vertical_new.png
 
 
 Thanks,
 
 Diego Casella
 


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


Re: Review Request 112208: KMix qml applet

2014-08-19 Thread Christian Esken


 On Aug. 12, 2014, 11:19 a.m., Christian Esken wrote:
  For me it looks fine, with some questions:
  1) Is this completely decoupled from KMix? I do not see a open mixer to 
  open the main application.
  2) Is it supposed to be integrated in KMix
  3) In general, the question is how to progress from here. Diego, do you 
  want KMix integration, or a standalone app? Have any ideas or plans to get 
  it in KDE?
 
 Martin Klapetek wrote:
 Note that new work on QML based KMix has started, however this will be 
 Plasma5 only -- http://apachelog.wordpress.com/2014/08/11/volume/
 
 Diego Casella wrote:
 I'll recap everything, hopefully once for all..
 This applet is meant to show the availabe mixers (with the option to show 
 all of them, or just the Master one), and modify/mute/unmute them. That's all.
 If you need to change Master channel or in general configure KMix, you 
 need to run the old KMix application. That's why, in the QML applet, I 
 added a button named Mixer setup: you click it, and the old KMix should 
 appear (in my opinion QML applets have be minimal and simple, so everything 
 else must be done in a regular QtWidget based application).
 Anyway back on the topic: I said should because at the moment KMix does 
 not appear, complaining that:
 
 QDBusConnection: session D-Bus connection created before 
 QCoreApplication. Application may misbehave.
 
 This is the reason why I mentioned, in the very first description of this 
 review request, that KMix needs to be patched :)
 As about how to progess from here, I've mentioned some ideas in the 
 beginning:
 
 * patch Plasma QML bindings to provide mouse wheel events, so I can 
 intercept scroll events in the panel and update volume levels accordingly, 
 providing the same functionality the current KMix tray icon does;
 * patch Plasma QML slider to provide mouse wheel support (don't know if 
 this has been already fixed in Plasma5);
 * patch KMix itself. It should run somewhat daemonized and, when QML 
 applet calls kmix the regular, QtWidget-based application, will pop up. 
 Since in the sourcecode I've seen references to kmixd, which i think it 
 stands for KMix daemon, this may be simple to implement: the daemon 
 provides the low-level connection to the hardware, and this applet and KMix 
 itself connect to the daemon (via DataEngine/DBus mechanism) to communicate 
 with it.
 
 
 That being said:
 * I've been trying to update this applet in the last couple of years, but
 * since KDE4/Plasma is now in maintenance mode (aka no new featured will 
 be added), those patches won't happen anymore;
 * and since someone is already on the process to reinvent the wheel;
 
 I'm kinda sad/tired of how things went so far. The applet is pretty 
 functional (I use it on a daily basis), and it could work fine in Plasma5 
 too. As far as I'm concerned, at this point I don't see any reason to keep 
 this review open. If Harald wants to use any portion of this code, I'm 100% 
 OK with that.
 
 Christian Esken wrote:
 Diego, I am not sure what went so wrong here. I am defintely also not 
 happy how things went on. What makes me most sad is, that you have a working 
 QML applet that just needs really minor integration work. All these minimal 
 issues like scrollwheel not supported (1) should really not stop us from 
 giving the users the option. If they must have scrollwheel or media buttons, 
 then they can use classic KMix dock. If they want a nicer integrated applet, 
 they can use your applet (2). It can be a simple option in KMix's 
 configuration dialog (Dock: Classic, Plasma, Off) instaed of (Dock: On, Off).
 
 Sigh :-|
 
 
 Whatever your decision is, I can understand you Diego. If you want to 
 integrate it would make me happy (see some technical tips below). If you do 
 not want it or you cannot (essential missing plasma features that you do not 
 want to add yourselves), then it is possibly better to close this review 
 request.
 
 --- Technical notes ---
 
 About daemonizing KMix: That exists since forever: Simply disable dock 
 into systray and close window, and KMix will vanish forever. The KMix will 
 show its GUI at every login behavior/issue is gone since the support of 
 hotplugging. To show (I cannot remember that you ever mentioned problems with 
 that before) just do a DBUS call show on the Window (a standard for all Qt 
 windows). If you want to try that, please head along. If 
 
 Side note (as you mentioned it): There is also a real daemon (technically 
 a kded module), but KMixD daemon has no GUI, and proviees just a Mixer DBUS 
 interface.
 
 
 (1) Some ramblings about QML/Plasma: Things seem to progress sometimes 
 slowly or not at all in QML/Plasma. Though much has happened Plasma/QML seems 
 to still lack features. Initially there were no sliders at all in Plasma, and 
 I knew

Re: Review Request 112208: KMix qml applet

2014-08-18 Thread Christian Esken


 On Aug. 12, 2014, 11:19 a.m., Christian Esken wrote:
  For me it looks fine, with some questions:
  1) Is this completely decoupled from KMix? I do not see a open mixer to 
  open the main application.
  2) Is it supposed to be integrated in KMix
  3) In general, the question is how to progress from here. Diego, do you 
  want KMix integration, or a standalone app? Have any ideas or plans to get 
  it in KDE?
 
 Martin Klapetek wrote:
 Note that new work on QML based KMix has started, however this will be 
 Plasma5 only -- http://apachelog.wordpress.com/2014/08/11/volume/
 
 Diego Casella wrote:
 I'll recap everything, hopefully once for all..
 This applet is meant to show the availabe mixers (with the option to show 
 all of them, or just the Master one), and modify/mute/unmute them. That's all.
 If you need to change Master channel or in general configure KMix, you 
 need to run the old KMix application. That's why, in the QML applet, I 
 added a button named Mixer setup: you click it, and the old KMix should 
 appear (in my opinion QML applets have be minimal and simple, so everything 
 else must be done in a regular QtWidget based application).
 Anyway back on the topic: I said should because at the moment KMix does 
 not appear, complaining that:
 
 QDBusConnection: session D-Bus connection created before 
 QCoreApplication. Application may misbehave.
 
 This is the reason why I mentioned, in the very first description of this 
 review request, that KMix needs to be patched :)
 As about how to progess from here, I've mentioned some ideas in the 
 beginning:
 
 * patch Plasma QML bindings to provide mouse wheel events, so I can 
 intercept scroll events in the panel and update volume levels accordingly, 
 providing the same functionality the current KMix tray icon does;
 * patch Plasma QML slider to provide mouse wheel support (don't know if 
 this has been already fixed in Plasma5);
 * patch KMix itself. It should run somewhat daemonized and, when QML 
 applet calls kmix the regular, QtWidget-based application, will pop up. 
 Since in the sourcecode I've seen references to kmixd, which i think it 
 stands for KMix daemon, this may be simple to implement: the daemon 
 provides the low-level connection to the hardware, and this applet and KMix 
 itself connect to the daemon (via DataEngine/DBus mechanism) to communicate 
 with it.
 
 
 That being said:
 * I've been trying to update this applet in the last couple of years, but
 * since KDE4/Plasma is now in maintenance mode (aka no new featured will 
 be added), those patches won't happen anymore;
 * and since someone is already on the process to reinvent the wheel;
 
 I'm kinda sad/tired of how things went so far. The applet is pretty 
 functional (I use it on a daily basis), and it could work fine in Plasma5 
 too. As far as I'm concerned, at this point I don't see any reason to keep 
 this review open. If Harald wants to use any portion of this code, I'm 100% 
 OK with that.

Diego, I am not sure what went so wrong here. I am defintely also not happy how 
things went on. What makes me most sad is, that you have a working QML applet 
that just needs really minor integration work. All these minimal issues like 
scrollwheel not supported (1) should really not stop us from giving the users 
the option. If they must have scrollwheel or media buttons, then they can use 
classic KMix dock. If they want a nicer integrated applet, they can use your 
applet (2). It can be a simple option in KMix's configuration dialog (Dock: 
Classic, Plasma, Off) instaed of (Dock: On, Off).

Sigh :-|


Whatever your decision is, I can understand you Diego. If you want to integrate 
it would make me happy (see some technical tips below). If you do not want it 
or you cannot (essential missing plasma features that you do not want to add 
yourselves), then it is possibly better to close this review request.

--- Technical notes ---

About daemonizing KMix: That exists since forever: Simply disable dock into 
systray and close window, and KMix will vanish forever. The KMix will show 
its GUI at every login behavior/issue is gone since the support of 
hotplugging. To show (I cannot remember that you ever mentioned problems with 
that before) just do a DBUS call show on the Window (a standard for all Qt 
windows). If you want to try that, please head along. If 

Side note (as you mentioned it): There is also a real daemon (technically a 
kded module), but KMixD daemon has no GUI, and proviees just a Mixer DBUS 
interface.


(1) Some ramblings about QML/Plasma: Things seem to progress sometimes slowly 
or not at all in QML/Plasma. Though much has happened Plasma/QML seems to still 
lack features. Initially there were no sliders at all in Plasma, and I knew it 
would be a very rocky way to get all the features in, like to add missing stuff 
like mousewheel over slider. Then I made

Re: Review Request 112208: KMix qml applet

2014-08-12 Thread Christian Esken

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


For me it looks fine, with some questions:
1) Is this completely decoupled from KMix? I do not see a open mixer to open 
the main application.
2) Is it supposed to be integrated in KMix
3) In general, the question is how to progress from here. Diego, do you want 
KMix integration, or a standalone app? Have any ideas or plans to get it in KDE?

- Christian Esken


On May 4, 2014, 9:46 p.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated May 4, 2014, 9:46 p.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Repository: kmix
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml 7e87c8e 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 26f2968 
   plasma/kmix-applet-qml/contents/ui/MixersList.qml 66bda73 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml 1702be7 
   plasma/kmix-applet-qml/contents/ui/VerticalMixerListDelegate.qml bab9ac6 
   plasma/kmix-applet-qml/contents/ui/kmixapplet.qml eecb91c 
 
 Diff: https://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 Kmix, horizontal view
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/04/9d6b0ca4-5a75-45cc-ab8e-61b13d4079e6__kmix_horizontal_new.png
 Kmix applet, vertical view
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/05/04/7efb308a-c306-47c2-883f-64d1f32db5b5__kmix_vertical_new.png
 
 
 Thanks,
 
 Diego Casella
 


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


Re: Review Request 112208: KMix qml applet

2014-04-04 Thread Christian Esken


 On March 22, 2014, 11:09 p.m., Mark Gaiser wrote:
  Is there still an intention on getting this in KDE 4.xx?
  Just wondering since it seems to be much better then the current kmix popup.

I also haven't heard about further development here. Diego as original 
submitter or somebody else would have to push this. I added this as a topic for 
the KDE Multimedia Sprint. It takes place in about 4 months (August 2014): 
https://sprints.kde.org/sprint/212 


- Christian


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Repository: kmix
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


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


Re: Review Request 112208: KMix qml applet

2014-04-04 Thread Christian Esken


 On March 22, 2014, 11:09 p.m., Mark Gaiser wrote:
  Is there still an intention on getting this in KDE 4.xx?
  Just wondering since it seems to be much better then the current kmix popup.
 
 Christian Esken wrote:
 I also haven't heard about further development here. Diego as original 
 submitter or somebody else would have to push this. I added this as a topic 
 for the KDE Multimedia Sprint. It takes place in about 4 months (August 
 2014): https://sprints.kde.org/sprint/212
 
 Diego Casella wrote:
 My bad, sorry guys but I'm still in a busy period of my life :/
 I'll try to fix the remaining issues pointed out in this review request, 
 and then submit the changes here.
 There is still an ongoing (and BIG imho) issue anyway: we really really 
 really need a Plasma QML callback to capture mouse scroll events. AFAIK there 
 still isn't in both Plasma 4.13 and also Plasma Next. And talking about kmix 
 applet, one feature everyone is using is the ability to adjust volume level 
 with just few mouse scrolls over the applet icon.
 I think we need to fix this problem as well, what do you think?

Yes, most definitely we should have mouse scroll events. I guess everybody is 
using it on the tray icon and in the main window, and so it should also be 
there for the applet.
But if users can choose the plasmoid as an optional, then this is not a 
showstopper. It is probably better to get it started as is. We could mark it 
early-access, so interested people can start playing around with it and using 
it.


- Christian


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


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Repository: kmix
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs
 -
 
   plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 
 PRE-CREATION 
   plasma/kmix-applet-qml/contents/ui/VerticalControl.qml PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/112208/diff/
 
 
 Testing
 ---
 
 Tested against master and works fine.
 
 
 File Attachments
 
 
 Default look
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png
 Menu Actions
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png
 Applet Config Options
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png
 Vertical Control
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png
 ToolButton label and Config page after updates
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png
 Control Icon and Label left aligned
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png
 
 
 Thanks,
 
 Diego Casella
 


___
Plasma-devel mailing list

Re: Review Request 112208: KMix qml applet

2013-11-29 Thread Christian Esken


 On Sept. 2, 2013, 10:32 p.m., Sebastian Kügler wrote:
  I've installed the applet, and had a look in more detail. It's coming along 
  really nicely, and already feels a lot better than the QWidget-based kmix. 
  :)
  
  Some issues I've found:
  - Resizing the popup dialog doesn't resize its content, this leads to 
  unfriendly resizing on the one hand, and clipping of the list of channel 
  items on the other
  - The listview is draggable when it doesn't have to be, use something like 
  interactive: height  contentsHeight in the ListView to prevent that from 
  happening
  - The vertical view needs definitely more spacing, if we really want to 
  keep it
  - Some spacing would do the listview good, this leads to better grouping of 
  the channel items
  - The item name isn't properly anchored and should either determine the 
  minimum width, or be elided
  - The percentage on the left hand side should not be bold
  - The slider feels a bit jerky, this is probably the event round-tripping 
  Xetuan mentions, see that part of the thread
  - The speaker icon feels a bit weird to mute the channel, I think that's 
  because it's used with different semantics for the applet as well: open 
  mixer (the panel icon). Maybe it could be done with a checkbox, and 
  possibly moved to the top-right, so that the layout
  - The initial values in the config dialog are not set up correctly. Try 
  switching to master channel only, OK, reopen config dialog: it's set to 
  show all channels. Haven't checked if this is also the case for the 
  orientation
  - Show all mixers - Show all Channels (i.e. does mixer make sense 
  here, or is it really a channel, or a Volume Control?
  - Mixer slider orientation - Orientation ?
  
  Overall, nice work. This is good stuff. =)
 
 Sebastian Kügler wrote:
 Haven't heard about this in a long time, what's the status?
 
 Diego Casella wrote:
 I'm in a quite busy period lately, but I didn't forget this. I'll be back 
 on it in a week or two :)
 
 Christian Esken wrote:
 From the screenshots its starting to look nice. I have a question about 
 how to integrate this. Likely some people will prefer classic tray (e.g. 
 media player control) and others the QML applet (fits KDE better). I would 
 like to see a seamless integration into KMix, so the user is able to choose 
 between classic tray and QML applet, even from within KMix (1).
 Do you think this is feasible, Diego? Or does anybody else have an 
 opinion about it?
 
 
 (1) I am currently redoing the configuration dialog using KConfigDialog 
 and am dedicating one Tab to Systray/Sound Menu related features, where this 
 would fit nicely: 
 http://kmix5.wordpress.com/2013/11/26/secret-view-in-the-new-configuration-dialog/
 
 Diego Casella wrote:
 I think Aaron or Marco have a way better answer than the one i could 
 provide, it's in the earlier comments of this rr.

Thanks for hinting this out. It's a long history, but now I found the comment 
it would be pretty trivial to add a check in the systemtray to see if the kmix 
applet exists on the system [...]. It sound to me like this is a future (= not 
yet existing) extension to the systemtray. But once its there it could be 
transpararent to KMix.
But apsects like global shortcuts (Volume ip/down/mute) and chosing the 
corresponding master control is only in the KMix GUI, so users must be made 
aware how they can change it (by starting KMix). This is probably not obvious. 
If the Mixer configuration would be available in systemsettings, it would be 
much more visible (as I do not know wheter a systemsettings module and a 
KConfigDialog can easily use common code, I will stick with KConfigDialog).


- Christian


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112208/#review39222
---


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Repository: kmix
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes

Re: Review Request 112208: KMix qml applet

2013-11-28 Thread Christian Esken


 On Sept. 2, 2013, 10:32 p.m., Sebastian Kügler wrote:
  I've installed the applet, and had a look in more detail. It's coming along 
  really nicely, and already feels a lot better than the QWidget-based kmix. 
  :)
  
  Some issues I've found:
  - Resizing the popup dialog doesn't resize its content, this leads to 
  unfriendly resizing on the one hand, and clipping of the list of channel 
  items on the other
  - The listview is draggable when it doesn't have to be, use something like 
  interactive: height  contentsHeight in the ListView to prevent that from 
  happening
  - The vertical view needs definitely more spacing, if we really want to 
  keep it
  - Some spacing would do the listview good, this leads to better grouping of 
  the channel items
  - The item name isn't properly anchored and should either determine the 
  minimum width, or be elided
  - The percentage on the left hand side should not be bold
  - The slider feels a bit jerky, this is probably the event round-tripping 
  Xetuan mentions, see that part of the thread
  - The speaker icon feels a bit weird to mute the channel, I think that's 
  because it's used with different semantics for the applet as well: open 
  mixer (the panel icon). Maybe it could be done with a checkbox, and 
  possibly moved to the top-right, so that the layout
  - The initial values in the config dialog are not set up correctly. Try 
  switching to master channel only, OK, reopen config dialog: it's set to 
  show all channels. Haven't checked if this is also the case for the 
  orientation
  - Show all mixers - Show all Channels (i.e. does mixer make sense 
  here, or is it really a channel, or a Volume Control?
  - Mixer slider orientation - Orientation ?
  
  Overall, nice work. This is good stuff. =)
 
 Sebastian Kügler wrote:
 Haven't heard about this in a long time, what's the status?
 
 Diego Casella wrote:
 I'm in a quite busy period lately, but I didn't forget this. I'll be back 
 on it in a week or two :)

From the screenshots its starting to look nice. I have a question about how to 
integrate this. Likely some people will prefer classic tray (e.g. media player 
control) and others the QML applet (fits KDE better). I would like to see a 
seamless integration into KMix, so the user is able to choose between classic 
tray and QML applet, even from within KMix (1).
Do you think this is feasible, Diego? Or does anybody else have an opinion 
about it?


(1) I am currently redoing the configuration dialog using KConfigDialog and am 
dedicating one Tab to Systray/Sound Menu related features, where this would fit 
nicely: 
http://kmix5.wordpress.com/2013/11/26/secret-view-in-the-new-configuration-dialog/


- Christian


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112208/#review39222
---


On Aug. 27, 2013, 8:40 a.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112208/
 ---
 
 (Updated Aug. 27, 2013, 8:40 a.m.)
 
 
 Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and 
 Igor Poboiko.
 
 
 Repository: kmix
 
 
 Description
 ---
 
 KMix qml applet.
 As you can see from the screenshot, the applet is pretty much functional: you 
 can display all the controls available, change its orientation, and decide to 
 whether show all of them or just the Master Control, and refresh its status 
 when new controls are added/removed/updated (such as Amarok current playing 
 track). See screenshots below :)
 Differences from the old kmix tray:
 * no media player controls ( I never investigated how to get them, but 
 honestly opening the audio applet to change/skip/pause audio track makes 
 little sense to me ... if anyone wants this feature back, don't be shy and 
 step in);
 * the button used to select which Mixers are visible has been changed to open 
 Phonon kcm page: since visible mixers are already configurable from KMix app, 
 having a button to show KMix *and* a button to modify Mixers visibilty made 
 little sense here too, so I preferred to give more visibility to Phonon kcm;
 
 Known issues:
 * there is still no way to get notified of mouse wheel events over the 
 popupIcon, so it is not possible to scroll over to increase/decrease the 
 master control volume;
 * no scroll events over the sliders too;
 * if you want to use the applet you most likely will disable KMix tray icon 
 but, if you do so, KMix will show its GUI at every login and you have to 
 close it manually. This requires KMix to be patched. Furthermore, if you 
 click KMix Setup button, KMix window will not restored anymore: this needs 
 to be pathed as well.
 * resize doesn't work properly.
 
 
 Diffs

Re: Review Request 6928: KMix Declarative Applet - First attempt

2013-08-11 Thread Christian Esken


 On July 26, 2013, 10:30 p.m., Christian Esken wrote:
  I have not followed this request, but it looks like I have not receieved 
  any followups on my last comment. I would like to know what to do with this 
  review request. Diego, Igor? Diego, are you still following up on this. 
  Care to contact Igor?
 
 Diego Casella wrote:
 Hi Christian, after hearing that someone else was working on it, I left 
 the development of that plasmoid; I didnt' want to step over Igor's work.
 Anyway, by looking at the current kmix status, it's still missing the 
 conversion to a full kded service: otherwise there will be two entries in the 
 tray: the old kmix, and its plasmoid counterpart. We also need to extend 
 its dbus interface and add a method to pop-up the full kmix gui: in that way, 
 the plasmoid can show the user interface, allowing the end-user to configure 
 kmix properly.
 As soon as those changes will be peformed (if you agree with it of 
 course, and if you want to implement the new kmix as a qml plasmoid), I'd be 
 happy to continue what I did so far :)

Commenting on:
   if you want to implement the new kmix as a qml plasmoid

Well, I have no knowledge in QML and no desire to learn it. If you still want 
to do so - focussing on the tray popup/plasmoid - then it should be much more 
functional than your prototype - especially: supporting multiple controls, 
adding Media player control like in todays KMix.
The KMix Mainwindow will surely always stay a normal application, everything 
else is a complete new application.


- Christian


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6928/#review11062
---


On March 28, 2012, 6:49 p.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6928/
 ---
 
 (Updated March 28, 2012, 6:49 p.m.)
 
 
 Review request for Plasma, Aaron Seigo, Christian Esken, and Marco Martin.
 
 
 Description
 ---
 
 First attempt of making a declarative kmix applet for plasma.
 What the apple does right now:
 * modifies the volume level and the mute/unmute status of the master channel;
 * reacts to changes of the volume level/status (i.e. made with multimedia 
 keys);
 * disables the slider if the channel gets muted, and enables it back as soon 
 as the channel gets unmuted;
 * collapses gracefully in a popup icon when placed inside the panel.
 
 
 Diffs
 -
 
   trunk/KDE/kdemultimedia/kmix/plasma/CMakeLists.txt 1287513 
   
 trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/contents/code/VerticalControl.qml
  PRE-CREATION 
   
 trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/contents/code/kmixapplet.qml 
 PRE-CREATION 
   trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/metadata.desktop 
 PRE-CREATION 
 
 Diff: http://svn.reviewboard.kde.org/r/6928/diff/
 
 
 Testing
 ---
 
 Tested against r1287510. For basic audio management it works great imho.
 
 However, there is a lot of room for improvements, but this is gonna need some 
 extra work outside the kmix applet scope:
 * first of all, the applet need kmix executable to run in order to perform 
 the dbus calls. You can of course disable KMix tray icon feature but, at 
 every login, KMix mainwindow will be shown and the user must closeby hand. 
 This is a kind of ugly behavior that should be avoided;
 * it will be great to great to add an action to allow the user to select the 
 master channel (by reusing KMix Select Master Channel widget), but this 
 will require tweaking KMix dbus interface;
 * as you noticed in the screenshots, the applet in the panel and in the 
 desktop have different size even if it __is__ actually the same: something is 
 going wrong when plasma shows the PopupApplet. This behavior was even worse 
 when I started implementing a flip action to change the layout from 
 horizontal to vertical and vice-versa, and for this reason I gave up and 
 simply stick with the vertical layout.
 
 Could this applet be shipped in the current status, or should we wait for all 
 the aforementioned improvements?
 Comments/ideas/suggestions?
 
 Cheers :)
 
 
 Screenshots
 ---
 
 Applet look in panel and desktop
   http://svn.reviewboard.kde.org/r/6928/s/627/
 Applet look in panel and desktop - audio muted
   http://svn.reviewboard.kde.org/r/6928/s/628/
 
 
 Thanks,
 
 Diego Casella
 


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


Re: Review Request 6928: KMix Declarative Applet - First attempt

2013-07-26 Thread Christian Esken

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6928/#review11062
---


I have not followed this request, but it looks like I have not receieved any 
followups on my last comment. I would like to know what to do with this review 
request. Diego, Igor? Diego, are you still following up on this. Care to 
contact Igor?

- Christian Esken


On March 28, 2012, 6:49 p.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6928/
 ---
 
 (Updated March 28, 2012, 6:49 p.m.)
 
 
 Review request for Plasma, Aaron Seigo, Christian Esken, and Marco Martin.
 
 
 Description
 ---
 
 First attempt of making a declarative kmix applet for plasma.
 What the apple does right now:
 * modifies the volume level and the mute/unmute status of the master channel;
 * reacts to changes of the volume level/status (i.e. made with multimedia 
 keys);
 * disables the slider if the channel gets muted, and enables it back as soon 
 as the channel gets unmuted;
 * collapses gracefully in a popup icon when placed inside the panel.
 
 
 Diffs
 -
 
   trunk/KDE/kdemultimedia/kmix/plasma/CMakeLists.txt 1287513 
   
 trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/contents/code/VerticalControl.qml
  PRE-CREATION 
   
 trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/contents/code/kmixapplet.qml 
 PRE-CREATION 
   trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/metadata.desktop 
 PRE-CREATION 
 
 Diff: http://svn.reviewboard.kde.org/r/6928/diff/
 
 
 Testing
 ---
 
 Tested against r1287510. For basic audio management it works great imho.
 
 However, there is a lot of room for improvements, but this is gonna need some 
 extra work outside the kmix applet scope:
 * first of all, the applet need kmix executable to run in order to perform 
 the dbus calls. You can of course disable KMix tray icon feature but, at 
 every login, KMix mainwindow will be shown and the user must closeby hand. 
 This is a kind of ugly behavior that should be avoided;
 * it will be great to great to add an action to allow the user to select the 
 master channel (by reusing KMix Select Master Channel widget), but this 
 will require tweaking KMix dbus interface;
 * as you noticed in the screenshots, the applet in the panel and in the 
 desktop have different size even if it __is__ actually the same: something is 
 going wrong when plasma shows the PopupApplet. This behavior was even worse 
 when I started implementing a flip action to change the layout from 
 horizontal to vertical and vice-versa, and for this reason I gave up and 
 simply stick with the vertical layout.
 
 Could this applet be shipped in the current status, or should we wait for all 
 the aforementioned improvements?
 Comments/ideas/suggestions?
 
 Cheers :)
 
 
 Screenshots
 ---
 
 Applet look in panel and desktop
   http://svn.reviewboard.kde.org/r/6928/s/627/
 Applet look in panel and desktop - audio muted
   http://svn.reviewboard.kde.org/r/6928/s/628/
 
 
 Thanks,
 
 Diego Casella
 


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


Re: Review Request 108223: use Plasma::Dialog for kmix osd

2013-02-10 Thread Christian Esken

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108223/#review27114
---


Just got a bug report that is likely realted to this change (started happening 
on kde 4.10): Bug 314803 - OSD glitches out and freezes
https://bugs.kde.org/show_bug.cgi?id=314803

- Christian Esken


On Jan. 6, 2013, 11:52 p.m., Xuetian Weng wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/108223/
 ---
 
 (Updated Jan. 6, 2013, 11:52 p.m.)
 
 
 Review request for Plasma, Solid, Aaron J. Seigo, Kai Uwe Broulik, and 
 Christian Esken.
 
 
 Description
 ---
 
 Similiar to https://git.reviewboard.kde.org/r/108222, this would fix the 
 shadow problem for kmix osd.
 
 
 Diffs
 -
 
   gui/osdwidget.h 9ec0100 
   gui/osdwidget.cpp 2dc202e 
 
 Diff: http://git.reviewboard.kde.org/r/108223/diff/
 
 
 Testing
 ---
 
 localily tested, no problem
 
 
 Thanks,
 
 Xuetian Weng
 


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


Re: Review Request 108223: use Plasma::Dialog for kmix osd

2013-01-15 Thread Christian Esken

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108223/#review25606
---


I did not expect that this patched would be pushed into KDE4.10, as we are past 
release candidate 2 already. Is this really well tested, especially with theme 
changes, font changes and so on?

- Christian Esken


On Jan. 6, 2013, 11:52 p.m., Xuetian Weng wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/108223/
 ---
 
 (Updated Jan. 6, 2013, 11:52 p.m.)
 
 
 Review request for Plasma, Solid, Aaron J. Seigo, Kai Uwe Broulik, and 
 Christian Esken.
 
 
 Description
 ---
 
 Similiar to https://git.reviewboard.kde.org/r/108222, this would fix the 
 shadow problem for kmix osd.
 
 
 Diffs
 -
 
   gui/osdwidget.h 9ec0100 
   gui/osdwidget.cpp 2dc202e 
 
 Diff: http://git.reviewboard.kde.org/r/108223/diff/
 
 
 Testing
 ---
 
 localily tested, no problem
 
 
 Thanks,
 
 Xuetian Weng
 


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


Re: Review Request: KMix Declarative Applet - First attempt

2012-04-17 Thread Christian Esken

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6928/#review10773
---


I am just wondering if and how much this patch overlaps with Igor Poboiko's 
work on an Kmix Plasmoid. I have notified Igor about this Patch/Review.

- Christian Esken


On March 28, 2012, 6:49 p.m., Diego Casella wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6928/
 ---
 
 (Updated March 28, 2012, 6:49 p.m.)
 
 
 Review request for Plasma, Aaron Seigo, Marco Martin, and Christian Esken.
 
 
 Description
 ---
 
 First attempt of making a declarative kmix applet for plasma.
 What the apple does right now:
 * modifies the volume level and the mute/unmute status of the master channel;
 * reacts to changes of the volume level/status (i.e. made with multimedia 
 keys);
 * disables the slider if the channel gets muted, and enables it back as soon 
 as the channel gets unmuted;
 * collapses gracefully in a popup icon when placed inside the panel.
 
 
 Diffs
 -
 
   trunk/KDE/kdemultimedia/kmix/plasma/CMakeLists.txt 1287513 
   
 trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/contents/code/VerticalControl.qml
  PRE-CREATION 
   
 trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/contents/code/kmixapplet.qml 
 PRE-CREATION 
   trunk/KDE/kdemultimedia/kmix/plasma/kmix-applet/metadata.desktop 
 PRE-CREATION 
 
 Diff: http://svn.reviewboard.kde.org/r/6928/diff/
 
 
 Testing
 ---
 
 Tested against r1287510. For basic audio management it works great imho.
 
 However, there is a lot of room for improvements, but this is gonna need some 
 extra work outside the kmix applet scope:
 * first of all, the applet need kmix executable to run in order to perform 
 the dbus calls. You can of course disable KMix tray icon feature but, at 
 every login, KMix mainwindow will be shown and the user must closeby hand. 
 This is a kind of ugly behavior that should be avoided;
 * it will be great to great to add an action to allow the user to select the 
 master channel (by reusing KMix Select Master Channel widget), but this 
 will require tweaking KMix dbus interface;
 * as you noticed in the screenshots, the applet in the panel and in the 
 desktop have different size even if it __is__ actually the same: something is 
 going wrong when plasma shows the PopupApplet. This behavior was even worse 
 when I started implementing a flip action to change the layout from 
 horizontal to vertical and vice-versa, and for this reason I gave up and 
 simply stick with the vertical layout.
 
 Could this applet be shipped in the current status, or should we wait for all 
 the aforementioned improvements?
 Comments/ideas/suggestions?
 
 Cheers :)
 
 
 Screenshots
 ---
 
 Applet look in panel and desktop
   http://svn.reviewboard.kde.org/r/6928/s/627/
 Applet look in panel and desktop - audio muted
   http://svn.reviewboard.kde.org/r/6928/s/628/
 
 
 Thanks,
 
 Diego Casella
 


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


Re: Review Request: Rework KMix DBus API and add KMix plasma dataengine

2011-08-22 Thread Christian Esken
Am Samstag, 20. August 2011, 14:36:40 schrieb Mark Gaiser:
 
  On Aug. 20, 2011, 1:22 a.m., Mark Gaiser wrote:
   Hi,
   
   I was just trying to do the same thing with kmix and wasted ~6 hours on 
that (or even more) just to find that is was already here but never committed. 
So how are we on this? Can this be committed?
   
   Regards,
   Mark
 
 Sorry, it is already committed. I was looking for a commit hook message or a 
Ship it! message.. Guess it was silently committed. It can already be 
found in the current 4.7 codebase.
 Sorry for the noise.

Hi Mark,

thanks for caring. No problem about the noise. :-)

I just looked at the review request and saw it is labeled with This change 
has been marked as submitted.. Should be enough, I would say. 
http://techbase.kde.org/Development/Review_Board#Closing_a_review_request is 
not too specific about how to properly close a review request, but I guess a 
simple close is enough.

 Christian

 
 
 - Mark
 
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6587/#review10358
 ---
 
 
 On April 7, 2011, 8:40 a.m., Igor Poboiko wrote:
  
  ---
  This is an automatically generated e-mail. To reply, visit:
  http://svn.reviewboard.kde.org/r/6587/
  ---
  
  (Updated April 7, 2011, 8:40 a.m.)
  
  
  Review request for Plasma and Diego Casella.
  
  
  Summary
  ---
  
  This patch reworks KMix DBus API and adds a plasma dataengine+service as a 
frontend to information provided by DBus.
  New DBus structure is:
   - /Mixers
  used to get some global information, such as available mixers list and 
global master mixer
   - /Mixers/MIXER_ID
  used to get information about mixer with id=MIXER_ID. It provides such 
information as list of available controls, name of this mixer, id, etc
   - /Mixers/MIXER_ID/CONTROL_ID
  used to get and set information about control. Such information as volume 
level, mute, name of control, etc.
  It also adds a DBus signals which are emitted when new mixer/control 
appears, or volume level changes.
  It also splits all dbus-related code to separate class, 
DBus{KMix,Mixer,Control}Wrapper.
  
  The Plasma Dataengine:
  By default, the only available source is KMix. It provides information 
global information about KMix: is KMix running, and list of available mixers. 
(its IDs)
  Source for every mixer is called by it's ID (for example, 
ALSA::HDA_Intel:1). This source provides such information about current 
Mixer as: it's readable name, is it opened, its balance and list of available 
controls. It also adds basic sources for every control, which provides only 
information about its readable name
  Sources for controls are called by 'MIXER_ID/CONTROL_ID' (for example, 
ALSA::HDA_Intel:1/Master:0). If you request this source, it will provide 
such information as its readable name, is it muted and its volume level (which 
are updates automatically, using DBus signals).
  There is a service available for controls sources. It provides such 
methods as setVolume() and setMute().
  
  It doesn't close bug 171287, but it becomes one step closer to its solving 
:)
  
  And, I'm not very familiar with CMake, but it would be great idea to make 
plasma part optional.
  
  
  This addresses bug 171287.
  https://bugs.kde.org/show_bug.cgi?id=171287
  
  
  Diffs
  -
  
/trunk/KDE/kdemultimedia/kmix/core/mixdevice.h 1225808 
/trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp 1225808 
/trunk/KDE/kdemultimedia/kmix/CMakeLists.txt 1225808 
/trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp 1225808 
/trunk/KDE/kdemultimedia/kmix/core/mixer.h 1225808 
/trunk/KDE/kdemultimedia/kmix/core/mixer.cpp 1225808 
/trunk/KDE/kdemultimedia/kmix/dbus/dbuscontrolwrapper.h PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/dbus/dbuscontrolwrapper.cpp PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/dbus/dbusmixerwrapper.h PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/dbus/dbusmixerwrapper.cpp PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/dbus/dbusmixsetwrapper.h PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/dbus/dbusmixsetwrapper.cpp PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.control.xml PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.mixer.xml PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.mixset.xml PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/gui/kmixdockwidget.cpp 1225808 
/trunk/KDE/kdemultimedia/kmix/plasma/CMakeLists.txt PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/plasma/engine/CMakeLists.txt PRE-CREATION 
/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixer.operations PRE-
CREATION 
/trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.h PRE-CREATION 

Re: Review Request: Rework KMix DBus API and add KMix plasma dataengine

2011-03-07 Thread Christian Esken

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6587/#review9969
---



/trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11182

I agree that mixer-toggleMute(md-id()); was quite ugly, and it is good 
that it was changed. But md-toggleMute() would be the really elegant 
solution. Is there was a real reason behind it? Can/should it e changed to 
md-toggleMute()?



/trunk/KDE/kdemultimedia/kmix/core/mixer.cpp
http://svn.reviewboard.kde.org/r/6587/#comment11183

It is good to see some cleanups here [controlChangedForwareder()] - thanks 
a lot.


- Christian


On March 6, 2011, 7:24 p.m., Igor Poboiko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6587/
 ---
 
 (Updated March 6, 2011, 7:24 p.m.)
 
 
 Review request for Plasma and Diego Casella.
 
 
 Summary
 ---
 
 This patch reworks KMix DBus API and adds a plasma dataengine+service as a 
 frontend to information provided by DBus.
 New DBus structure is:
  - /Mixers
 used to get some global information, such as available mixers list and global 
 master mixer
  - /Mixers/MIXER_ID
 used to get information about mixer with id=MIXER_ID. It provides such 
 information as list of available controls, name of this mixer, id, etc
  - /Mixers/MIXER_ID/CONTROL_ID
 used to get and set information about control. Such information as volume 
 level, mute, name of control, etc.
 It also adds a DBus signals which are emitted when new mixer/control appears, 
 or volume level changes.
 It also splits all dbus-related code to separate class, 
 DBus{KMix,Mixer,Control}Wrapper.
 
 The Plasma Dataengine:
 By default, the only available source is KMix. It provides information 
 global information about KMix: is KMix running, and list of available mixers. 
 (its IDs)
 Source for every mixer is called by it's ID (for example, 
 ALSA::HDA_Intel:1). This source provides such information about current 
 Mixer as: it's readable name, is it opened, its balance and list of available 
 controls. It also adds basic sources for every control, which provides only 
 information about its readable name
 Sources for controls are called by 'MIXER_ID/CONTROL_ID' (for example, 
 ALSA::HDA_Intel:1/Master:0). If you request this source, it will provide 
 such information as its readable name, is it muted and its volume level 
 (which are updates automatically, using DBus signals).
 There is a service available for controls sources. It provides such methods 
 as setVolume() and setMute().
 
 It doesn't close bug 171287, but it becomes one step closer to its solving :)
 
 And, I'm not very familiar with CMake, but it would be great idea to make 
 plasma part optional.
 
 
 This addresses bug 171287.
 https://bugs.kde.org/show_bug.cgi?id=171287
 
 
 Diffs
 -
 
   /trunk/KDE/kdemultimedia/kmix/CMakeLists.txt 1223808 
   /trunk/KDE/kdemultimedia/kmix/apps/kmix.cpp 1223808 
   /trunk/KDE/kdemultimedia/kmix/core/mixdevice.h 1223808 
   /trunk/KDE/kdemultimedia/kmix/core/mixdevice.cpp 1223808 
   /trunk/KDE/kdemultimedia/kmix/core/mixer.h 1223808 
   /trunk/KDE/kdemultimedia/kmix/core/mixer.cpp 1223808 
   /trunk/KDE/kdemultimedia/kmix/dbus/dbuscontrolwrapper.h PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/dbus/dbuscontrolwrapper.cpp PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/dbus/dbusmixerwrapper.h PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/dbus/dbusmixerwrapper.cpp PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/dbus/dbusmixsetwrapper.h PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/dbus/dbusmixsetwrapper.cpp PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.control.xml PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.mixer.xml PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/dbus/org.kde.kmix.mixset.xml PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/plasma/CMakeLists.txt PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/plasma/engine/CMakeLists.txt PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixer.operations PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.h PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerengine.cpp PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerservice.h PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/plasma/engine/mixerservice.cpp PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/plasma/engine/plasma-engine-mixer.desktop 
 PRE-CREATION 
   /trunk/KDE/kdemultimedia/kmix/tests/CMakeLists.txt 1223808 
 
 Diff: http://svn.reviewboard.kde.org/r/6587/diff
 
 
 Testing
 ---
 
 KMix from KDE SC 4.6.0 compiles ok with this patch, and patch applies to 
 current trunk.
 Tested on system with one