Resolve conflict between link_system_ffmpeg and link_vcpkg_ffmpeg…

… features (#119) * Remove conflict between `link_system_ffmpeg` and `link_vcpkg_ffmpeg` * Revisit README * Fix macOS CI
CCExtractor · Jul 22, 2024 · 4011018 · 4011018
1 parent 84f7a9e
commit 4011018
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 47 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -124,7 +124,7 @@ jobs:
           VCPKG_ROOT: ${{ github.workspace }}/vcpkg
           LIBCLANG_PATH: ${{ github.workspace }}/clang/lib
           LLVM_CONFIG_PATH: ${{ github.workspace }}/clang/bin/llvm-config
-        run: cargo clippy -- -D warnings
+        run: cargo clippy --features link_vcpkg_ffmpeg -- -D warnings
 
   build_static_and_test_ubuntu_with_system_ffmpeg:
     runs-on: ubuntu-latest
@@ -389,7 +389,7 @@ jobs:
           VCPKG_DEFAULT_TRIPLET: ${{ matrix.config.vcpkg_triplet }}
           LIBCLANG_PATH: ${{ github.workspace }}/clang/lib
           LLVM_CONFIG_PATH: ${{ github.workspace }}/clang/bin/llvm-config
-        run: cargo build --target ${{ matrix.config.target }} --verbose
+        run: cargo build --features link_vcpkg_ffmpeg --target ${{ matrix.config.target }} --verbose
 
   build_dynamic_and_test_ubuntu:
     runs-on: ubuntu-latest
@@ -525,7 +525,7 @@ jobs:
             --enable-gpl \
             --enable-libx264 \
             --enable-pic
-          make -j$(nproc)
+          make -j$(sysctl -n hw.logicalcpu)
           make install
           cd build/lib/
           cp ~/x264_prebuilt/lib/libx264.a .

diff --git a/README.md b/README.md
@@ -5,7 +5,7 @@
 [![Downloads](https://img.shields.io/crates/d/rusty_ffmpeg)](https://lib.rs/crates/rusty_ffmpeg)
 [![CI](https://github.com/CCExtractor/rusty_ffmpeg/workflows/CI/badge.svg?branch=master&style=flat-square)](https://github.com/CCExtractor/rusty_ffmpeg/actions)
 
-Cross platform FFI bindings for FFmpeg inner libraries. This is a crate that:
+Cross platform FFI bindings for FFmpeg internal libraries. This is a crate that:
 
 1. Links FFmpeg libraries for you.
 2. Generates Rust binding for FFmpeg libraries.
@@ -16,37 +16,43 @@ To use this crate, you need to set several environment variables.
 
 ### The simplest usage:
 
-#### *nix
+#### Linux, macOS..(*nix)
 
-Build ffmpeg statically and set `FFMPEG_PKG_CONFIG_PATH` to the path of the generated FFmpeg `pkg-config` files. And you don't need to set other environment variables for static linking.
+If you have FFmpeg installed with package manager, import `rusty_ffmpeg` with feature `link_system_ffmpeg`. Then it should work.
 
-(Hint: setting `FFMPEG_PKG_CONFIG_PATH` to some placeholder value will leave a `rusty_ffmpeg` probing system library.)
+If you built FFmpeg from source, set `FFMPEG_PKG_CONFIG_PATH` to the path of the generated FFmpeg `pkg-config` directory. Then it should work.
 
 #### Windows
 
-`rusty_ffmpeg` can link FFmpeg using `vcpkg`. Install [`vcpkg`](https://github.com/microsoft/vcpkg), check [documentation of the vcpkg *crate*](https://docs.rs/vcpkg) for the environment variables to set, then it works.
+`rusty_ffmpeg` can link FFmpeg using `vcpkg`:
+1. Install [`vcpkg`](https://github.com/microsoft/vcpkg), check [documentation of the vcpkg *crate*](https://docs.rs/vcpkg) for the environment variables to set.
+2. Import `rusty_ffmpeg` with feature `link_vcpkg_ffmpeg`, Then it should work.
 
 ### Fine-grained usage:
 
-You need to set several environment variables for both linking and binding generating procedures.
+You need to set several environment variables for both the linking and binding generating procedures.
 
-#### To link: 
+#### To link prebuilt libraries: 
 
-1. Dynamic linking with pre-built dylib: Set `FFMPEG_DLL_PATH` to the path of `dll` or `so`. (Windows: Put corresponding `.lib` file next to the `.dll` file.)
+1. Dynamic linking with pre-built dylib: Set `FFMPEG_DLL_PATH` to the path of `dll` or `so` files. (Windows: Put corresponding `.lib` file next to the `.dll` file.)
 
-2. Static linking with pre-built staticlib: Set `FFMPEG_LIBS_DIR` to the path of the FFmpeg pre-built libs directory.
+2. Static linking with pre-built staticlib: Set `FFMPEG_LIBS_DIR` to the path of FFmpeg pre-built libs directory.
 
 #### To generate bindings: 
 
-1. Compile-time binding generation([requires the `Clang` dylib](https://github.com/KyleMayes/clang-sys/blob/c9ae24a7a218e73e1eccd320174349eef5a3bd1a/build.rs#L23)): Set `FFMPEG_INCLUDE_DIR` to the path to the header files for binding generation.
+1. Compile-time binding generation([requires the `Clang` dylib](https://github.com/KyleMayes/clang-sys/blob/c9ae24a7a218e73e1eccd320174349eef5a3bd1a/build.rs#L23)): Set `FFMPEG_INCLUDE_DIR` to the path of the header files for binding generation.
 
-2. Use your pre-built binding: Set `FFMPEG_BINDING_PATH` to the pre-built binding file. The pre-built binding is usually copied from the `OUT_DIR` of the compile-time binding generation, by using it you don't need to regenerate the same binding file again and again.
+2. Use your prebuilt binding: Set `FFMPEG_BINDING_PATH` to the pre-built binding file. The pre-built binding is usually copied from the `OUT_DIR` of the compile-time binding generation, using it will prevent the need to regenerate the same binding file repeatedly.
 
 ### Linking FFmpeg installed by package manager on (*nix)
 
-You can enable system-wide FFmpeg linking by enabling feature `link_system_ffmpeg`.
+You can link FFmpeg libraries installed by package manager by enabling feature `link_system_ffmpeg` (which uses pkg-config underneath).
 
-### Use specific FFmpeg version
+### Linking FFmpeg installed by vcpkg
+
+You can link FFmpeg libraries installed by vcpkg by enabling feature `link_vcpkg_ffmpeg` on Windows, macOS, and Linux.
+
+### Use a specific FFmpeg version
 
 - Do nothing when you are using FFmpeg `4.*`
 - Enable `ffmpeg5` feature when you are using FFmpeg `5.*`
@@ -55,4 +61,4 @@ You can enable system-wide FFmpeg linking by enabling feature `link_system_ffmpe
 
 ## Attention
 
-FFI is not that easy, especially when you are dealing with a big old C project. Don't feel depressed when there are some problems. The CI check already has some typical ffmpeg compilation and use cases for you to check. File an issue if you still have any problem.
+FFI is not that easy, especially when you are dealing with a big old C project. Don't get discouraged if you encounter some problems. The CI check already has some typical ffmpeg compilation and use cases for you to check. File an issue if you still have any problems.
diff --git a/build.rs b/build.rs
@@ -290,18 +290,29 @@ fn remove_verbatim(path: String) -> PathBuf {
 }
 
 #[cfg(not(target_os = "windows"))]
-mod non_windows {
+mod pkg_config_linking {
     use super::*;
 
-    pub fn static_linking_with_pkg_config(library_names: &[&str]) -> Vec<PathBuf> {
-        // TODO: if specific library is not enabled, we should not probe it. If we
-        // want to implement this, we Should modify try_probe_system_ffmpeg() too.
+    /// Returns error when some library are missing. Otherwise, returns the paths of the libraries.
+    ///
+    /// Note: no side effect if this function errors.
+    pub fn linking_with_pkg_config(
+        library_names: &[&str],
+    ) -> Result<Vec<PathBuf>, pkg_config::Error> {
+        // dry run for library linking
+        for libname in library_names {
+            pkg_config::Config::new()
+                .cargo_metadata(false)
+                .env_metadata(false)
+                .print_system_libs(false)
+                .print_system_cflags(false)
+                .probe(&format!("lib{}", libname))?;
+        }
+
+        // real linking
         let mut paths = HashSet::new();
         for libname in library_names {
             let new_paths = pkg_config::Config::new()
-                // currently only support building with static libraries.
-                .statik(true)
-                .cargo_metadata(true)
                 .probe(&format!("lib{}", libname))
                 .unwrap_or_else(|_| panic!("{} not found!", libname))
                 .include_paths;
@@ -310,36 +321,39 @@ mod non_windows {
                 paths.insert(new_path);
             }
         }
-        paths.into_iter().map(PathBuf::from).collect()
+        Ok(paths.into_iter().map(PathBuf::from).collect())
     }
 }
 
-#[cfg(all(feature = "link_vcpkg_ffmpeg", feature = "link_system_ffmpeg"))]
-compile_error!("Features link_vcpkg_ffmpeg and link_system_ffmpeg features are mutually exclusive and cannot be enabled together.");
-
-#[cfg(any(feature = "link_vcpkg_ffmpeg", target_os = "windows"))]
+#[cfg(feature = "link_vcpkg_ffmpeg")]
 mod vcpkg_linking {
     use super::*;
 
-    fn static_linking_vcpkg(_env_vars: &EnvVars, _library_names: &[&str]) -> Vec<PathBuf> {
-        vcpkg::Config::new()
-            .find_package("ffmpeg")
-            .unwrap()
+    fn linking_with_vcpkg(
+        _env_vars: &EnvVars,
+        _library_names: &[&str],
+    ) -> Result<Vec<PathBuf>, vcpkg::Error> {
+        Ok(vcpkg::Config::new()
+            .find_package("ffmpeg")?
             .include_paths
             .into_iter()
             .map(|x| PathBuf::from_path_buf(x).unwrap())
-            .collect()
+            .collect())
     }
 
-    pub fn generate_vcpkg_bindings(env_vars: &EnvVars, output_binding_path: &Path) {
-        let include_paths = static_linking_vcpkg(env_vars, &*LIBS);
+    pub fn linking_with_vcpkg_and_bindgen(
+        env_vars: &EnvVars,
+        output_binding_path: &Path,
+    ) -> Result<(), vcpkg::Error> {
+        let include_paths = linking_with_vcpkg(env_vars, &*LIBS)?;
         if let Some(ffmpeg_binding_path) = env_vars.ffmpeg_binding_path.as_ref() {
             use_prebuilt_binding(ffmpeg_binding_path, output_binding_path);
         } else {
             generate_bindings(&include_paths[0], &HEADERS)
                 .write_to_file(output_binding_path)
                 .expect("Cannot write binding to file.");
         }
+        Ok(())
     }
 }
 
@@ -385,9 +399,12 @@ fn static_linking(env_vars: &EnvVars) {
 
     #[cfg(not(target_os = "windows"))]
     {
-        fn link_and_bindgen(env_vars: &EnvVars, output_binding_path: &Path) {
+        fn static_linking_with_pkg_config_and_bindgen(
+            env_vars: &EnvVars,
+            output_binding_path: &Path,
+        ) -> Result<(), pkg_config::Error> {
             // Probe libraries(enable emitting cargo metadata)
-            let include_paths = static_linking_with_pkg_config(&*LIBS);
+            let include_paths = linking_with_pkg_config(&*LIBS)?;
             if let Some(ffmpeg_binding_path) = env_vars.ffmpeg_binding_path.as_ref() {
                 use_prebuilt_binding(ffmpeg_binding_path, output_binding_path);
             } else if let Some(ffmpeg_include_dir) = env_vars.ffmpeg_include_dir.as_ref() {
@@ -400,8 +417,9 @@ fn static_linking(env_vars: &EnvVars) {
                     .write_to_file(output_binding_path)
                     .expect("Cannot write binding to file.");
             }
+            Ok(())
         }
-        use non_windows::*;
+        use pkg_config_linking::*;
         // Hint: set PKG_CONFIG_PATH to some placeholder value will let pkg_config probing system library.
         if let Some(ffmpeg_pkg_config_path) = env_vars.ffmpeg_pkg_config_path.as_ref() {
             if !Path::new(ffmpeg_pkg_config_path).exists() {
@@ -411,7 +429,8 @@ fn static_linking(env_vars: &EnvVars) {
                 );
             }
             env::set_var("PKG_CONFIG_PATH", ffmpeg_pkg_config_path);
-            link_and_bindgen(env_vars, output_binding_path);
+            static_linking_with_pkg_config_and_bindgen(env_vars, output_binding_path)
+                .expect("Static linking with pkg-config failed.");
         } else if let Some(ffmpeg_libs_dir) = env_vars.ffmpeg_libs_dir.as_ref() {
             static_linking_with_libs_dir(&*LIBS, ffmpeg_libs_dir);
             if let Some(ffmpeg_binding_path) = env_vars.ffmpeg_binding_path.as_ref() {
@@ -424,13 +443,48 @@ fn static_linking(env_vars: &EnvVars) {
                 panic!("No binding generation method is set!");
             }
         } else {
-            #[cfg(feature = "link_system_ffmpeg")]
-            link_and_bindgen(env_vars, output_binding_path);
-            #[cfg(feature = "link_vcpkg_ffmpeg")]
-            vcpkg_linking::generate_vcpkg_bindings(env_vars, output_binding_path);
             #[cfg(not(any(feature = "link_system_ffmpeg", feature = "link_vcpkg_ffmpeg")))]
-            panic!("No linking method set!");
-        };
+            panic!(
+                "
+!!!!!!! rusty_ffmpeg: No linking method set!
+Use FFMPEG_PKG_CONFIG_PATH or FFMPEG_LIBS_DIR if you have prebuilt FFmpeg libraries.
+Enable `link_system_ffmpeg` feature if you want to link ffmpeg libraries install in system path.
+Enable `link_vcpkg_ffmpeg` feature if you want to link ffmpeg provided by vcpkg.
+"
+            );
+            #[cfg(any(feature = "link_system_ffmpeg", feature = "link_vcpkg_ffmpeg"))]
+            {
+                let mut success = false;
+                let mut error = String::new();
+                #[cfg(feature = "link_system_ffmpeg")]
+                if !success {
+                    if let Err(e) =
+                        static_linking_with_pkg_config_and_bindgen(env_vars, output_binding_path)
+                    {
+                        error.push('\n');
+                        error.push_str(&format!("Link system FFmpeg failed: {:?}", e));
+                    } else {
+                        println!("Link system FFmpeg succeeded.");
+                        success = true;
+                    }
+                }
+                #[cfg(feature = "link_vcpkg_ffmpeg")]
+                if !success {
+                    if let Err(e) =
+                        vcpkg_linking::linking_with_vcpkg_and_bindgen(env_vars, output_binding_path)
+                    {
+                        error.push('\n');
+                        error.push_str(&format!("Link vcpkg FFmpeg failed: {:?}", e));
+                    } else {
+                        println!("Link vcpkg FFmpeg succeeded.");
+                        success = true;
+                    }
+                }
+                if !success {
+                    panic!("FFmpeg linking trial failed: {}", error);
+                }
+            }
+        }
     }
     #[cfg(target_os = "windows")]
     {
@@ -446,7 +500,17 @@ fn static_linking(env_vars: &EnvVars) {
                 panic!("No binding generation method is set!");
             }
         } else {
-            vcpkg_linking::generate_vcpkg_bindings(env_vars, output_binding_path);
+            #[cfg(feature = "link_vcpkg_ffmpeg")]
+            vcpkg_linking::linking_with_vcpkg_and_bindgen(env_vars, output_binding_path)
+                .expect("Linking FFmpeg with vcpkg failed.");
+            #[cfg(not(feature = "link_vcpkg_ffmpeg"))]
+            panic!(
+                "
+!!!!!!! rusty_ffmpeg: No linking method set!
+Use FFMPEG_PKG_CONFIG_PATH or FFMPEG_LIBS_DIR if you have prebuilt FFmpeg libraries.
+Enable `link_vcpkg_ffmpeg` feature if you want to link ffmpeg provided by vcpkg.
+"
+            );
         }
     }
 }