D1230: GBM remote access support for KWin

[Roman Gilg]

subdiff added a comment.


  Hi Oleg,
  
  sorry that I haven't given you feedback to your patch series until now. First some smaller points:
  
  - Please merge current master.
  - What about the software cursor dynamically switching? I don't see where it does that.
  - The RemoteAccessManager is active all the time. It would be nice if it would be only active if there are clients who ask for buffers.
  
  Then in general did you look at GNOME's way forward of screen recording/sharing on Wayland? They use PipeWire (http://pipewire.org/) for that. I'm not very knowledgeable on the whole desktop sharing / video stream side, so just a small info on what you think about it and how this relates to your code would be nice. I'm asking that, because I find joint work on single solutions very important.

INLINE COMMENTS

> remoteaccess_manager.cpp:77
> +    auto buf = new BufferHandle;
> +    buf->setFd(gbm_bo_get_fd(gbmbuf->getBo()));
> +    buf->setSize(gbm_bo_get_width(gbmbuf->getBo()), gbm_bo_get_height(gbmbuf->getBo()));

Local var for `gbmbuf->getBo()`

> remoteaccess_manager.h:24
> +#include "drm_backend.h"
> +#include "../../../wayland_server.h"
> +// KWayland

Do you need all these includes in the header file, or could you forward declare for example the KWayland::Server::RemoteAccessManagerInterface and then include in the cpp file?

REPOSITORY
  R108 KWin

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

To: Kanedias, graesslin, davidedmundson
Cc: subdiff, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma-devel, bwowk, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171023/0e6f76ac/attachment-0001.html>