D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol added a comment.


  Moving to kwayland-server

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson, zzag
Cc: meven, davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol abandoned this revision.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson, zzag
Cc: meven, davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Vlad Zahorodnii]

zzag added a comment.


  In D28882#658780 , @apol wrote:
  
  > > In future, it might be faster to put up just the interface xml for review 
first.
  >
  > @davidedmundson  @zzag I don't really see how it would have made a 
difference, you only decided to review it 7 to 10 days after I first submitted 
it. 路
  >
  > > What about using existing wl_output objects? The add_source event can be 
simplified quite a lot (even maybe dropped). For windows, we could use string 
handles.
  >
  > So your proposal would be to pass a random resource id or string and see 
what the compositor spits back?
  
  
  Well, it's not that different from passsing uints like this patch proposes. 
In either case we have to have some protection against garbage input values and 
possibly inert resources. wl_outputs are not some random objects and 
practically every GUI application keeps track of them.
  
  > My intention was to keep the protocol auto-contained to some extent so 
every client doesn't need to track windows and screens to be able to request a 
stream.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson, zzag
Cc: meven, davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol added a comment.


  > In future, it might be faster to put up just the interface xml for review 
first.
  
  @davidedmundson  @zzag I don't really see how it would have made a 
difference, you only decided to review it 7 to 10 days after I first submitted 
it. 路
  
  > What about using existing wl_output objects? The add_source event can be 
simplified quite a lot (even maybe dropped). For windows, we could use string 
handles.
  
  So your proposal would be to pass a random resource id or string and see what 
the compositor spits back?
  My intention was to keep the protocol auto-contained to some extent so every 
client doesn't need to track windows and screens to be able to request a stream.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson, zzag
Cc: meven, davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Vlad Zahorodnii]

zzag requested changes to this revision.
zzag added a comment.
This revision now requires changes to proceed.


  > In future, it might be faster to put up just the interface xml for review 
first.
  
  ++
  
  ---
  
  What about using existing `wl_output` objects? The `add_source` event can be 
simplified quite a lot (even maybe dropped). For windows, we could use string 
handles.
  

  
  
  



  
  
  

  
  Please notice that the manager object also needs to advertise supported 
cursor modes
  

  
  
  



  
  


INLINE COMMENTS

> screencast.xml:8
> +  ]]>
> +
> +

Interfaces usually come without `unstable` in the name.

s/zkde_screencast_unstable_v1/zkde_screencast_v1/

> screencast.xml:52-54
> +
> +
> +

zkde_screencast_stream_unstable_v1 must have a destructor request.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson, zzag
Cc: meven, davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 81341.
apol added a comment.


  Address review comments

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=81113=81341

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich, davidedmundson
Cc: meven, davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol marked 2 inline comments as done.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson
Cc: meven, davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> screencasting.cpp:141
> +}
> +void zkde_screencast_unstable_v1_removeSource(uint32_t sourceId) 
> override {
> +auto it = std::find_if(m_sources.begin(), m_sources.end(), 
> [sourceId] (const ScreencastingSource ) {

Missing blank line

> screencasting.cpp:156
> +
> +
> +

Two blank lines too mang

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson
Cc: meven, davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, 
cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> screencast.xml:46
> +
> +
> +

add type = "destructor"

so that clients calling this release the resource

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson
Cc: davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 81113.
apol added a comment.


  Refactor the protocol as suggested by David and Vlad

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80984=81113

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich, davidedmundson
Cc: davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol marked an inline comment as done.
apol added inline comments.

INLINE COMMENTS

> davidedmundson wrote in screencast.xml:31
> this is racey with create.

Any suggestion on how to do it better?

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson
Cc: davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[David Edmundson]

davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  In future, it might be faster to put up just the interface xml for review 
first.
  
  --
  
  In terms of wayland protocols this is non-standard.
  
  All clients get a list of available sources. On bind they get all all 
available sources via addSource. Additional sources are new events. 
  That part's fine.
  
  The part with nodeID is unusual. It's convention (though not technically the 
law) that globals broadcast the same thing to all bound resources.
  
  ClientA requests create.
  All clients get a created event
  There's no way to tie a created or failed to the original request 
  Client A and B can both connect to the same source, yet either can call close.
  
  If we're doing a waland protocol I would have expected:
  
ScreenCastInterface - global
event: source_added
event: done
request: get_stream (returns a ScreenStream object)

ScreenStream - resource
event: created
event: failed
event: closed (closed by external sources, clients should now release the 
stream resource)
request: release   (closes the stream if applicable, is also of type 
destructor)

INLINE COMMENTS

> screencast.xml:16
> +
> +
> +

Typically events are like signals, so would be called "source_added"

Convention is also lower_snake_case

> screencast.xml:31
> +
> +
> +

this is racey with create.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson
Cc: davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Jan Grulich]

jgrulich added a comment.


  I have tested this and it now works as before, tested with Chromium, while 
checking all the values we pass to the portal and PipeWire which seem to be 
correct.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80984.
apol added a comment.


  Also propagate the buffer size, it's important for the client to know what 
buffer size it will have.

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80801=80984

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80801.
apol added a comment.


  Hopefully fix the build for Jan

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80798=80801

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80798.
apol added a comment.


  - Test Cleanup
  - When a resource is destroyed, emit to close all its streams

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80677=80798

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Jan Grulich]

jgrulich added a comment.


  In D28882#653483 , @jgrulich wrote:
  
  > Doesn't seem to build here:
  >
  >   In file included from 
/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.cpp:7:
  >   
/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.h:32:121:
 error: ‘std::function’ has not been declared
  >  32 | ScreencastingSource(const QString , const QString 
, bool isOutput, const QRect , std::function);
  > |   
  ^~~~
  >   In file included from 
/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.cpp:7:
  >   
/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.h:32:129:
 error: expected ‘,’ or ‘...’ before ‘<’ token
  >  32 | ScreencastingSource(const QString , const QString 
, bool isOutput, const QRect , std::function);
  > |   
  ^
  >   
/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.cpp:41:1:
 error: no declaration matches 
‘KWayland::Server::ScreencastingSource::ScreencastingSource(const QString&, 
const QString&, bool, const QRect&, std::function)’
  >  41 | ScreencastingSource::ScreencastingSource(const QString 
, const QString , bool isOutput, const QRect , 
std::function call)
  > | ^~~
  >
  >
  
  
  Missing #include 

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Jan Grulich]

jgrulich added a comment.


  Doesn't seem to build here:
  
In file included from 
/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.cpp:7:

/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.h:32:121:
 error: ‘std::function’ has not been declared
   32 | ScreencastingSource(const QString , const QString 
, bool isOutput, const QRect , std::function);
  | 
^~~~
In file included from 
/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.cpp:7:

/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.h:32:129:
 error: expected ‘,’ or ‘...’ before ‘<’ token
   32 | ScreencastingSource(const QString , const QString 
, bool isOutput, const QRect , std::function);
  | 
^

/home/jgrulich/development/projects/kde/kwayland/src/server/screencasting_interface.cpp:41:1:
 error: no declaration matches 
‘KWayland::Server::ScreencastingSource::ScreencastingSource(const QString&, 
const QString&, bool, const QRect&, std::function)’
   41 | ScreencastingSource::ScreencastingSource(const QString 
, const QString , bool isOutput, const QRect , 
std::function call)
  | ^~~

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80677.
apol added a comment.


  Improve how we initialise the sourceId

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80662=80677

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80662.
apol added a comment.


  Iterate tests (which work now) and uses

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80658=80662

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80658.
apol added a comment.


  Fix test

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80637=80658

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80637.
apol added a comment.


  iterate test

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80605=80637

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80605.
apol added a comment.


  oops

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80604=80605

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80604.
apol added a comment.


  Include server side, a test (that doesn't pass) and some renaming

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80411=80604

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80411.
apol added a comment.


  Include the server side and some renames

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80284=80411

BRANCH
  master

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

AFFECTED FILES
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol marked 2 inline comments as done.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Roman Gilg]

romangg added inline comments.

INLINE COMMENTS

> screencast.xml:8
> +  ]]>
> +
> +

Interface names should be scoped too. Example 
.

Don't ask me why exactly why. I think it has something to do with the C structs 
becoming ambiguous otherwise. Someone else knows?

But in any case I would follow the examples set by upstream.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich
Cc: romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28882: Create protocol to manage video feeds

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

> screencast.xml:13
> +
> +
> +

Can we switch the values to follow xdg-desktop-portal specification?

See 
https://github.com/flatpak/xdg-desktop-portal/blob/master/data/org.freedesktop.impl.portal.ScreenCast.xml#L168.

Even not following the specs, output will be the most used one.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich
Cc: zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 80284.
apol added a subscriber: zzag.
apol added a comment.


  renamed protocol as per @zzag's suggestion

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80278=80284

BRANCH
  master

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

AFFECTED FILES
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/videostreaming.cpp
  src/client/videostreaming.h

To: apol, #kwin, jgrulich
Cc: zzag, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol added a dependent revision: D28884: screencasting: Adoption of the 
org_kde_plasma_video_streaming protocol.

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol created this revision.
apol added reviewers: KWin, jgrulich.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
apol requested review of this revision.

REVISION SUMMARY
  Creates a protocol based on Pipewire that allows us to request feeds to
  be set up by KWin rather than having to do it from a separate process.
  
  This will offer tighter control to KWin to do serve not only outputs but
  also windows as well.

TEST PLAN
  Treat this as a proof of concept to discuss if it's the protocol
  we need right now.

REPOSITORY
  R127 KWayland

BRANCH
  master

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

AFFECTED FILES
  src/client/CMakeLists.txt
  src/client/protocols/video-streaming.xml
  src/client/videostreaming.cpp
  src/client/videostreaming.h

To: apol, #kwin, jgrulich
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns