Bug 419209

Summary:	Duplicate buffer copy
Product:	[Plasma] xdg-desktop-portal-kde	Reporter:	kkartaltepe
Component:	general	Assignee:	Jan Grulich <jgrulich>
Status:	RESOLVED FIXED
Severity:	normal	CC:	jgrulich
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://commits.kde.org/xdg-desktop-portal-kde/45092f756316a6b6718a2da5d319355d94abca45	Version Fixed In:

Description kkartaltepe 2020-03-25 02:51:41 UTC

SUMMARY

After downloading the buffer [1] it is copied an additional time [2]
before being passed to passed to pipewire. If the goal is to support standard protocols they shouldnt be needless kneecapped like this (downloading a frame at all is already slow enough without doing it twice). Mutter implementation downloads directly into the pw buffer avoiding the additional copy [3].

[1] https://github.com/KDE/xdg-desktop-portal-kde/blob/master/src/waylandintegration.cpp#L635
[2] https://github.com/KDE/xdg-desktop-portal-kde/blob/master/src/screencaststream.cpp#L518
[3] https://github.com/GNOME/mutter/blob/mainline/src/backends/meta-screen-cast-stream-src.c#L470

OBSERVED RESULT

Performance is worse than mutter

EXPECTED RESULT

Performance should be on par with mutter when both implementations are passing around cpu buffers.

SOFTWARE/OS VERSIONS

Code reviewed on github master. Its unclear if there is an alternative repository more up to date.

ADDITIONAL INFORMATION

Comment 1 Jan Grulich 2020-03-25 09:55:10 UTC

Thanks for the report. I pushed a change for review to address this: https://phabricator.kde.org/D28272

Comment 2 Jan Grulich 2020-03-25 13:56:44 UTC

Git commit f6e559702cf5496a4d029cb3424e1c41a41b9836 by Jan Grulich.
Committed on 25/03/2020 at 13:56.
Pushed by grulich into branch 'master'.

Avoid copying buffer twice

Summary:
Previously we copied frames first to a temporary QImage and then to PipeWire buffer. This shouldn't be
necessary as we can copy frames directly to PipeWire buffers which should be much more effecient.

Reviewers: #plasma, zzag

Reviewed By: #plasma, zzag

Subscribers: meven, zzag, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D28272

M  +55   -4    src/screencaststream.cpp
M  +3    -1    src/screencaststream.h
M  +14   -50   src/waylandintegration.cpp
M  +13   -1    src/waylandintegration.h
M  +3    -10   src/waylandintegration_p.h

https://commits.kde.org/xdg-desktop-portal-kde/f6e559702cf5496a4d029cb3424e1c41a41b9836

Comment 3 Jan Grulich 2020-03-25 14:22:35 UTC

Git commit 45092f756316a6b6718a2da5d319355d94abca45 by Jan Grulich.
Committed on 25/03/2020 at 14:22.
Pushed by grulich into branch 'Plasma/5.18'.

Avoid copying buffer twice

Summary:
Previously we copied frames first to a temporary QImage and then to PipeWire buffer. This shouldn't be
necessary as we can copy frames directly to PipeWire buffers which should be much more effecient.

Reviewers: #plasma, zzag

Reviewed By: #plasma, zzag

Subscribers: meven, zzag, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D28272

M  +55   -4    src/screencaststream.cpp
M  +3    -1    src/screencaststream.h
M  +14   -50   src/waylandintegration.cpp
M  +13   -1    src/waylandintegration.h
M  +3    -10   src/waylandintegration_p.h

https://commits.kde.org/xdg-desktop-portal-kde/45092f756316a6b6718a2da5d319355d94abca45

Comment 4 kkartaltepe 2020-03-25 15:31:09 UTC

Thanks I appreciate it.