fix: windows crashes & builds (#55)

- Doing jobs in parallels in GHA (Divide build time by 3).
- Now use custom webrtc builds for this repo
- Windows crashes when receiving video is now fixed (thanks @cloudwebrtc)
    - Now reading defines from webrtc.ninja
- Added a small create_pc test to be sure libwebrtc is working/linked correctly 
- Fix MacOS builds 
- Fixed Linux duplicated main build error (due to nasm)

Author: theomonnom

.github/workflows/webrtc-builds.yml

@@ -1,26 +1,48 @@
 name: WebRTC builds
 on: workflow_dispatch
-env:
-  CARGO_TERM_COLOR: always
 
 jobs:
   build:
     strategy:
+      fail-fast: false
       matrix:
+        os:
+          - windows-latest
+          - ubuntu-latest
+          - macos-latest
+        arch:
+          - x64
+          - arm64
+        profile:
+          - release
+          - debug
         include:
           - os: windows-latest
-            cmd: .\build_windows.cmd 
-            artifacts: windows
+            cmd: .\build_windows.cmd
+            name: win
           - os: ubuntu-latest
             cmd: ./build_linux.sh
-            artifacts: linux
+            name: linux
           - os: macos-latest
             cmd: ./build_macos.sh
-            artifacts: macos
+            name: macos
 
-    name: Build webrtc (${{ matrix.os }})
+    name: Build webrtc (${{ matrix.name }}-${{ matrix.arch }}-${{ matrix.profile }})
     runs-on: ${{ matrix.os }}
     steps:
+      - name: Setup vars
+        id: setup
+        run: |
+          echo "OUT=${{ matrix.name }}-${{ matrix.arch }}-${{ matrix.profile }}" >> "$GITHUB_OUTPUT"
+          echo "ZIP=webrtc-${{ matrix.name }}-${{ matrix.arch }}-${{ matrix.profile }}.zip" >> "$GITHUB_OUTPUT"
+        shell: bash
+
+      # Print some debug infos to be sure everything is ok before doing really long tasks..
+      - name: Info
+        run: |
+          echo "OutName: ${{ steps.setup.outputs.OUT }}"
+          echo "OutZip: ${{ steps.setup.outputs.ZIP }}"
+
       - uses: actions/checkout@v3
         with:
           submodules: true
@@ -33,22 +55,34 @@ jobs:
         if: ${{ matrix.os == 'macos-latest' }}
         run: brew install ninja
 
+      # It doesn't seem to be used?
       - name: Install windows dependencies
         if: ${{ matrix.os == 'windows-latest' }}
         run: |
           Invoke-WebRequest -Uri "https://github.com/ninja-build/ninja/releases/latest/download/ninja-win.zip" -OutFile ninja.zip
           Expand-Archive -Path ninja.zip -DestinationPath ninja
-          echo "${{github.workspace}}\ninja" >> $GITHUB_PATH
+          echo "${{ github.workspace }}\ninja" >> $GITHUB_PATH
 
       - name: Print ninja version
         run: ninja --version
 
-      - name: Build
-        run: ${{ matrix.cmd }}
+      - name: Build WebRTC
+        run: ${{ matrix.cmd }} --arch ${{ matrix.arch }} --profile ${{ matrix.profile }}
         working-directory: webrtc-sys/libwebrtc
 
+      - name: Zip artifact (Unix)
+        if: ${{ matrix.os != 'windows-latest' }}
+        run: |
+          cd webrtc-sys/libwebrtc/${{ steps.setup.outputs.OUT }}
+          zip ${{ github.workspace }}/${{ steps.setup.outputs.ZIP }} ./* -r
+
+      - name: Zip artifact (Windows)
+        if: ${{ matrix.os == 'windows-latest' }}
+        run: Compress-Archive -Path .\webrtc-sys\libwebrtc\${{ steps.setup.outputs.OUT }}\* -DestinationPath ${{ steps.setup.outputs.ZIP }}
+
+      # doublezip here but I don't think there is an alternative
       - name: Upload artifacts
         uses: actions/upload-artifact@v3
         with:
-          name: builds-${{ matrix.artifacts }}
-          path: webrtc-sys/libwebrtc/${{ matrix.artifacts }}
+          name: ${{ steps.setup.outputs.ZIP }}
+          path: ${{ steps.setup.outputs.ZIP }}

livekit-webrtc/src/native/peer_connection.rs

@@ -110,6 +110,7 @@ impl From<sys_pc::ffi::SignalingState> for SignalingState {
 
 #[derive(Clone)]
 pub struct PeerConnection {
+    #[allow(dead_code)]
     native_observer: SharedPtr<sys_pc::ffi::NativePeerConnectionObserver>,
     observer: Arc<PeerObserver>,
 

livekit-webrtc/src/native/rtp_parameters.rs

@@ -1,5 +1,4 @@
 use crate::rtp_parameters::*;
-use crate::MediaType;
 use webrtc_sys::rtp_parameters as sys_rp;
 use webrtc_sys::webrtc as sys_webrtc;
 

livekit-webrtc/src/native/rtp_transceiver.rs

@@ -5,7 +5,6 @@ use crate::rtp_receiver;
 use crate::rtp_sender;
 use crate::rtp_transceiver::RtpTransceiverDirection;
 use crate::rtp_transceiver::RtpTransceiverInit;
-use crate::MediaType;
 use crate::RtcError;
 use cxx::SharedPtr;
 use webrtc_sys::rtc_error as sys_err;
@@ -33,7 +32,6 @@ impl From<RtpTransceiverDirection> for sys_webrtc::ffi::RtpTransceiverDirection
             RtpTransceiverDirection::RecvOnly => Self::RecvOnly,
             RtpTransceiverDirection::Inactive => Self::Inactive,
             RtpTransceiverDirection::Stopped => Self::Stopped,
-            _ => panic!("unknown RtpTransceiverDirection"),
         }
     }
 }

livekit-webrtc/src/peer_connection.rs

@@ -238,3 +238,84 @@ impl Debug for PeerConnection {
             .finish()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::peer_connection::*;
+    use crate::peer_connection_factory::*;
+    use log::trace;
+    use tokio::sync::mpsc;
+
+    fn init_log() {
+        let _ = env_logger::builder().is_test(true).try_init();
+    }
+
+    #[tokio::test]
+    async fn create_pc() {
+        init_log();
+
+        let factory = PeerConnectionFactory::default();
+        let config = RtcConfiguration {
+            ice_servers: vec![IceServer {
+                urls: vec!["stun:stun1.l.google.com:19302".to_string()],
+                username: "".into(),
+                password: "".into(),
+            }],
+            continual_gathering_policy: ContinualGatheringPolicy::GatherOnce,
+            ice_transport_type: IceTransportsType::All,
+        };
+
+        let bob = factory.create_peer_connection(config.clone()).unwrap();
+        let alice = factory.create_peer_connection(config.clone()).unwrap();
+
+        let (bob_ice_tx, mut bob_ice_rx) = mpsc::channel::<IceCandidate>(16);
+        let (alice_ice_tx, mut alice_ice_rx) = mpsc::channel::<IceCandidate>(16);
+        let (alice_dc_tx, mut alice_dc_rx) = mpsc::channel::<DataChannel>(16);
+
+        bob.on_ice_candidate(Some(Box::new(move |candidate| {
+            bob_ice_tx.blocking_send(candidate).unwrap();
+        })));
+
+        alice.on_ice_candidate(Some(Box::new(move |candidate| {
+            alice_ice_tx.blocking_send(candidate).unwrap();
+        })));
+
+        alice.on_data_channel(Some(Box::new(move |dc| {
+            alice_dc_tx.blocking_send(dc).unwrap();
+        })));
+
+        let bob_dc = bob
+            .create_data_channel("test_dc", DataChannelInit::default())
+            .unwrap();
+
+        let offer = bob.create_offer(OfferOptions::default()).await.unwrap();
+        trace!("Bob offer: {:?}", offer);
+        bob.set_local_description(offer.clone()).await.unwrap();
+        alice.set_remote_description(offer).await.unwrap();
+
+        let answer = alice.create_answer(AnswerOptions::default()).await.unwrap();
+        trace!("Alice answer: {:?}", answer);
+        alice.set_local_description(answer.clone()).await.unwrap();
+        bob.set_remote_description(answer).await.unwrap();
+
+        let bob_ice = bob_ice_rx.recv().await.unwrap();
+        let alice_ice = alice_ice_rx.recv().await.unwrap();
+
+        bob.add_ice_candidate(alice_ice).await.unwrap();
+        alice.add_ice_candidate(bob_ice).await.unwrap();
+
+        let (data_tx, mut data_rx) = mpsc::channel::<String>(1);
+        let alice_dc = alice_dc_rx.recv().await.unwrap();
+        alice_dc.on_message(Some(Box::new(move |buffer| {
+            data_tx
+                .blocking_send(String::from_utf8_lossy(buffer.data).to_string())
+                .unwrap();
+        })));
+
+        bob_dc.send(b"This is a test", true).unwrap();
+        assert_eq!(data_rx.recv().await.unwrap(), "This is a test");
+
+        alice.close();
+        bob.close();
+    }
+}

webrtc-sys/build.rs

@@ -1,17 +1,22 @@
 use regex::Regex;
 use std::env;
 use std::fs;
+use std::io::BufRead;
 use std::io::{self, Write};
 use std::path;
 use std::process::Command;
 
 const WEBRTC_TAG: &str = "webrtc-beb0471";
+const IGNORE_DEFINES: [&str; 2] = ["CR_CLANG_REVISION", "CR_XCODE_VERSION"];
 
 fn download_prebuilt_webrtc(
     out_path: path::PathBuf,
 ) -> Result<path::PathBuf, Box<dyn std::error::Error>> {
     let target_arch = env::var("CARGO_CFG_TARGET_ARCH").unwrap();
     let target_os = env::var("CARGO_CFG_TARGET_OS").unwrap();
+
+    // This is not yet supported on all platforms.
+    // On Windows, we need Rust to link against libcmtd.
     let use_debug = {
         let var = env::var("LK_DEBUG_WEBRTC");
         var.is_ok() && var.unwrap() == "true"
@@ -95,19 +100,26 @@ fn main() {
         let var = env::var("LK_CUSTOM_WEBRTC");
         var.is_ok() && var.unwrap() == "true"
     };
+    println!("cargo:rerun-if-env-changed=LK_CUSTOM_WEBRTC");
 
-    let (webrtc_include, webrtc_lib) = if use_custom_webrtc {
+    let (webrtc_dir, webrtc_include, webrtc_lib) = if use_custom_webrtc {
         // Use a local WebRTC version (libwebrtc folder)
         let webrtc_dir = path::PathBuf::from("./libwebrtc");
-        (webrtc_dir.join("src"), webrtc_dir.join("src/out/Dev/obj"))
+        (
+            webrtc_dir.clone(),
+            webrtc_dir.join("include"),
+            webrtc_dir.join("lib"),
+        )
     } else {
         // Download a prebuilt version of WebRTC
         let download_dir = env::var("OUT_DIR").unwrap() + "/webrtc-sdk";
         let webrtc_dir = download_prebuilt_webrtc(path::PathBuf::from(download_dir)).unwrap();
-
-        (webrtc_dir.join("include"), webrtc_dir.join("lib"))
+        (
+            webrtc_dir.clone(),
+            webrtc_dir.join("include"),
+            webrtc_dir.join("lib"),
+        )
     };
-    println!("cargo:rerun-if-env-changed=LK_CUSTOM_WEBRTC");
 
     // Just required for the bridge build to succeed.
     let includes = &[
@@ -140,23 +152,25 @@ fn main() {
         "src/helper.rs",
     ]);
 
-    builder.file("src/peer_connection.cpp");
-    builder.file("src/peer_connection_factory.cpp");
-    builder.file("src/media_stream.cpp");
-    builder.file("src/data_channel.cpp");
-    builder.file("src/jsep.cpp");
-    builder.file("src/candidate.cpp");
-    builder.file("src/rtp_receiver.cpp");
-    builder.file("src/rtp_sender.cpp");
-    builder.file("src/rtp_transceiver.cpp");
-    builder.file("src/rtp_parameters.cpp");
-    builder.file("src/rtc_error.cpp");
-    builder.file("src/webrtc.cpp");
-    builder.file("src/video_frame.cpp");
-    builder.file("src/video_frame_buffer.cpp");
-    builder.file("src/video_encoder_factory.cpp");
-    builder.file("src/video_decoder_factory.cpp");
-    builder.file("src/audio_device.cpp");
+    builder.files(&[
+        "src/peer_connection.cpp",
+        "src/peer_connection_factory.cpp",
+        "src/media_stream.cpp",
+        "src/data_channel.cpp",
+        "src/jsep.cpp",
+        "src/candidate.cpp",
+        "src/rtp_receiver.cpp",
+        "src/rtp_sender.cpp",
+        "src/rtp_transceiver.cpp",
+        "src/rtp_parameters.cpp",
+        "src/rtc_error.cpp",
+        "src/webrtc.cpp",
+        "src/video_frame.cpp",
+        "src/video_frame_buffer.cpp",
+        "src/video_encoder_factory.cpp",
+        "src/video_decoder_factory.cpp",
+        "src/audio_device.cpp",
+    ]);
 
     for include in includes {
         builder.include(include);
@@ -167,6 +181,20 @@ fn main() {
         webrtc_lib.canonicalize().unwrap().to_str().unwrap()
     );
 
+    // Read preprocessor definitions from webrtc.ninja
+    let webrtc_gni = fs::File::open(webrtc_dir.join("webrtc.ninja")).unwrap();
+    let mut reader = io::BufReader::new(webrtc_gni).lines();
+    let defines_line = reader.next().unwrap().unwrap(); // The first line contains the defines
+    let defines_re = Regex::new(r"-D(\w+)(?:=([^\s]+))?").unwrap();
+    for cap in defines_re.captures_iter(&defines_line) {
+        let define_name = &cap[1];
+        let define_value = cap.get(2).map(|m| m.as_str());
+        if IGNORE_DEFINES.contains(&define_name) {
+            continue;
+        }
+        builder.define(define_name, define_value);
+    }
+
     let target_os = env::var("CARGO_CFG_TARGET_OS").unwrap();
     match target_os.as_str() {
         "windows" => {
@@ -186,12 +214,7 @@ fn main() {
             println!("cargo:rustc-link-lib=dylib=dwmapi");
             println!("cargo:rustc-link-lib=static=webrtc");
 
-            builder
-                .flag("/std:c++17")
-                .flag("/EHsc")
-                .define("WEBRTC_WIN", None)
-                //.define("WEBRTC_ENABLE_SYMBOL_EXPORT", None) Not necessary when using WebRTC as a static library
-                .define("NOMINMAX", None);
+            builder.flag("/std:c++17").flag("/EHsc");
         }
         "linux" => {
             println!("cargo:rustc-link-lib=dylib=Xext");
@@ -203,10 +226,7 @@ fn main() {
             println!("cargo:rustc-link-lib=dylib=m");
             println!("cargo:rustc-link-lib=static=webrtc");
 
-            builder
-                .flag("-std=c++17")
-                .define("WEBRTC_POSIX", None)
-                .define("WEBRTC_LINUX", None);
+            builder.flag("-std=c++17");
         }
         "macos" => {
             println!("cargo:rustc-link-lib=framework=Foundation");
@@ -254,19 +274,10 @@ fn main() {
             builder
                 .flag("-stdlib=libc++")
                 .flag("-std=c++17")
-                .flag(format!("-isysroot{}", sysroot).as_str())
-                .define("WEBRTC_ENABLE_OBJC_SYMBOL_EXPORT", None)
-                .define("WEBRTC_POSIX", None)
-                .define("WEBRTC_MAC", None);
+                .flag(format!("-isysroot{}", sysroot).as_str());
         }
         "ios" => {
-            builder
-                .flag("-std=c++17")
-                .file("src/objc_test.mm")
-                .define("WEBRTC_ENABLE_OBJC_SYMBOL_EXPORT", None)
-                .define("WEBRTC_MAC", None)
-                .define("WEBRTC_POSIX", None)
-                .define("WEBRTC_IOS", None);
+            builder.flag("-std=c++17");
         }
         "android" => {
             let ndk_env = env::var("ANDROID_NDK_HOME").expect(
@@ -330,11 +341,7 @@ fn main() {
                 vs_path.to_str().unwrap()
             );
 
-            builder
-                .flag("-std=c++17")
-                .define("WEBRTC_LINUX", None)
-                .define("WEBRTC_POSIX", None)
-                .define("WEBRTC_ANDROID", None);
+            builder.flag("-std=c++17");
         }
         _ => {
             panic!("Unsupported target, {}", target_os);