Allow sharing of a region of a monitor #154
Comments
|
IIRC Mutter uses Pipewire metadata to do the cropping. @columbarius, is this correct? To reduce the amount of pixel copies, the best would be to ask wlroots to do the cropping directly. The screencopy protocol already supports this. I wonder how we can plumb all of this to the chooser. Hardcoding the region in the config file is fine for simple use-cases, but ideally we should allow users to directly select the region with slurp. That would also allow users to feed custom selection boxes to slurp if they want, and easily snap the cropped region to window boundaries via |
|
I assume this would still let me hard code a region if I really wanted to? In any case, anything that would let me do screen sharing from my desktop PC without changing the resolution of my monitor I would greatly appreciate 😁 |
|
Note, I'm not necessarily against an I'd also like to think how to allow users to interactively select the region, because that's what most people will want. |
Yes. Using the crop metadata would make it really easy to add this feature in the screencast part. I see the main issue in changing our chooser output format. We could either switch to a structured format like json, or extend our current format with some csv style formatting like:
I would see that option as an addition to the I guess slurp can return to relative coordinates of the rectangle to the screen instead of the global ones? |
Right. If we pick a format that can begin with the output name and a separator, we can make it backwards-compatible. Note, from a protocol PoV, the output name can contain any ASCII character. I'm a little worried about
Not yet, but it would be easy to add e.g. |
|
I like the multi-line with keys for new options idea. |
|
@Hubro |
I would love nothing more than to test this, but I'll need more detailed instructions 😄 |
|
The easiest way would be to hardcode the view here https://github.com/columbarius/xdg-desktop-portal-wlr/blob/cropping/src/screencast/screencast.c#L61 like and compile it |
diff --git a/src/screencast/screencast.c b/src/screencast/screencast.c
index b0ec1d7..6453b64 100644
--- a/src/screencast/screencast.c
+++ b/src/screencast/screencast.c
@@ -59,6 +59,10 @@ void xdpw_screencast_instance_init(struct xdpw_screencast_context *ctx,
cast->framerate = cast->max_framerate;
cast->with_cursor = with_cursor;
cast->refcount = 1;
+ cast->crop.x = 1280;
+ cast->crop.y = 30;
+ cast->crop.width = 2560;
+ cast->crop.height = 1410;
logprint(INFO, "xdpw: screencast instance %p has %d references", cast, cast->refcount);
wl_list_insert(&ctx->screencast_instances, &cast->link);
logprint(INFO, "xdpw: %d active screencast instances",Did I screw something up here 🤔 I'm not really a C developer. I don't see a struct xdpw_screencast_instance {
// list
struct wl_list link;
// xdpw
uint32_t refcount;
struct xdpw_screencast_context *ctx;
bool initialized;
struct xdpw_frame simple_frame;
// pipewire
struct pw_stream *stream;
struct spa_hook stream_listener;
struct spa_video_info_raw pwr_format;
uint32_t seq;
uint32_t node_id;
bool pwr_stream_state;
uint32_t framerate;
// wlroots
struct zwlr_screencopy_frame_v1 *frame_callback;
struct xdpw_wlr_output *target_output;
uint32_t max_framerate;
struct zwlr_screencopy_frame_v1 *wlr_frame;
struct xdpw_screencopy_frame screencopy_frame;
bool with_cursor;
int err;
bool quit;
// fps limit
struct fps_limit_state fps_limit;
};Should I do this? cast->simple_frame.crop.x = 1280;
cast->simple_frame.crop.y = 30;
cast->simple_frame.crop.width = 2560;
cast->simple_frame.crop.height = 1410; |
|
Actually when I try to build your branch with no changes, I can't get it to work. It starts OK, but as soon as I try to record from pipewire through OBS: If I build the master branch of https://github.com/emersion/xdg-desktop-portal-wlr, it works fine. |
|
Interesting. I based the changes on my restructure MR. Looks like you found an edgecase which doesn't occure on my setup. Could you please run it with -l TRACE and paste the output at #141.
|
|
@columbarius Done! This wouldn't be the first edge case I've run into by having a 5120x1440 240Hz monitor... 😕 |
|
@columbarius I noticed you pushed some new stuff to the pull request, so I re-tested the cropping branch. It no longer crashes, and I can successfully capture the screen. However, I couldn't get it to crop the image.
Trace log while capturing in OBS: https://gist.githubusercontent.com/Hubro/2fd9dde8f19c33d3f436c74e6b0f8a6d/raw/e6c6e82d92c00339f06c1c566d50d4f573644dfc/STDOUT |
|
@Hubro Just noticed that i didn't increase the numbers of params pushed to pipewire, so it didn't register, that we want to announce cropping. Should be fixed now. |
|
@columbarius Alright, we're getting closer 😁 For some reason, the window size reported to OBS is correct, and everything to the left of the crop is gone, but everything to the right of the capture is still visible, outside of the box... It's hard to explain, take a look at the screenshot:
This is with: + cast->simple_frame.crop.x = 1280;
+ cast->simple_frame.crop.y = 0;
+ cast->simple_frame.crop.width = 2560;
+ cast->simple_frame.crop.height = 1440;Here is the TRACE: https://gist.githubusercontent.com/Hubro/2fd9dde8f19c33d3f436c74e6b0f8a6d/raw/aff3b6af47ea807a50c6d4da640da78e4e116759/STDOUT |
|
Though I should mention this is probably only a breaking issue for OBS and other composable streaming/recording applications. For simple screen sharing use cases, only the contents of the rectangle is displayed anyway, so the desired effect is achieved!
Except that there is probably some wasted resources when transmitting the test of the picture outside of the rectangle to the application. |
|
Great! Then I consider this done until #141 is merged. |
|
Or if anyone wants to write the config file options or extend the chooser code, please add this to #156 ;). |
I thought the point of doing the cropping in Pipewire was to avoid transmitting unnecessary amounts of data to the client 🤔 How could this be an OBS bug if nothing outside of the crop rectangle should be transmitted to OBS in the first place. |
|
Cropping with Pipewire metadata doesn't prevent the whole screen from being copied and sent to consumers. In other words, cropping with Pipewire metadata doesn't reduce bandwidth between the compositor and consumers, so is unlikely to help a lot with your use case. We can do the cropping at the screencopy protocol level to help with that. |
|
No, cropping via pipewire is just a meta information and tells the consumer, that it should only use/import part of the buffer. The size of the buffer doesn't affect the transmission via pipewire at all, since pipewire only stores a file descriptor to some memory location where the buffer is stored. A consumer can then use that information to only copy the significant part of the buffer into it's own memory and save bandwidth there. What we can't change atm is the buffer we obtain from the compositor, since the wlr_screencopy protocol doesn't support to specify anything else then a screen output buffer. That would be improved with #107. |
screencopy does support cropping. |
ahh, there is capture_output_region ... missed that. thanks. |
|
Ok I see, then this is indeed a bug in OBS. In fact, if I go into "Edit transform..." (Ctrl+E) and apply 1px of cropping to the right of the bounding box, the entire overflow disappears. I wouldn't consider this issue completely solved until the crop can be configured in the config file though, so I'll leave it open for now 😄 Thank you guys for your work, people like you make using Wayland feasible! |
|
@Hubro I updated #156 with the option to do cropping with wlroots. Additional to the cropping region you have to specify depending on which you want to use. |
It works! And the performance is significantly better. I look forward for all this to be available through the config file 😁 Meanwhile I will be using your branch as my daily driver @columbarius |
|
@columbarius Good afternoon sir, I happened to notice you pushed this commit 😁 columbarius@6f1e972 Is it now possible to configure the crop region in a config file? Is it stable enough to be worth testing? As I'm still running on commit |
|
@Hubro A fine evening sir, your polite question is much appreciated :) Yes it is possible to configure the crop region via config file. Everything is tracked in #156 and the attached branch. ... Dang I just assumed that you would get notifications on the MR so I forgot to report changes in this issue. Feel free to use the branch. It is stable in the sense that it's feature set is finished and is not expected to change in the future, so feel free to use run it. It won't be merged anytime soon, since I see it just as a stop gap until we can share single windows and to do this we have to switch to a new wayland-protocol, which is in development atm (If there is a reason I'm missing, please remember me). Until the new protocol is finished this transition is another reason, why I don't want to officially add this feature. Feel free to create the equivalent of an AUR package for your distro of choice of this branch. |
|
Regardless of whether we ever get window sharing, I would like to be able to share a region of a screen. |
I realize #107 is still far away, but what about a workaround that will get us most of the way there with very little work?
2 out of my 3 monitors are rather huge, way too large to realistically screenshare the entire monitor in a teams meeting. (One of them is 5120x1440.)
Would it be possible to add a config option to specify a region of a monitor to screenshare?
For example:
This would share a 1920x1080 region of my monitor starting at 100px from the left, 100px from the top. After setting this up, I could make a script using
swaymsgto place the windows precisely within this rectangle.This would avoid all the complexity of window selection, but still make it possible for people with large monitors (ultrawides, 40+ inch 4K monitors etc.) to screenshare. I think this would be a nice workaround until #107 becomes a possibility.
If this is already possible somehow, my bad. I have looked around but I'm not able to find anything.
The text was updated successfully, but these errors were encountered: