1841851 - libwebrtc framerate negotiation breaks screensharing on Wayland

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P3)

Description

[David Turner]

Attachment: Output from pipewire upon failure

System info

• Raspberry Pi 4, aarch64, running Raspberry Pi OS based on Debian Bookworm
• Wayland, using Wayfire compositor
• Using the xdg-desktop-portal-wlr for screen capture
• Pipewire+wireplumber 0.3.72 built from latest git (but I can also reproduce the problem with pipewire+wireplumber 0.3.65 installed from Bookworm packages)

Steps to reproduce:

• Checkout current Firefox trunk, build and run firefox
• Visit the GUM test page https://mozilla.github.io/webrtc-landing/gum_test.html and click "Screen Capture"
• Click "Allow" on the desktop-portal request

What happens:

• Screen sharing appears to start and the active sharing popup appears
• But no screen content is seen on the GUM page
• I've attached the errors seen in the pipewire log (My pipewire is modified to add a little more info here)
• The output from libwebrtc trace is copied at the bottom

Debugging:

• I've done a git bisect and the regression was introduced by 29e87376, which vendors the libwebrtc commit "desktop_capturer: Support frame rate negotiation via pipewire"
• It looks like pipewire is failing to find a matching format between the xdg-desktop-portal-wlr output and the libwebrtc input.
• Given that nobody else has noticed this in over a month I'm suspicious that this might be specific to xdg-desktop-portal-wlr

libwebrtc trace output:

[Parent 17589: Main Thread]: D/webrtc_trace (screen_capture_portal_interface.cc:34): Successfully created proxy for the portal.
[Parent 17589: Main Thread]: D/webrtc_trace (xdg_desktop_portal_utils.cc:119): Desktop session requested.
[Parent 17589: Main Thread]: D/webrtc_trace (screen_capture_portal_interface.cc:53): Initializing the session.
[Parent 17589: Main Thread]: D/webrtc_trace (screencast_portal.cc:264): Requesting sources from the screen cast session.
[Parent 17589: Main Thread]: D/webrtc_trace (screencast_portal.cc:290): Sources requested from the screen cast session.
[Parent 17589: Main Thread]: D/webrtc_trace (screencast_portal.cc:305): Subscribed to sources signal.
[Parent 17589: Main Thread]: D/webrtc_trace (screencast_portal.cc:320): Received sources signal from session.
[Parent 17589: Main Thread]: D/webrtc_trace (xdg_desktop_portal_utils.cc:155): Starting the portal session.
[Parent 17589: Main Thread]: D/webrtc_trace (screen_capture_portal_interface.cc:123): Subscribed to the start signal.
[Parent 17589: Main Thread]: D/webrtc_trace (screencast_portal.cc:359): Start signal received.
[Parent 17589: Main Thread]: D/webrtc_trace (screencast_portal.cc:426): Opening the PipeWire remote.
[Parent 17589: Main Thread]: D/webrtc_trace (egl_dmabuf.cc:476): Egl initialization succeeded
[Parent 17589: Main Thread]: D/webrtc_trace (shared_screencast_stream.cc:518): PipeWire remote opened.
[Parent 17589: Unnamed thread 7ecda7c940]: D/webrtc_trace (shared_screencast_stream.cc:257): PipeWire stream format changed.
[Parent 17589: Unnamed thread 7ecda7c940]: D/webrtc_trace (shared_screencast_stream.cc:234): PipeWire stream state error: no more input formats
[Parent 17589: Unnamed thread 7ecda7c940]: D/webrtc_trace (shared_screencast_stream.cc:197): PipeWire remote error: no more input formats

Comment 1

[Dan Baker[:dbaker]]

Is this something you could help look into?

Comment 2

[David Turner]

Looks like it's a buffer overflow in webrtc/pipewire 🎉

The pipewire format PODs setup by webrtc to receive screen captures (screencast_stream_utils.cc:BuildFormat()) were pretty close to the 2048-byte size limit before the framerate and maxFramerate properties were added, and adding those was enough to push it over the edge and overflow the buffer. Something else presumably overwrites the overflowed portion of the format POD and pipewire ignores the invalid (overwritten) format options, which in my case means there's no matching format since the one I wanted was near the end. Other setups may or may not see the problem depending on memory layout and which format they actually use.

To confirm this hypothesis: Making all the SPA POD builder buffers 4096 bytes (in shared_screencast_stream.cc do s/1024/4096/ and s/2048/4096/) makes the problem go away.

In theory pipewire should throw an error if the builder buffer overflows, so I need to work out why this isn't happening. And then we'll need an upstream webrtc patch to increase the buffer size. I'll probably get patches out for both of these next week.

Ideally someone should test if this affects more common Wayland platforms (i.e. Asahi and x86_64+GNOME). The problematic commit didn't make it into the 115 release but I think it's in the beta branch.

Comment 3

[David Turner]

Ah, it turns out when Pipewire says "if the buffer is not large enough, an appropriate error will be generated" they mean "Will fire the optional callback nobody ever uses and return an error code nobody ever checks and which is discarded by some of our macros". But it's not obviously buggy even if it is super unhelpful.

I'll file a bug report with webrtc.

Comment 4

[David Turner]

webrtc ticket: https://bugs.chromium.org/p/webrtc/issues/detail?id=15346

Comment 5

[Jan Grulich]

I proposed a fix to upstream. I will submit a backport once it gets approved and merged so I can add a tracking commit hash for future WebRTC backports on Firefox side.

Comment 6

[Jan Grulich]

Attachment: Bug 1841851 - WebRTC backport: PipeWire capturer: increase buffer size to avoid buffer overflow r=jib

This is a simple backport of an WebRTC upstream change.

Upstream commit: 8fcc6df79daf1810cd4ecdb8d2ef1d361abfdc9c

Comment 7

[Martin Stránský [:stransky] (ni? me)]

Seems to be handled now, thanks.

Comment 8

[Paul Adenot (:padenot)]

Comment on attachment 9343434 [details]
Bug 1841851 - WebRTC backport: PipeWire capturer: increase buffer size to avoid buffer overflow r=jib

Beta/Release Uplift Approval Request

• User impact if declined: Screen sharing using pipewire is going to be broken in 116 without this.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This has been well understood and is simply making a buffer larger. This is a patch from upstream that we're backporting.
• String changes made/needed: none
• Is Android affected?: No

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/263be02fdeb0
WebRTC backport: PipeWire capturer: increase buffer size to avoid buffer overflow r=jib,webrtc-reviewers,mjf

Comment 10

[Dianna Smith [:diannaS]]

Comment on attachment 9343434 [details]
Bug 1841851 - WebRTC backport: PipeWire capturer: increase buffer size to avoid buffer overflow r=jib

Switching flag to release for consideration for the next dot release

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/263be02fdeb0

Comment 12

[Dianna Smith [:diannaS]]

Comment on attachment 9343434 [details]
Bug 1841851 - WebRTC backport: PipeWire capturer: increase buffer size to avoid buffer overflow r=jib

Approved for the 116.0.3 dot release

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/42e03f48b510