D15745: Implement Media and MediaEndpoint API

David Rosca noreply at phabricator.kde.org
Thu Oct 4 08:50:08 BST 2018


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.

> manager_p.cpp:162
> +        if (interfaces.contains(Strings::orgBluezMedia1())) {
> +            m_media = MediaPtr(new Media(path, this));
> +        }

As it is smart pointer, it must not have a parent.

> mediaendpoint.cpp:46
> +
> +const QVariantMap& MediaEndpoint::properties() const
> +{

`const QVariantMap &`

> mediaendpoint.cpp:74
> +        else if (caps.channel_mode & SBC_CHANNEL_MODE_JOINT_STEREO) caps.channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO;
> +        //else if (caps.channel_mode & SBC_CHANNEL_MODE_DUAL_CHANNEL) caps.channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL;
> +        else break;

Remove commented code.

Also coding style:

  if (..) { // braces always, even for one statement
     ..
  } else if {
     ...
  }

> mediaendpoint.h:141
> +     */
> +    void configurationSet(const QString& transportObjectPath, const QVariantMap& properties);
> +

`const QVariantMap &properties`

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

To: mweichselbaumer, drosca
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20181004/78b759ca/attachment.html>


More information about the Kde-frameworks-devel mailing list