Initial work done on making it work natively on all platforms/buildsystems

[Wirtos]

Tried to make it less platform-dependent, the only bug so far is pts assertion at line 45, stream.c if you try to close the window. Not sure what causes it. By integrating vcpkg and cmake manifest adding more libraries will be more trivial too. A review would be nice.
All changes narrow down to removing gcc function macros by turning them into comma operator ones, removing C11 stdatmic and using SDL atomics instead, also removed VLAs and used size_t instead of ssize_t since there's no real need to use ssize_t at all.

[rom1v]

Hi,

Thank you for your work.

By integrating vcpkg and cmake manifest adding more libraries will be more trivial too.

However, I will not merge another build system, we will not maintain several build systems (#2008 (comment)).

removing C11 stdatmic and using SDL atomics instead

SDL atomics usage had been removed by 54ccccd. In the end, I would like to make a "core" independant of SDL (and use SDL only for the UI).
The scrcpy target is C11.

used size_t instead of ssize_t since there's no real need to use ssize_t at all.

We use ssize_t in VLC, which is compiled for many platforms without problems. (OK, we don't support MSVC, but this specific ssize_t issue could be fixed by a typedef)

(Also, with size_t raw comparisons with -1 will warn with -Wsign-compare, and < 0 are invalid.)

also removed VLAs

I'm ok to remove VLAs.

the only bug so far is pts assertion at line 45, stream.c if you try to close the window. Not sure what causes it.

diff --git a/app/src/stream.c b/app/src/stream.c
index d1b8b9f3..cdf682c1 100644
--- a/app/src/stream.c
+++ b/app/src/stream.c
@@ -38,7 +38,7 @@ stream_recv_packet(struct stream *stream, AVPacket *packet) {
 
     uint64_t pts = buffer_read64be(header);
     uint32_t len = buffer_read32be(&header[8]);
-    assert(pts == NO_PTS || (pts & 0x8000000000000000) == 0);
+    assert(pts == NO_PTS || (pts & 0x8000000000000000ULL) == 0);
     assert(len);
 
     if (av_new_packet(packet, len)) {

(EDIT: I'm ok with your >> 63 too 👍)

?

[rom1v]

Oh, thank you: #1358 (comment)

[Wirtos]

The scrcpy target is C11.

Well, MSVC has a roadmap on supporting C11 threading, but knowing them it's not going to happen anytime soon.
The solution is to create adapter functions + typedefs which would make other atomics/theads a drop-in replacement.

We use ssize_t in VLC, which is compiled for many platforms without problems. (OK, we don't support MSVC, but this specific ssize_t issue could be fixed by a typedef)

I don't see why non-portable ssize_t might be in use here since you can't replicate its behaviour with standard language tools. (size_t) -1 fits just fine and follows the standard.

(Also, with size_t raw comparisons with -1 will warn with -Wsign-compare, and < 0 are invalid.)

Indeed, probably should add an explicit cast.

the only bug so far is pts assertion at line 45, stream.c if you try to close the window. Not sure what causes it.
?

When closing the window assertion happens with MSB set for some reason. Some kind of race condition?

However, I will not merge another build system, we will not maintain several build systems (#2008 (comment)).

Completely up to you, but imo it's way cleaner plus allows to integrate nicely with source-based dependency manager like vcpkg without a ton of platform-specific code to get ffmpeg, for example. A lot of projects use cmake and it allows to use make/visual studio generators instead of ninja. If needed, I might replace meson with cmake completely.

[rom1v]

The scrcpy target is C11.

Well, MSVC has a roadmap on supporting C11 threading, but knowing them it's not going to happen anytime soon.

I understand your wish to use MSVC, but supporting exotic (and non-open source) compilers which do not support C11 is not a goal for scrcpy (yes, I'm ing a bit). It brings additional complexity that I would like to avoid.

We can make some small changes to fix issues with MSVC, but not prevent using C features that MSVC do not provide. MSVC support is not a goal (I'm sorry).

I try to only use standard things when possible, but if a convenient feature is available in both gcc and clang, then I might use it (I'm less strict than VLC on that point). This can be discussed though: for example, if the only blocking point to support MSVC was the usage of "compound statements" in macros, then I would adapt the code.

When closing the window assertion happens with MSB set for some reason. Some kind of race condition?

Because if you change the type from ssize_t to size_t, then -1 is not < HEADER_SIZE anymore:
https://github.com/Genymobile/scrcpy/pull/2397/files#diff-10a4372c63b8fcd8c07fc76d3f5e8ac626be9a4eef1811c6031400aa1d1f7b08L40

imo it's way cleaner plus allows to integrate nicely with source-based dependency manager like vcpkg without a ton of platform-specific code to get ffmpeg

I agree that building for Windows with the dependencies is a PITA. But for other systems, it's just:

dependencies = [
    dependency('libavformat'),
    dependency('libavcodec'),
    dependency('libavutil'),
    dependency('sdl2'),
]

If there is an open source package manager which provides/builds FFmpeg and SDL dependencies easily for Windows (but uses the dependencies from the system for other platforms), compatible with Meson, then why not? My own additional constraint: the Windows release must be cross-compiled from Linux, the whole release must be buildable from Linux.

Also see discussions on #1753.

A lot of projects use cmake and it allows to use make/visual studio generators instead of ninja. If needed, I might replace meson with cmake completely.

No, thanks. I definitely want to keep Meson over CMake. Sorry.

[Wirtos]

It brings additional complexity that I would like to avoid.

Fair. What about making conditional __STDC_NO_ATOMICS__ along with __STDC_VERSION__ macro replacement with SDL counter-variants then? it's an optional C11 feature, so might as well define SDL fallback.

Because if you change the type from ssize_t to size_t, then -1 is not < HEADER_SIZE anymore:
https://github.com/Genymobile/scrcpy/pull/2397/files#diff-10a4372c63b8fcd8c07fc76d3f5e8ac626be9a4eef1811c6031400aa1d1f7b08L40

Whoops, a little bit of an oversight on my part, shouldn't have used automatic type replacement 👀

No, thanks. I definitely want to keep Meson over CMake. Sorry.

Roger that. It still should work just fine for meson even with the given changes code-wise.
So what should I actually keep/remove?

[rom1v]

What about making conditional __STDC_NO_ATOMICS__ along with __STDC_VERSION__ macro replacement with SDL counter-variants then? it's an optional C11 feature, so might as well define SDL fallback.

That would still be a SDL dependency. One of my goal (in the future) is to have a "scrcpy-core" which does not depend on SDL at all. Moreover, the SDL version is very limited, and does not support the memory order parameter, so it's not just a function name replacement.

So what should I actually keep/remove?

I'm interested in the VLA removals, and a change to grow the Windows cmd buffer (even to 8192, the max length on Windows, but with a malloc() instead of stack-allocated).

If you could, please open a PR for each (otherwise, please tell me).
Also, you should base your changes on dev, not master.

Thank you 😉

[Wirtos]

k, will do