use correct type for the session object path #609
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The session id is defined as the object path of the session. We should use a type of 'o' instead of 's'.
GeorgesStavracas
approved these changes
Sep 15, 2021
GeorgesStavracas
approved these changes
Sep 15, 2021
GeorgesStavracas
added a commit
to GeorgesStavracas/obs-studio
that referenced
this pull request
Sep 17, 2021
g_variant_lookup() obligatorily receives the type of the variant to
lookup. This function is used when retrieving the session handle
from the portal's response, and the variant type passed is "s" (a
string).
However, xdg-desktop-portal had a bug: the documentation explicitly
mentions that the session handle is an object path (of variant type
"o"), but it passed a string (of variant type "s"). This mismatch
was fixed in the xdg-desktop-portal release 1.10 [1], but that broke
the PipeWire capture code, which was passing specifically the "s"
value to the variant lookup.
Fix this by not checking the variant type at all. Object paths ("o")
are simply strings with a few extra checks, and we don't actually need
to perform these checks.
This change probably broke other apps, and that makes me extremely sad :(
[1] flatpak/xdg-desktop-portal#609
6 tasks
jp9000
pushed a commit
to obsproject/obs-studio
that referenced
this pull request
Sep 17, 2021
g_variant_lookup() obligatorily receives the type of the variant to
lookup. This function is used when retrieving the session handle
from the portal's response, and the variant type passed is "s" (a
string).
However, xdg-desktop-portal had a bug: the documentation explicitly
mentions that the session handle is an object path (of variant type
"o"), but it passed a string (of variant type "s"). This mismatch
was fixed in the xdg-desktop-portal release 1.10 [1], but that broke
the PipeWire capture code, which was passing specifically the "s"
value to the variant lookup.
Fix this by not checking the variant type at all. Object paths ("o")
are simply strings with a few extra checks, and we don't actually need
to perform these checks.
This change probably broke other apps, and that makes me extremely sad :(
[1] flatpak/xdg-desktop-portal#609
|
It's unfortunate that this will break existing clients, for example it will take weeks (or months) until fixed WebRTC (Chrome) gets to users and until then they won't be able to share their screen. |
|
Fedora 35 already updated to the new version and that effectively broke screensharing in Chrome. @matthiasclasen can we revert this change at least in Fedora until we get this fixed in Chrome? I can see OBS and Firefox fixed it the way it doesn't matter what type we use so it should be safe to revert it for a while so things don't break and people are happy. |
|
Oh, grr. Lets see what we can do about this |
Thank you. I believe we can re-revert this change back in ~1-2 months once we are sure it doesn't break anything. |
|
I cherry-picked a fix for obs-studio to deal with this too... https://pkgs.rpmfusion.org/cgit/free/obs-studio.git/commit/?id=311816c1758129bb843a0def17a3b394b2642da3 |
ibaoger
pushed a commit
to ibaoger/webrtc
that referenced
this pull request
Oct 6, 2021
Desktop sharing via Pipewire will break for clients updating to xdg-desktop-portal 1.10 due to a bug fix in the API implementation[1]. This ports over a fix from OBS Studio[2] that also is used in the downstream Firefox WebRTC copy[3]. 1: flatpak/xdg-desktop-portal#609 2: obsproject/obs-studio#5294 3: https://phabricator.services.mozilla.com/D126053 Bug: webrtc:13192 Change-Id: I497dd1bb53cc39dee3732c2e0014e2e36a7afb6c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232329 Reviewed-by: Erik Språng <sprang@webrtc.org> Reviewed-by: Tommi <tommi@webrtc.org> Commit-Queue: Tommi <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#35153}
eric
pushed a commit
to eric/obs-studio
that referenced
this pull request
Jan 1, 2022
g_variant_lookup() obligatorily receives the type of the variant to
lookup. This function is used when retrieving the session handle
from the portal's response, and the variant type passed is "s" (a
string).
However, xdg-desktop-portal had a bug: the documentation explicitly
mentions that the session handle is an object path (of variant type
"o"), but it passed a string (of variant type "s"). This mismatch
was fixed in the xdg-desktop-portal release 1.10 [1], but that broke
the PipeWire capture code, which was passing specifically the "s"
value to the variant lookup.
Fix this by not checking the variant type at all. Object paths ("o")
are simply strings with a few extra checks, and we don't actually need
to perform these checks.
This change probably broke other apps, and that makes me extremely sad :(
[1] flatpak/xdg-desktop-portal#609
flaeri
pushed a commit
to flaeri/obs-studio
that referenced
this pull request
Jan 26, 2022
g_variant_lookup() obligatorily receives the type of the variant to
lookup. This function is used when retrieving the session handle
from the portal's response, and the variant type passed is "s" (a
string).
However, xdg-desktop-portal had a bug: the documentation explicitly
mentions that the session handle is an object path (of variant type
"o"), but it passed a string (of variant type "s"). This mismatch
was fixed in the xdg-desktop-portal release 1.10 [1], but that broke
the PipeWire capture code, which was passing specifically the "s"
value to the variant lookup.
Fix this by not checking the variant type at all. Object paths ("o")
are simply strings with a few extra checks, and we don't actually need
to perform these checks.
This change probably broke other apps, and that makes me extremely sad :(
[1] flatpak/xdg-desktop-portal#609
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The session id is defined as the object path of the session.
We should use a type of 'o' instead of 's'.
Note that the docs mentions the correct type https://flatpak.github.io/xdg-desktop-portal/portal-docs.html#gdbus-method-org-freedesktop-portal-ScreenCast.CreateSession.