D11250: Expose presentation via MPRIS to rich remote controllers

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> dbusabstractadaptor.h:32
> + */
> +class DBusAbstractAdaptor : public QDBusAbstractAdaptor
> +{

This seems like random glue code, wouldn't this make more sense in Qt or in KF5?

> mprismediaplayer2player.cpp:50
> +    if (flags & Pixmap) {
> +        // TODO: can we be sure about this?
> +        emit pixmapAvailable(page);

Should be about right

> mprismediaplayer2player.cpp:58
> +    : mThumbnailCacheDir(new QTemporaryDir)
> +    // TODO: can we assume document is unchanged while in presentation?
> +    , mThumbnailForPageCreated(document->pages())

No, you can not, file can change at any moment.

> mprismediaplayer2player.cpp:304
> +
> +    // TODO: how to find out if there still is another pixmap request ongoing?
> +    // not a QObject, so cannot connect to destroyed signal

Why do you need that?

> mprismediaplayer2player.h:99
> +public Q_SLOTS: // D-Bus API
> +    void Next();
> +    void Previous();

are this ugly uppercase tyied to the spec? Isn't there like a magic way in Qt to create the api from the xml spec?

REPOSITORY
  R223 Okular

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

To: kossebau, #okular
Cc: aacid, pino, rkflx, #kde_connect, michaelweghorn, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180318/652e5b50/attachment.html>