Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reland "Fix build on macosarm64 and pay attention to USE_SANDBOX flag" #83

Merged
merged 14 commits into from
Jul 4, 2024

Conversation

SteveMcFarlin
Copy link
Contributor

@SteveMcFarlin SteveMcFarlin commented Mar 27, 2024

The issue with the prior PR was USE_SANBOX was ON by default. I clearly did not pay attention enough to that. IMO the default should be OFF, and only be enabled if the arch is arm and the target is macos.

  • This updates the CEF version to 122.1.13. The reason for this is on the M1 you will get a flood of linker errors due to incompatibilities with the older 103.0.9.
  • I had to rename USE_SANBOX to SANDBOX as find_packages was resetting it to ON if set to OFF prior to the call
  • Tested build/inspect on Mac M1 in Release and Linux 64 in Release

Additionally I had to add a few additional cmake options for the MacARM build otherwise I would get errors, and in one case lib_cef_dllwrapper was being compiled for i386. Let me know if there is something wrong there.

Apple clang version 15.0.0 (clang-1500.1.0.2.5)
Target: arm64-apple-darwin23.2.0
cmake version 3.24.1

@SteveMcFarlin SteveMcFarlin marked this pull request as ready for review March 27, 2024 05:40
@reinismu
Copy link
Contributor

It worked on MacOS as well or just compiled?

@SteveMcFarlin
Copy link
Contributor Author

I compiled and did a gst-inspect-1.0 on it. I did not test but I can run one now... BUMMER. SIGSEGV. Changing to draft. I will work on it tomorrow. It's late for me.

@SteveMcFarlin SteveMcFarlin marked this pull request as draft March 27, 2024 07:07
@SteveMcFarlin
Copy link
Contributor Author

SteveMcFarlin commented Mar 27, 2024

GStreamer 1.22.4

0:00:00.789074000 57748 0x60000396f2a0 LOG               aggregator gstaggregator.c:2194:gst_aggregator_query_latency_unlocked:<mix> Signaling src from thread 0x60000396f2a0
0:00:00.789077000 57748 0x60000396f2a0 DEBUG             aggregator gstaggregator.c:2197:gst_aggregator_query_latency_unlocked:<mix> configured latency live:true min:33219954 max:-1
0:00:00.789096000 57748 0x60000396f2a0 LOG               aggregator gstaggregator.c:488:gst_aggregator_check_pads_ready:<mix> checking pads
0:00:00.789099000 57748 0x60000396f2a0 LOG               aggregator gstaggregator.c:532:gst_aggregator_check_pads_ready:<mix:sink_0> Have no buffer and not EOS yet
0:00:00.789101000 57748 0x60000396f2a0 LOG               aggregator gstaggregator.c:532:gst_aggregator_check_pads_ready:<mix:sink_1> Have no buffer and not EOS yet
0:00:00.789103000 57748 0x60000396f2a0 LOG               aggregator gstaggregator.c:587:gst_aggregator_check_pads_ready:<mix> pad not ready to be aggregated yet
0:00:00.789106000 57748 0x60000396f2a0 LOG               aggregator gstaggregator.c:880:gst_aggregator_wait_and_check:<mix> Waiting for src on thread 0x60000396f2a0
 thread 1 , stop reason = signal SIGSTOP

No such issue on Linux

@reinismu
Copy link
Contributor

You can see here #67 Plus I started on my branch to make it work. master...reinismu:gstcefsrc:macos-support but I'm not familiar with cmake and so so with MacOS.

We might put a bounty on this thing. Running this plugin on MacOS would benefit us.

@reinismu
Copy link
Contributor

We had a discussion and we decided to reward 1000$ to someone who can make this plugin work on Apple Silicon MacOS :)

@SteveMcFarlin
Copy link
Contributor Author

You are headed on the right path with what you have done in gstcefsubprocess. If If I were to do more work on this I would look to the CEF examples. Specifically the mac process helper. If I have time this evening I will see what I can come up with.

@SteveMcFarlin
Copy link
Contributor Author

So over lunch I took another look at this. The segfault is coming from CefString. In static gpointer run_cef(GstCefSrc *src) { There is the line:

CefString(&settings.browser_subprocess_path)
      .FromASCII(browser_subprocess_path);

That is where the segfault happens. I will look into it more later today if I have time.

@SteveMcFarlin
Copy link
Contributor Author

SteveMcFarlin commented Mar 27, 2024

At this point not sure what is going on here. From the code CefString str("LETS CRASH");.

* thread #17, name = 'cef-ui-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x0000000000000000
error: memory read failed for 0x0
Target 0: (gst-launch-1.0) stopped.
(lldb) bt
* thread #17, name = 'cef-ui-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000000000000
    frame #1: 0x0000000104291d98 libgstcef.dylib`cef_string_utf8_to_utf16(src="LETS CRASH", src_len=10, output=0x00006000025189c0) at libcef_dll_dylib.cc:1583:10
    frame #2: 0x00000001041a6f74 libgstcef.dylib`CefStringTraitsUTF16::from_string(data="LETS CRASH", length=10, s=0x00006000025189c0) at cef_string_wrappers.h:269:12
    frame #3: 0x00000001041a6e4c libgstcef.dylib`CefStringBase<CefStringTraitsUTF16>::FromString(this=0x00000001706babe0, data="LETS CRASH", length=10) at cef_string_wrappers.h:684:12
    frame #4: 0x00000001041a6d80 libgstcef.dylib`CefStringBase<CefStringTraitsUTF16>::CefStringBase(this=0x00000001706babe0, src="LETS CRASH", length=0) at cef_string_wrappers.h:385:7
    frame #5: 0x00000001041a620c libgstcef.dylib`CefStringBase<CefStringTraitsUTF16>::CefStringBase(this=0x00000001706babe0, src="LETS CRASH", length=0) at cef_string_wrappers.h:383:38
    frame #6: 0x00000001041ab13c libgstcef.dylib`run_cef(src=0x000000012d80eae0) at gstcefsrc.cc:477:13
    frame #7: 0x00000001007101fc libglib-2.0.0.dylib`g_thread_proxy + 68
    frame #8: 0x0000000188bb6034 libsystem_pthread.dylib`_pthread_start + 136

@SteveMcFarlin
Copy link
Contributor Author

SteveMcFarlin commented Mar 27, 2024

I thought maybe trying to load the library in gstcefsrc.c might be the issue, but as soon as I touch

// Initialize the macOS sandbox for this helper process.
  CefScopedSandboxContext sandbox_context;

I will get a crash. I think this might be a mem allocator issue? CefString does an allocation, and so does CefScopedSandboxContext. I can do this

CefBrowserSettings browserSettings;

  settings.no_sandbox = !src->sandbox;
  settings.windowless_rendering_enabled = true;
  settings.log_severity = src->log_severity;

which is fine given this is a simple structure on the stack. It has been a while since I have dealt with these types of issues. I guess if I were going to debug this further I would build CEF against my local tool chain.

@SteveMcFarlin
Copy link
Contributor Author

Closing this as I will not have any more time to tinker with this.

@SteveMcFarlin
Copy link
Contributor Author

SteveMcFarlin commented Mar 31, 2024

@reinismu I would like to get this working at some point, but I simply do not have the time to dig into it. As I stated above the issue revolves around memory allocation AFAICT. There is a bad access at as you can see. I ran the CEF sample minimal without any issue, so I know CEF works on OS-X and the M series chips. At this point I don't know. I have a habit of closing PR's that I will not work on. However, given minimal amount of traffic here I will reopen in hopes someone will have an aha moment.

@SteveMcFarlin SteveMcFarlin reopened this Mar 31, 2024
@reinismu
Copy link
Contributor

Maybe someone from Centricular will get interested in this as well.

I wonder if bad access is not caused by MacOS security stuff. The last I recall from my try was setting up process metadata

@SteveMcFarlin
Copy link
Contributor Author

I am compiling CEF locally, so I can at least debug a bit better. I will continue to look into this when I have some free time.

@MathieuDuponchelle
Copy link
Collaborator

We had a discussion and we decided to reward 1000$ to someone who can make this plugin work on Apple Silicon MacOS :)

Noted, I will check with our MacOS people :)

@reinismu
Copy link
Contributor

We had a discussion and we decided to reward 1000$ to someone who can make this plugin work on Apple Silicon MacOS :)

Noted, I will check with our MacOS people :)

Did any MacOS people seem interested? :) If price is too low, we can discuss it

@MathieuDuponchelle
Copy link
Collaborator

We had a discussion and we decided to reward 1000$ to someone who can make this plugin work on Apple Silicon MacOS :)

Noted, I will check with our MacOS people :)

Did any MacOS people seem interested? :) If price is too low, we can discuss it

Hey, sorry for not following up, I mentioned this in our internal chat but no one picked up on it at the time. Maybe shoot us a mail at contact@centricular.com to get the ball rolling :)

@amyspark
Copy link
Collaborator

Hi! @nirbheek told me I could have a look at this. The issue you were facing is that, on macOS, what you're calling is in fact an interposer, lazy-loader dylib: https://github.com/chromiumembedded/cef/blob/00118ddcdbdd6f7a5ed6e131417e6fb222bb33b0/libcef_dll/wrapper/cef_library_loader_mac.mm#L63-L65

Since that library was never initialized in the gstcef side, all CEF calls were rendered null pointer dereferences. I believe these changes should be sufficient to get you going, feel free to include and rebase them : SteveMcFarlin/gstcefsrc@cmake-macosarm64...amyspark:gstcefsrc:cmake-macosarm64

Bear in mind, however, that it seems CEF intends you to consume the library as an Apple framework -- the loader attempts to find the Framework relative to the executable's path, as when deployed into an app bundle.

@reinismu
Copy link
Contributor

Hi! @nirbheek told me I could have a look at this. The issue you were facing is that, on macOS, what you're calling is in fact an interposer, lazy-loader dylib: https://github.com/chromiumembedded/cef/blob/00118ddcdbdd6f7a5ed6e131417e6fb222bb33b0/libcef_dll/wrapper/cef_library_loader_mac.mm#L63-L65

Since that library was never initialized in the gstcef side, all CEF calls were rendered null pointer dereferences. I believe these changes should be sufficient to get you going, feel free to include and rebase them : SteveMcFarlin/gstcefsrc@cmake-macosarm64...amyspark:gstcefsrc:cmake-macosarm64

Bear in mind, however, that it seems CEF intends you to consume the library as an Apple framework -- the loader attempts to find the Framework relative to the executable's path, as when deployed into an app bundle.

Thanks for looking into this :)

Currently trying it out on my M2 Mac Mini. Compiled and set up everything on it.

Sadly for me it segfaults :(

Command

GST_PLUGIN_PATH=Release:$GST_PLUGIN_PATH gst-launch-1.0 -e \
    cefsrc url="https://soundcloud.com/platform/sama" ! \
    video/x-raw, width=1920, height=1080, framerate=60/1 ! \
    cefdemux name=demux ! queue ! videoconvert ! \
    queue max-size-bytes=0 max-size-buffers=0 max-size-time=3000000000 ! x264enc ! queue ! \
    mp4mux name=muxer ! filesink location='test.mp4' \
    audiotestsrc do-timestamp=true is-live=true  volume=0.0 ! audiomixer name=mix ! \
    queue max-size-bytes=0 max-size-buffers=0 max-size-time=3000000000 ! audioconvert ! \
    audiorate ! audioresample ! faac bitrate=128000 ! queue ! muxer. \
    demux. ! queue ! mix.

Logs:

.... setting up pipeline

0:00:00.026971459 30738 0x600000391770 INFO               structure gststructure.c:2957:gst_structure_get_valist: Expected field 'channel-mask' in structure: audio/x-raw, rate=(int)[ 1, 2147483647 ], channels=(int)[ 1, 2147483647 ];
0:00:00.026979917 30738 0x6000003ac370 INFO              GST_STATES gstelement.c:2825:gst_element_continue_state:<queue5> completed state change to PAUSED
0:00:00.026985000 30738 0x6000003ac370 INFO              GST_STATES gstelement.c:2728:_priv_gst_element_state_changed:<queue5> notifying about state-changed READY to PAUSED (VOID_PENDING pending)
0:00:00.026989375 30738 0x6000003ac370 INFO              GST_STATES gstbin.c:2942:gst_bin_change_state_func:<pipeline0> child 'queue5' changed state to 3(PAUSED) successfully
0:00:00.026992834 30738 0x6000003ac370 INFO              GST_STATES gstbin.c:2484:gst_bin_element_set_state:<demux> current READY pending VOID_PENDING, desired next PAUSED
0:00:00.026996125 30738 0x6000003ac370 INFO              GST_STATES gstelement.c:2825:gst_element_continue_state:<demux> completed state change to PAUSED
0:00:00.026998625 30738 0x6000003ac370 INFO              GST_STATES gstelement.c:2728:_priv_gst_element_state_changed:<demux> notifying about state-changed READY to PAUSED (VOID_PENDING pending)
0:00:00.027001792 30738 0x6000003ac370 INFO              GST_STATES gstbin.c:2942:gst_bin_change_state_func:<pipeline0> child 'demux' changed state to 3(PAUSED) successfully
0:00:00.027004542 30738 0x6000003ac370 INFO              GST_STATES gstbin.c:2484:gst_bin_element_set_state:<capsfilter0> current READY pending VOID_PENDING, desired next PAUSED
0:00:00.027007459 30738 0x6000003ac370 INFO              GST_STATES gstelement.c:2825:gst_element_continue_state:<capsfilter0> completed state change to PAUSED
0:00:00.027009834 30738 0x6000003ac370 INFO              GST_STATES gstelement.c:2728:_priv_gst_element_state_changed:<capsfilter0> notifying about state-changed READY to PAUSED (VOID_PENDING pending)
0:00:00.027012709 30738 0x6000003ac370 INFO              GST_STATES gstbin.c:2942:gst_bin_change_state_func:<pipeline0> child 'capsfilter0' changed state to 3(PAUSED) successfully
0:00:00.027015250 30738 0x6000003ac370 INFO              GST_STATES gstbin.c:2484:gst_bin_element_set_state:<cefsrc0> current READY pending VOID_PENDING, desired next PAUSED
0:00:00.027054750 30738 0x6000003a00a0 WARN              aggregator gstaggregator.c:2283:gst_aggregator_query_latency_unlocked:<mix> Latency query failed
0:00:00.027065417 30738 0x6000003ae620 INFO                  cefsrc gstcefsrc.cc:481:run_cef: Initializing CEF
zsh: segmentation fault  GST_PLUGIN_PATH=Release:$GST_PLUGIN_PATH gst-launch-1.0 -e cefsrc  !     !

I tried running gstcefsubprocess manually and it segfaults as well.

sharehand@SH-MacMini-M2 Release % sudo lldb ./gstcefsubprocess
(lldb) target create "./gstcefsubprocess"
Current executable set to '/Users/sharehand/projects/gstcefsrc/build/Release/gstcefsubprocess' (arm64).
(lldb) run
Process 30799 launched: '/Users/sharehand/projects/gstcefsrc/build/Release/gstcefsubprocess' (arm64)
Process 30799 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x0000000000000000
error: memory read failed for 0x0
Target 0: (gstcefsubprocess) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000000000000
    frame #1: 0x0000000100036f50 gstcefsubprocess`CefExecuteProcess(CefMainArgs const&, scoped_refptr<CefApp>, void*) + 36
    frame #2: 0x000000010000278c gstcefsubprocess`main + 120
    frame #3: 0x000000019547bf28 dyld`start + 2236
(lldb)

Maybe I missed something?

@reinismu
Copy link
Contributor

reinismu commented May 21, 2024

I investigated a bit further and noticed that my gstcefsubprocess doesn't link to libgstcef.dylib

sharehand@SH-MacMini-M2 Release % otool -L gstcefsubprocess
gstcefsubprocess:
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.61.1)
	/System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa (compatibility version 1.0.0, current version 24.0.0)
	/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 2487.30.104)
	/usr/lib/libsandbox.1.dylib (compatibility version 1.0.0, current version 1.0.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1600.157.0)

Compared with my linux build folder and realized that cef libs are missing

sharehand@SH-MacMini-M2 Release % ls -l
total 2080
-rwxr-xr-x  1 sharehand  staff  478464 May 21 10:49 gstcefsubprocess
-rwxr-xr-x  1 sharehand  staff  583648 May 21 10:49 libgstcef.dylib

it seems to try to run cef lib functions, but fails to do so

@amyspark
Copy link
Collaborator

Hi! That's correct, CEF libraries are missing because on macOS, there are none. What the Spotify tarball ships is instead a framework.

I assume the intention of the developers was to consume it from an app, and so they architected the lazy load to resolve from said path (path_to_executable/../Frameworks/Chromium Embedded Framework.framework/...). It'll fail completely if you try to use it from a standalone executable, are you building such an example?

@reinismu
Copy link
Contributor

Hi! That's correct, CEF libraries are missing because on macOS, there are none. What the Spotify tarball ships is instead a framework.

I assume the intention of the developers was to consume it from an app, and so they architected the lazy load to resolve from said path (path_to_executable/../Frameworks/Chromium Embedded Framework.framework/...). It'll fail completely if you try to use it from a standalone executable, are you building such an example?

Did you manage to run gstcefsrc on your end? If so could you provide example?

@SteveMcFarlin SteveMcFarlin marked this pull request as ready for review May 30, 2024 17:39
@reinismu
Copy link
Contributor

@SteveMcFarlin Hey! Did you manage to run these changes on your end? I feel like something still is missing there or I'm doing something wrong as I didn't get it to work :/

@SteveMcFarlin
Copy link
Contributor Author

@reinismu I have not yet. I am getting setup to test it either today or tomorrow.

@amyspark
Copy link
Collaborator

amyspark commented Jul 2, 2024

Nits addressed and initialization variables converted (thanks @MathieuDuponchelle for the insight!), I'd need one more look before merging. @sdroege ?

@nirbheek
Copy link

nirbheek commented Jul 2, 2024

If you haven't already @amyspark, I would suggest testing it once last time on macOS before merging, because non-trivial changes were made to the code.

@amyspark
Copy link
Collaborator

amyspark commented Jul 2, 2024

@nirbheek I did just before pushing, I found another race condition in the event pump (active timers must always be cancelled if the pump is called, irrespective of the plugin's status). Both macOS and Windows launch, run and exit without any issues.

gstcefsrc.cc Outdated Show resolved Hide resolved
gstcefsrc.cc Show resolved Hide resolved
gstcefnsapplication.h Outdated Show resolved Hide resolved
gstcefsrc.cc Outdated Show resolved Hide resolved
gstcefsrc.cc Outdated Show resolved Hide resolved
gstcefsrc.cc Outdated Show resolved Hide resolved
@reinismu
Copy link
Contributor

reinismu commented Jul 3, 2024

Found one issue, but not sure if it is related to changes here or not.

Build works fine for me on MacOS, but if I try to package it together with Nix I run into issue
ld: library not found for -lsandbox

Here is the nix file https://gist.github.com/reinismu/31452372283a7b89715399bd7e763aad

I did some investigation and came to that clang that is installed by Nix only throws this.

Not working command

/nix/store/24ljvc5iwbs01svv9s8zvfcl5qs876kp-clang-wrapper-16.0.6/bin/clang++ -O3 -DNDEBUG -arch arm64 -mmacosx-version-min=10.15 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -Wl,-search_paths_first -Wl,-ObjC -Wl,-pie -Wl,-dead_strip -L/nix/store/wmkr30nbg14vrg08xxvyggwgkixppa3g-gstreamer-1.22.9/lib -L/nix/store/pnafnhig3vz9h0zixvp2gxr5wcywq7kz-glib-2.78.4/lib -L/nix/store/yjzrvf8mppc3yj3qyixkjpvqar0w900k-gst-plugins-base-1.22.9/lib -lgstvideo-1.0 -lgstaudio-1.0 -lgstbase-1.0 -lgstreamer-1.0 -Wl,-rpath,/nix/store/wmkr30nbg14vrg08xxvyggwgkixppa3g-gstreamer-1.22.9/lib -lgobject-2.0 -lglib-2.0 CMakeFiles/gstcefsubprocess.dir/gstcefloader.cc.o CMakeFiles/gstcefsubprocess.dir/gstcefsubprocess.cc.o -o Release/gstcefsubprocess.app/Contents/MacOS/gstcefsubprocess  libcef_dll_wrapper/libcef_dll_wrapper.a -lpthread -framework Cocoa -framework AppKit -lsandbox /tmp/nix-build-gstcefsrc-0.1.0.drv-0/source/third_party/cef/cef_binary_122.1.13+gde5b724+chromium-122.0.6261.130_macosarm64/Release/cef_sandbox.a /nix/store/pnafnhig3vz9h0zixvp2gxr5wcywq7kz-glib-2.78.4/lib/libglib-2.0.dylib -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks

if I call it with /Library/Developer/CommandLineTools/usr/bin/c++ and the same arguments it works

I tried both V15 and V16 Clang and they had the same issue

Update:

Adding export LIBRARY_PATH=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib:$LIBRARY_PATH for build script solved the issue

@MathieuDuponchelle MathieuDuponchelle merged commit 187b280 into centricular:master Jul 4, 2024
1 check passed
@MathieuDuponchelle
Copy link
Collaborator

Let's gooo! Thanks again everyone, this is so nice to finally see this solved :)

@reinismu
Copy link
Contributor

reinismu commented Jul 4, 2024

@amyspark Was this commit 1255b31 related to my nix build issue?

If it was it seems that it still won't build without export LIBRARY_PATH=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/lib:$LIBRARY_PATH

Today was trying to get all my stack building in nix and run into issue :( It seems that it doesn't start the browser and just stucks here https://github.com/centricular/gstcefsrc/blob/master/gstcefsrc.cc#L786

Maybe this is something obvious in your eyes. If not I will just bundle all binaries and distribute it that way :)

@amyspark
Copy link
Collaborator

amyspark commented Jul 4, 2024

@Reinis No, that commit was related to an issue one of my colleagues found, CMake runs all steps linearly, and the last install(DIRECTORY... DESTINATION .) was copying over gstcefsubprocess without the executable bit set.

Can you send me the recipe you're using? Either here or through email (amy at centricular).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants