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

Add soxr library #7728

Merged
merged 15 commits into from
Oct 25, 2021
8 changes: 8 additions & 0 deletions recipes/soxr/all/conandata.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
sources:
"0.1.3":
url: https://sourceforge.net/projects/soxr/files/soxr-0.1.3-Source.tar.xz
sha1: "32ea46b1a8c0c15f835422892d02fce8286aec3c"
patches:
"0.1.3":
- base_path: source_subfolder
patch_file: "patches/findpackage-openmp.patch"
91 changes: 91 additions & 0 deletions recipes/soxr/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import os
from conans import ConanFile, CMake, tools


class SoxrConan(ConanFile):
name = "soxr"
description = "The SoX Resampler library libsoxr performs fast, high-quality one-dimensional sample rate conversion."
homepage = "https://sourceforge.net/projects/soxr/"
topics = ("resampling", "audio", "sample-rate", "conversion")
license = "LGPL-2.1-or-later"
url = "https://github.com/conan-io/conan-center-index"
settings = "os", "compiler", "build_type", "arch"
options = {
"shared": [True, False],
"fPIC": [True, False],
"with_openmp": [True, False],
"with_lsr_bindings": [True, False]
Copy link
Contributor

@madebr madebr Oct 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option will create a soxr-lsr library (see bottom of this cmake script)
It will also create a soxr-lsr.pc file.

So you will probably need to use modules in package_info.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split it up into two components "core" and "lsr". Please have a look: fc13703

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for your contribution!

}
default_options = {
"shared": False,
"fPIC": True,
"with_openmp": False,
"with_lsr_bindings": True
}
generators = "cmake"
exports_sources = ["patches/**", "licenses/**"]

_cmake = None

@property
def _source_subfolder(self):
return "source_subfolder"

@property
def _build_subfolder(self):
return "build_subfolder"

def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC

def configure(self):
if self.options.shared:
del self.options.fPIC
del self.settings.compiler.cppstd
del self.settings.compiler.libcxx

def source(self):
tools.get(**self.conan_data["sources"][self.version],
destination=self._source_subfolder, strip_root=True)

def _configure_cmake(self):
if self._cmake:
return self._cmake
self._cmake = CMake(self)
if self.settings.compiler == "Visual Studio":
self._cmake.definitions["BUILD_SHARED_RUNTIME"] = "MD" in self.settings.compiler.runtime
elif self.settings.compiler == "msvc":
self._cmake.definitions["BUILD_SHARED_RUNTIME"] = self.settings.compiler.runtime == "dynamic"
self._cmake.definitions["BUILD_TESTS"] = False
self._cmake.definitions["WITH_OPENMP"] = self.options.with_openmp
self._cmake.definitions["WITH_LSR_BINDINGS"] = self.options.with_lsr_bindings
self._cmake.configure(source_folder=self._source_subfolder, build_folder=self._build_subfolder)
return self._cmake

def build(self):
for patch in self.conan_data.get("patches", {}).get(self.version, []):
tools.patch(**patch)
cmake = self._configure_cmake()
cmake.build()

def package(self):
self.copy("LICENCE", dst="licenses", src=self._source_subfolder)
self.copy("licenses/pffft", dst="licenses", keep_path=False)
cmake = self._configure_cmake()
cmake.install()
tools.rmdir(os.path.join(self.package_folder, "doc"))

def package_info(self):
self.cpp_info.libs = ["soxr"]
chausner marked this conversation as resolved.
Show resolved Hide resolved
if not self.options.shared and self.options.with_openmp:
chausner marked this conversation as resolved.
Show resolved Hide resolved
if self.settings.compiler in ("Visual Studio", "msvc"):
openmp_flags = ["/openmp"]
chausner marked this conversation as resolved.
Show resolved Hide resolved
elif self.settings.compiler in ("gcc", "clang"):
openmp_flags = ["-fopenmp"]
elif self.settings.compiler == "apple-clang":
openmp_flags = ["-Xpreprocessor", "-fopenmp"]
else:
openmp_flags = []
self.cpp_info.cxxflags = openmp_flags
chausner marked this conversation as resolved.
Show resolved Hide resolved
self.cpp_info.sharedlinkflags = openmp_flags
52 changes: 52 additions & 0 deletions recipes/soxr/all/licenses/pffft
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
Copyright (c) 2013 Julien Pommier ( pommier@modartt.com )

Based on original fortran 77 code from FFTPACKv4 from NETLIB
(http://www.netlib.org/fftpack), authored by Dr Paul Swarztrauber
of NCAR, in 1985.

As confirmed by the NCAR fftpack software curators, the following
FFTPACKv5 license applies to FFTPACKv4 sources. My changes are
released under the same terms.

FFTPACK license:

http://www.cisl.ucar.edu/css/software/fftpack5/ftpk.html

Copyright (c) 2004 the University Corporation for Atmospheric
Research ("UCAR"). All rights reserved. Developed by NCAR's
Computational and Information Systems Laboratory, UCAR,
www.cisl.ucar.edu.

Redistribution and use of the Software in source and binary forms,
with or without modification, is permitted provided that the
following conditions are met:

- Neither the names of NCAR's Computational and Information Systems
Laboratory, the University Corporation for Atmospheric Research,
nor the names of its sponsors or contributors may be used to
endorse or promote products derived from this Software without
specific prior written permission.

- Redistributions of source code must retain the above copyright
notices, this list of conditions, and the disclaimer below.

- Redistributions in binary form must reproduce the above copyright
notice, this list of conditions, and the disclaimer below in the
documentation and/or other materials provided with the
distribution.

THIS SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR IMPLIED, INCLUDING, BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
NONINFRINGEMENT. IN NO EVENT SHALL THE CONTRIBUTORS OR COPYRIGHT
HOLDERS BE LIABLE FOR ANY CLAIM, INDIRECT, INCIDENTAL, SPECIAL,
EXEMPLARY, OR CONSEQUENTIAL DAMAGES OR OTHER LIABILITY, WHETHER IN AN
ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS WITH THE
SOFTWARE.


PFFFT : a Pretty Fast FFT.

This file is largerly based on the original FFTPACK implementation, modified in
order to take advantage of SIMD instructions of modern CPUs.
15 changes: 15 additions & 0 deletions recipes/soxr/all/patches/findpackage-openmp.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Adds the REQUIRED flag to find_package for OpenMP so that
we can be sure in the conanfile that if with_openmp is set
that OpenMP is indeed used. This is important since we need to add
compiler flags in package_info() in this case.
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -105,7 +105,7 @@ if (${BUILD_EXAMPLES})
endif ()

if (WITH_OPENMP)
- find_package (OpenMP)
+ find_package (OpenMP REQUIRED)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it required? It is an option in the recipe, what happens if soxr:with_openmp=False?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment at the top of recipes/soxr/all/patches/findpackage-openmp.patch. I am setting the "REQUIRED" flag because I want CMake to fail early if users set "with_openmp=True" and CMake cannot find the package. Otherwise, the build would succeed and later builds would fail when attempting to link to libsoxr because the compiler flags are still getting set for OpenMP in package_info.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be even better to check it on Conan side, the message to the user could be better and the check performed even before trying to build anything. Also, Conan can run the check even if the binary is already available, when a consumer tries to use this recipe (and the compiler flag is propagated), not just when it is being built.

Is this some library we can build from sources (we have llvm-openmp)? Is this one use-case for a openmp/system package?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be even better to check it on Conan side,

Yes, I agree it would be better to check on the Conan side. But I don't think there is a reliable way to check for the presence of a CMake package from within Conan (without invoking CMake itself), is there? What do you suggest in this case?

Is this some library we can build from sources (we have llvm-openmp)? Is this one use-case for a openmp/system package?

Hmm, normally OpenMP is not built from sources. On Linux and macOS, it's commonly installed as a system package. On Windows, it comes with Visual Studio. I am not familiar with llvm-openmp but I doubt this would be the package you normally would want to use on Windows, for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be even better to check it on Conan side,

Yes, I agree it would be better to check on the Conan side. But I don't think there is a reliable way to check for the presence of a CMake package from within Conan (without invoking CMake itself), is there? What do you suggest in this case?

The build system should follow instructions from the package-manager. If the package manager is configured with with_openmp=True, it should pass the option to the build-system and this one should find and use it or fail (the REQUIRED is a good thing). We try to avoid auto-detection as much as possible, because it makes the process uncertain and not reproducible.

If OpenMP is expected to be in the system, then the best way to go is to create a system package, Conan will check if the package is available in the system (using some underlying tool like pkg_config) and generate the proper FindOpenMP.cmake to use it from system libraries.... or fail if the package is required and not available.

It would work like any other requirement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(the REQUIRED is a good thing)

Okay, yes I agree and might have misunderstood you here.

If OpenMP is expected to be in the system, then the best way to go is to create a system package,

I see. Unfortunately, I will not be able to contribute that one. I am surprised it does not exist yet, as there should be more existing packages in Conan Center that depend on OpenMP in some way.

For now, I suggest to either go with the current solution in this PR, or if you prefer, I can also completely remove the with_openmp option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there are other packages in ConanCenter with a with_openmp option and all of them default to False, like yours, and it is ok. We can leave it as it is, probably it is something to be addressed in the future and, at that point, we can improve all the recipes.

My initial concern about this issue was if creating (and maintaining) a patch to add the REQUIRED keyword was worth it or not, because it is the only patch in this recipe and the only line changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks fine to me.

if (OPENMP_FOUND)
set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
8 changes: 8 additions & 0 deletions recipes/soxr/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
cmake_minimum_required(VERSION 3.1)
project(test_package C)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup()

add_executable(${PROJECT_NAME} test_package.c)
target_link_libraries(${PROJECT_NAME} ${CONAN_LIBS})
chausner marked this conversation as resolved.
Show resolved Hide resolved
16 changes: 16 additions & 0 deletions recipes/soxr/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import os
from conans import ConanFile, CMake, tools

class TestPackageConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
generators = "cmake", "cmake_find_package"
chausner marked this conversation as resolved.
Show resolved Hide resolved

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self):
bin_path = os.path.join("bin", "test_package")
self.run(bin_path, run_environment=True)
7 changes: 7 additions & 0 deletions recipes/soxr/all/test_package/test_package.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <soxr.h>
#include <stdio.h>

int main() {
printf("soxr version: %s\n", soxr_version());
return 0;
}
3 changes: 3 additions & 0 deletions recipes/soxr/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
versions:
"0.1.3":
folder: all