D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
This revision was automatically updated to reflect the committed changes. Closed by commit R269:5f12404807cc: Implement Media and MediaEndpoint API (authored by mweichselbaumer, committed by drosca). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D15745?vs=42877=42878#toc REPOSITORY

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread Manuel Weichselbaumer
mweichselbaumer added a comment. In D15745#336711 , @drosca wrote: > Thanks. > I'll need your full name + e-mail if you don't have dev account to push it. You're welcome. It's been a pleasure to contribute to this lib. Manuel

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca added a comment. Thanks. I'll need your full name + e-mail if you don't have dev account to push it. REVISION DETAIL https://phabricator.kde.org/D15745 To: mweichselbaumer, drosca Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread Manuel Weichselbaumer
mweichselbaumer updated this revision to Diff 42877. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15745?vs=42861=42877 REVISION DETAIL https://phabricator.kde.org/D15745 AFFECTED FILES autotests/CMakeLists.txt autotests/fakebluez/CMakeLists.txt

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca added a comment. In D15745#336662 , @mweichselbaumer wrote: > In D15745#336644 , @drosca wrote: > > > Remove NoInputNoOutputAgent and it's good to go. > > > Agree. Is it ok to move it

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread Manuel Weichselbaumer
mweichselbaumer added a comment. In D15745#336644 , @drosca wrote: > Remove NoInputNoOutputAgent and it's good to go. Agree. Is it ok to move it to mediaendpointconnector? REVISION DETAIL https://phabricator.kde.org/D15745 To:

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca accepted this revision. drosca added a comment. This revision is now accepted and ready to land. Remove NoInputNoOutputAgent and it's good to go. REVISION DETAIL https://phabricator.kde.org/D15745 To: mweichselbaumer, drosca Cc: broulik, kde-frameworks-devel, michaelh, ngraham,

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca added a comment. In D15745#336615 , @mweichselbaumer wrote: > In D15745#336593 , @drosca wrote: > > > Alright, last thing: > > > > Why NoInputNoOutputAgent? That should be implemented by

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread Manuel Weichselbaumer
mweichselbaumer added a comment. In D15745#336593 , @drosca wrote: > Alright, last thing: > > Why NoInputNoOutputAgent? That should be implemented by the application, and not be part of library. In almost all cases you actually want to

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca added a comment. Alright, last thing: Why NoInputNoOutputAgent? That should be implemented by the application, and not be part of library. In almost all cases you actually want to inform user that something is trying to connect anyway. REVISION DETAIL

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread Manuel Weichselbaumer
mweichselbaumer updated this revision to Diff 42861. mweichselbaumer added a comment. Fixed style issues and smart pointer construction CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15745?vs=42813=42861 REVISION DETAIL https://phabricator.kde.org/D15745 AFFECTED FILES

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread Manuel Weichselbaumer
mweichselbaumer marked 3 inline comments as done and an inline comment as not done. REVISION DETAIL https://phabricator.kde.org/D15745 To: mweichselbaumer, drosca Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15745: Implement Media and MediaEndpoint API

2018-10-04 Thread David Rosca
drosca requested changes to this revision. drosca added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > manager_p.cpp:46 > , m_bluezProfileManager(nullptr) > +, m_media(nullptr) > , m_initialized(false) No need because it is smart pointer. >

D15745: Implement Media and MediaEndpoint API

2018-10-03 Thread Manuel Weichselbaumer
mweichselbaumer updated this revision to Diff 42813. mweichselbaumer added a comment. Added autotests and additional test: mediaendpointconnector CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15745?vs=42313=42813 REVISION DETAIL https://phabricator.kde.org/D15745 AFFECTED FILES

D15745: Implement Media and MediaEndpoint API

2018-10-03 Thread Manuel Weichselbaumer
mweichselbaumer edited the summary of this revision. REVISION DETAIL https://phabricator.kde.org/D15745 To: mweichselbaumer, drosca Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D15745: Implement Media and MediaEndpoint API

2018-09-27 Thread David Rosca
drosca added inline comments. INLINE COMMENTS > mweichselbaumer wrote in media_p.h:37 > MediaPrivate will later act as parent for child objects (inheriting QObject). Then just parent them to Media instead of private class. REVISION DETAIL https://phabricator.kde.org/D15745 To:

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread Manuel Weichselbaumer
mweichselbaumer updated this revision to Diff 42313. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15745?vs=42300=42313 REVISION DETAIL https://phabricator.kde.org/D15745 AFFECTED FILES src/CMakeLists.txt src/a2dp-codecs.h src/interfaces/org.bluez.Media1.xml

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread Manuel Weichselbaumer
mweichselbaumer marked 5 inline comments as done. mweichselbaumer added inline comments. INLINE COMMENTS > broulik wrote in media_p.h:37 > Any particular reason this class must inherit `QObject`, you don't seem to be > using `signal` or `slot` in it MediaPrivate will later act as parent for

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread David Rosca
drosca added inline comments. INLINE COMMENTS > broulik wrote in media_p.h:35 > I think this needs `Q_DECL_HIDDEN` Why? This class is not exported by default, afaik it is only needed if MediaPrivate was declared inside Media class (eg. Media::MediaPrivate), which it is not. REPOSITORY R269

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > media_p.h:35 > + > +class MediaPrivate : public QObject > +{ I think this needs `Q_DECL_HIDDEN` > media_p.h:37 > +{ > +Q_OBJECT > + Any particular reason this class must inherit `QObject`, you don't seem to be using `signal` or `slot` in

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread David Rosca
drosca requested changes to this revision. drosca added a comment. This revision now requires changes to proceed. Looks good apart from the coding style. Also it would be great to have at least basic autotest. INLINE COMMENTS > media.h:95 > + > +friend class MediaPrivate; > +}; Not

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread Kai Uwe Broulik
broulik added a reviewer: drosca. REPOSITORY R269 BluezQt REVISION DETAIL https://phabricator.kde.org/D15745 To: mweichselbaumer, drosca Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15745: Implement Media and MediaEndpoint API

2018-09-25 Thread Manuel Weichselbaumer
mweichselbaumer created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. mweichselbaumer requested review of this revision. REPOSITORY R269 BluezQt REVISION DETAIL https://phabricator.kde.org/D15745 AFFECTED FILES src/CMakeLists.txt