--===============7862743532423286212== Content-Type: multipart/alternative; boundary="Boundary-01=_ftrxPpQ+p4pclCb" Content-Transfer-Encoding: 7bit --Boundary-01=_ftrxPpQ+p4pclCb Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Em Wednesday 30 May 2012, cyberbeat@gmx.de escreveu: > > On May 31, 2012, 12:01 a.m., Lamarque Souza wrote: > Thanks for the quick review. I'll fix the other issues soon. > > Do you also experience, that you cannot choose in nowlistening preferences > the media-player, when you do not modify the checkbox "use special > mediaplayer.."? You mean if "use special mediaplayer" is unchecked then you cannot select the media-player? Yes, that happens here but it seems logical. If you do not want a special media player why let the list enabled? > I fixed that by using a klistwidget instead of k3listbox. KCMModule seems > to not recognizes changes on k3listbox-selections. > > > On May 31, 2012, 12:01 a.m., Lamarque Souza wrote: > > > /trunk/KDE/kdenetwork/kopete/plugins/nowlistening/nlmpris2.cpp, line 61 > > > > > 61> > > > > > > This implementation is basically a copy of the mpris plugin > > > replacing the dbus service and interface names. In the original > > > mpris implementation this line search for the string "org.mpris." > > > and not "org.mpris.MediaPlayer". Now I am in doubt if this line > > > should also be "org.mpris." or the other line should be > > > "org.mpris.MediaPlayer". I am not an expert in mpris so I do not > > > know each one should be used here. > > I think the original Implementation is not good, because it will also find > mpris2 Interface, but cannot use it. Ok, thanks for the clarification. Well, looking again at the patch there are better ways to implement the m_client management. For example, you can use QDBusServiceWatcher to check when the service org.mpris.MediaPlayer2 disappears, that way you will not need to delete and allocate memory for m_client everytime you update the song info, which happens every 5s if I recall correctly. I am also not fan of QDBusReply (and waitForFinished for the matter). They block the event loop while they are processed, which means a problem in the player can freeze the nowlistening plugin. I do not know if a Kopete plugin can freeze the entire Kopete, but anyway, it is better not take the chance. -- Lamarque V. Souza KDE's Network Management maintainer http://planetkde.org/pt-br --Boundary-01=_ftrxPpQ+p4pclCb Content-Type: text/html; charset="iso-8859-15" Content-Transfer-Encoding: 7bit

Em Wednesday 30 May 2012, cyberbeat@gmx.de escreveu:

> > On May 31, 2012, 12:01 a.m., Lamarque Souza wrote:

> Thanks for the quick review. I'll fix the other issues soon.

>

> Do you also experience, that you cannot choose in nowlistening preferences

> the media-player, when you do not modify the checkbox "use special

> mediaplayer.."?

 

You mean if "use special mediaplayer" is unchecked then you cannot select the media-player? Yes, that happens here but it seems logical. If you do not want a special media player why let the list enabled?

> I fixed that by using a klistwidget instead of k3listbox. KCMModule seems

> to not recognizes changes on k3listbox-selections.

>

> > On May 31, 2012, 12:01 a.m., Lamarque Souza wrote:

> > > /trunk/KDE/kdenetwork/kopete/plugins/nowlistening/nlmpris2.cpp, line 61

> > > <http://svn.reviewboard.kde.org/r/6960/diff/1/?file=48034#file48034line

> > > 61>

> > >

> > > This implementation is basically a copy of the mpris plugin

> > > replacing the dbus service and interface names. In the original

> > > mpris implementation this line search for the string "org.mpris."

> > > and not "org.mpris.MediaPlayer". Now I am in doubt if this line

> > > should also be "org.mpris." or the other line should be

> > > "org.mpris.MediaPlayer". I am not an expert in mpris so I do not

> > > know each one should be used here.

>

> I think the original Implementation is not good, because it will also find

> mpris2 Interface, but cannot use it.

 

Ok, thanks for the clarification. Well, looking again at the patch there are better ways to implement the m_client management. For example, you can use QDBusServiceWatcher to check when the service org.mpris.MediaPlayer2 disappears, that way you will not need to delete and allocate memory for m_client everytime you update the song info, which happens every 5s if I recall correctly.

 

I am also not fan of QDBusReply (and waitForFinished for the matter). They block the event loop while they are processed, which means a problem in the player can freeze the nowlistening plugin. I do not know if a Kopete plugin can freeze the entire Kopete, but anyway, it is better not take the chance.

 

--

Lamarque V. Souza

KDE's Network Management maintainer

http://planetkde.org/pt-br

--Boundary-01=_ftrxPpQ+p4pclCb-- --===============7862743532423286212== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kopete-devel mailing list kopete-devel@kde.org https://mail.kde.org/mailman/listinfo/kopete-devel --===============7862743532423286212==--