haasn / libplacebo

Official mirror of libplacebo
http://libplacebo.org/
GNU Lesser General Public License v2.1
523 stars 63 forks source link

libplacebo forces c++_shared #256

Closed lcksk closed 1 month ago

lcksk commented 2 months ago

After upgrading mpv recently, it consistently crashes at random during runtime. After extensive Googling, I discovered that a possible cause might be that my program is linking to two incompatible libc++ runtimes, one of which is from the vendors SDK library. However, both mpv and libplacebo are pure C, with the exception of the recently introduced convert.cc. Is it possible to remove the dependency on c++_shared? Although it's very useful, is it necessary to introduce a C++ dependency in a pure C project? the C++ dependency also significantly increases the app download/install size by requiring the unneeded library. The libc++_shared.so is nearly 1MB (911696 bytes) as seen in app/build/intermediates/stripped_native_libs/release/out/lib/arm64-v8a.

kasper93 commented 2 months ago

Patches welcome. If you have better solution for sane convert.cc implementation.

As for crashes it is your job to ensure proper standard library is used. You have not provided what actually crashes and since there is not c++ in api boundary I doubt the crashes are caused by libplacebo itself.

haasn commented 2 months ago

Remind me, what was the reason we switched from our implementation to convert.cc again?

kasper93 commented 2 months ago

Because there were issues with NIHed parser not working in some corner cases and had quirks like printing 4.000000000000000000000000 instead of 4.0 for example.

[08.08.2023::02:30:13] <haasn> y'know
[08.08.2023::02:30:23] <haasn> probably the sane thing to do is to build a C++ module for string formatting and parsing
[08.08.2023::02:30:28] <haasn> and expose an internal C API for it
[08.08.2023::02:30:47] <haasn> because god knows the C stdlib doesn't get improvements, ever

As for C++ std lib issue, linking statically is an option and will pull only small part of it.

lcksk commented 2 months ago

It may just be a special need of mine, and many people may not care about it. I believe that the current implementation in C++ is quite concise. After handling some urgent matters, I will try my best to experiment with some other methods, such as this: https://github.com/hermantb/fast_convert.git

I'm not sure if it covers part of our needs or is completely unrelated.

kasper93 commented 1 month ago

As for C++ std lib issue, linking statically is an option and will pull only small part of it.

For context, dirty test with conversion function removed. libc++ linked statically -static All removed

1974272 B to 2153984 B: ~9.1%

Removed fast float only

1974272 B to 2122752 B: ~7.5%

Note this is removed completely so only stub functions left, no actual replacement was provided. I don't think the increase in size in significant and if size is an issue there are way more significant areas to optimize in libplacebo.

lcksk commented 1 month ago

I don't think the increase in size in significant >and if size is an issue there are way >more significant areas to optimize in >libplacebo.

Indeed, it is pointless to debate the size in 2024, and the current implementation is already quite refined. I think I'm nitpicking too much. Let's close this matter to keep everyone's focus elsewhere