search

emersion / xdg-desktop-portal-wlr

xdg-desktop-portal backend for wlroots
MIT License
596 stars 59 forks source link

xdpw should retry after a "out of buffers" error #122

Open emersion opened 3 years ago

emersion commented 3 years ago

Instead of stalling the screencast, xdpw should continue pumping out frames from the compositor after running out of buffers.

danshick commented 3 years ago

Do you mean we should retry every frame until it succeeds? Currently, when we're out of buffers, we free the frame and reregister the wlroots callbacks. We lose a frame but less chance of pushing a stale frame if pipewire lags. The screencast session should still continue regardless.

emersion commented 3 years ago

Hmmm. In my experience the screencast became completely stuck after the first "out of buffers" error, without any retry. I'll check again the code and see if we missed a case.

danshick commented 3 years ago

@emersion, on the latest build, I currently get a ton of "out of buffers" errors from pipewire and the screencast continues to work. Can you check and see if you can reproduce the original issue?

emersion commented 3 years ago

Sorry, I won't have time to investigate this further. I'll re-open if need be.

columbarius commented 3 years ago

One thing to reduce out of buffer occurrences is to increase XDPW_PWR_BUFFERS so we don't get blocked, if a client uses the only buffer on the bus, but i didn't note any blocks because of that.

emersion commented 3 years ago

Ref https://github.com/Plagman/gamescope/pull/219/commits/ef78e2e2707a75b6660809445fab578959a1548e - we need to add a process stream handler to know when a buffer becomes available again, and kick off wlr-screencopy again. This means we drop a frame -- but at least we don't stall the whole screen sharing session.

columbarius commented 3 years ago

I'm not sure what we want to prioritise: Do we want to stall the wlroots loop, when pipewire is out of buffers (which can be avoided with more pipewire buffers) or do we want to create a wlroots screencopy_frame for each frame of the screen and waste it by not sending it via pipewire, when no buffers are available.

Version 1:

  1. register screencopy callback
  2. create screencopy_frame
  3. get buffer events
  4. import buffer from pipewire
    • buffer available -> copy frame from wlroots
    • buffer not available -> goto 6.
  5. screencopy successull -> export buffer
  6. destroy screencopy_frame
  7. goto 1.

Version 2:

  1. register screencopy callback
  2. create screencopy_frame
  3. get buffer events
  4. import buffer from pipewire
    • buffer available -> copy frame from wlroots
    • buffer not available -> wait for buffer to become available and goto 4.
  5. screencopy successull -> export buffer
  6. destroy screencopy_frame
  7. goto 1.

Version 1 would get screencopy events as frequent as possible, but some might not be passed to pipewire since buffers can be unavailable. This will result in dropped, but not missed screencopy_frames.

Version 2 would stall the screencopy on a frame until the next pipewire buffer is available. This results in all screencopy_frames passed over to pipewire with none dropped, but we will miss some possible events if the clients need more time then the output refreshrate.

emersion commented 3 years ago

I think we should stall the Wayland copy loop until we have a new PipeWire buffer available. We should create one wl_buffer per pw_buffer in the add_buffer hook. When there's no PipeWire buffer available we also don't have any wl_buffer available to ask the compositor to perform the copy with.

we will miss some possible events if the clients need more time then the output refreshrate

Not sure what the implications of this are. If we don't have a PipeWire buffer, what can we do with the Wayland events?