input/ipc-win: allow Windows handles for --input-ipc-client - #18250
Conversation
You can inherit FDs by passing them in mpv/osdep/win32-console-wrapper.c Lines 46 to 66 in e5486b9 |
| fd = ll; | ||
| long long fd = bstrtoll(str, &str, 0); | ||
| if (!str.len && fd >= 0 && fd <= INT_MAX) | ||
| h = (HANDLE)_get_osfhandle((int)fd); |
There was a problem hiding this comment.
Please remove this useless cast. Why LLMs like this integer casts so much. You already check the range. And we don't have integer conversion warnings enabled, but even then it shouldn't trigger here.
There was a problem hiding this comment.
All of the code was written by me, and I usually like being explicit. I can remove it though.
|
Right, I didn't find that honestly. Unfortunately I use Go's os/exec API, which doesn't expose any way to set the reserve fields, so I still think this PR would be helpful. |
Sure, the change is ok. I'm just saying, it's possible to share FDs and is commonly used even if not documented, there was a need for such mechanism. In Go world indeed it's not nativelly supported golang/go#53686 |
Previously, you could only provide a CRT FD, which works great for the most part, but when using CreateProcess on Windows, it requires the use of the reserved fields of STARTUPINFO to inherit the FD to a child mpv process. This becomes a problem for example if you use Go's os/exec API, which does not expose a way to set those reserved fields. Exposing a way to allow passing handles directly fixes the mentioned scenario, and makes --input-ipc-client more flexible at no cost.
f03c306 to
970e5ba
Compare
|
Alright, pushed the changes. Hopefully this satisfies the failed workflow as well. |
|
@kasper93 is there anything else you want me to change? |
2 participants
Previously, you could only provide a CRT FD, which works great on Unix systems, but it makes life hard on Windows. As far as I can tell, there is no way to make CreateProcess inherit CRT FDs, which made using this option impossible in cases where you couldn't just switch to an API that allows for that. CreateProcess does allow inheriting Windows handles however, which the FD would have been converted to anyway.