xrdp
xrdp copied to clipboard
Implement VideoToolbox-powered H.264 encoder for macOS
Changes / Notes
-
This PR is based / depends on Nexarian/xrdp/mainline_merge
-
This PR adds H.264/yuv420 support for macOS.
-
~~This PR contains Obj-C codes; these codes may not meet coding convention of this repository. (I thought it had to be written in Objective-C, but it wasn't.)~~
-
Initialization of VTCompressionSession will fail if
fork=true
inxrdp.ini
TODO
- [x] add support for dynamic-sized drects
- [x] set
kVTVideoEncoderSpecification_EnableHardwareAcceleratedVideoEncoder
to true - [x] (if possible) rewrite codes in C
@jsorg71 and I are working on how to merge in everything into devel, but I'm afraid it might be a while. There's a lot to test and make sure is right. I suggest taking the pieces from mainline merge and working with us to build a plan on what should go first, and what can wait.
I agree with @Nexarian's comment above. I'm happy to contribute to the review when you think the time is right. In the meantime, the CI pipeline should at least now be working on this PR.
Hi Gyuhwan,
You wrote:
DESCRIPTION OF PROBLEM Hello, we are implementing macOS support for an open source remote desktop project called xrdp.
We are implementing a feature to compress image data with h.264 codec to save bandwidth, but when the daemon process fork(), VTCompressionSessionCreate() fails to initialize and returns -12903( kVTInvalidSessionErr).
If VTCompressionSessionCreate() is called without fork(), initialization succeeds normally.
Is there something we are doing wrong?
(+ I heard before that there is a problem that fork() breaks the Objective-C runtime library (Foundation?), is the VideoToolbox code written in plain C also affected?)
STEPS TO REPRODUCE
- call fork()
- call VTCompressionSessionCreate() from child process
It seems that you’re calling fork without calling exec*. Is that right?
If so, that’s not supported on macOS. Many Apple frameworks use Mach messaging to communicate with helper processes, for example, launchd daemons and agents. Mach messaging and fork don’t play well together. The child inherits the parent’s memory but it does not inherit the parent’s Mach port namespace. So the child thinks it has access to a bunch of Mach services but it doesn’t.
Many of the most-commonly used frameworks, like Foundation, have asserts that detect this situation. It seems that VideoToolbox is reporting an error in this case.
In short, if you call fork without calling exec* you will run into problems. If you’re porting Unix-y code that relies on this then you may get away with this, but not if you use any of Apple’s frameworks.
If you're spinning up a new process for some VideoToolbox work, most folks use threads for this sort thing.
Apple Developer Technical Support
@unstabler - one way to solve this might be to re-exec xrdp after the xrdp fork()
call, but ensure the child gets the file descriptor from the accept()
.
I'm currently working on a branch where I've implemented some of what we'd need for this. If you're interested, let me know and I'll open a feature request where I can add more detail.
@matt335672
I was also working on this part: https://github.com/team-unstablers/xrdp-tumod/pull/1 .
However, I was afraid that my work did not fit the overall direction of this project, so I forked it under the name xrdp-tumod.
If I complete https://github.com/team-unstablers/xrdp-tumod/pull/1 , I will post a PR on xrdp as well.
Thanks for the update.
I'm currently looking at splitting sesman into two executables to fix some long-standing problems. I've got to solve some of the same problems you have to get it working securely.
If you want me to review anything, let me know. We probably don't want these to drift too far apart.
@matt335672
Then, if you don't mind, I would like to request a partial review of https://github.com/team-unstablers/xrdp-tumod/pull/1, although the work is not that advanced yet. (+ can i post / move this PR to here?)
Also, I need to reconstruct the struct trans
in the child process, should I pass the contents of the fields needed for reconstruction to the child process through args[]
(or piping) and assign them directly in the child process?
(sorry for repost!)
To reconstruct a struct trans
, I suggest you pass just the file descriptor over, and then just set up a new one in the child.
You can either leave the file descriptor open and pass the number over, or use libipm
to pass the file descriptor directly. The first is easier to set up, but the second lets you close the file descriptor safely in the parent if that's what you want to do, without a race condition.
I've got examples of both in my split_session_driver branch, but be aware this is changing quite a lot at the moment.