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

[AUBIO] Add new package (3/3) #1313

Closed
wants to merge 1 commit into from
Closed

Conversation

bagong
Copy link
Contributor

@bagong bagong commented Jun 19, 2017

Add package aubio. This is a nifty package for audio analysis.

This is the final PR in the chain pkg-config-waf->aubio (aubio can only detect dependencies based on pkg config)

Tested to build for x86/x64 static/dynamic md/mt. The static experience is slightly impeded by the fact that ffmpeg is fake static 😄

@ras0219-msft
Copy link
Contributor

@bagong
Copy link
Contributor Author

bagong commented Jun 20, 2017

OMG, impressive. I just read it. I'll try it tonight. Thank you so much!

@ras0219-msft
Copy link
Contributor

I pushed a fix for static and confirmed that it builds in x86-windows and x64-windows-static for me.

It only builds the library itself -- none of the executables. Is that fine? I don't know enough about the library ;p.

@bagong
Copy link
Contributor Author

bagong commented Jun 20, 2017

It would be nice to have the executables in the tools/share/aubio folder, sources in the example folder. They're good for testing and basic analyzing (they also verify that everything links correctly).

Another question: Is it maybe easy to generate the symbols with cmake rather than using that static list? Or does it still have to parse the sources? The disadvantage of that list is that it needs to be maintained separately and doesn't adjust to changing the use of deps. It's not a demand, of course, but I only settled on that static def file, because I wasn't able to handle that properly with waf ;)

@ras0219-msft
Copy link
Contributor

ras0219-msft commented Jun 20, 2017

CMake does have the ability to auto-export every symbol via CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS, which basically acts like a static lib would.

I've pushed that and did a bit of cleanup. If everything looks like it works to you, I can rebase and merge.

Edit: Btw, the def file was very useful to make sure I had enabled all the components you wanted -- it generates link errors if symbols aren't available.

@bagong
Copy link
Contributor Author

bagong commented Jun 20, 2017

Fantastic! Thank you very much.

@bagong
Copy link
Contributor Author

bagong commented Jun 20, 2017

I've built them all too now, looks great! No mercy on the tools, right ;) Well, thank you a lot anyways!! I'll take your cmake script as a template to learn cmake properly ;)

@bagong
Copy link
Contributor Author

bagong commented Jun 20, 2017

I do have a few suggestions, 1 change to link_libraries (add the transitive deps ogg, flac, vorbis, vorbisenc for static linking, I got linker errors in static builds when building the executables without them) and the option to build the 5 tiny executables in the example folder. It's just in the cmake file, portfile is unchanged ;)

One thing maybe worth a mention: I get weird warnings about libsndfile in the static build, a bunch of the ones below. They're just warnings, but two things irritate me: where does the name sndfile-static.pdb come from, should it exist? Second even weirder: "libsndfile-1.lib" - unfortunately in the libsndfile build the static library is called libsndfile.lib (no -1), whereas the import library is in fact called libsndfile-1.lib.

libsndfile-1.lib(sndfile.c.obj) : warning LNK4099: PDB 'sndfile-static.pdb' was not found with 'libsndfile-1.lib(sndfile.c.obj)' or at 'C:\vcpkg\buildtrees\aubio\x64-windows-static-rel\sndfile-static.pdb'; linking object as if no debug info

libsndfile-1.lib(common.c.obj) : warning LNK4099: PDB 'sndfile-static.pdb' was not found with 'libsndfile-1.lib(common.c.obj)' or at 'C:\vcpkg\buildtrees\aubio\x64-windows-static-rel\sndfile-static.pdb'; linking object as if no debug info

libsndfile-1.lib(command.c.obj) : warning LNK4099: PDB 'sndfile-static.pdb' was not found with 'libsndfile-1.lib(command.c.obj)' or at 'C:\vcpkg\buildtrees\aubio\x64-windows-static-rel\sndfile-static.pdb'; linking object as if no debug info

@bagong
Copy link
Contributor Author

bagong commented Jun 20, 2017

I guess I can't push to your version, so I'll post my suggestions here:

Portfile is just a single added debug option to avoid building the tools:

vcpkg_configure_cmake(
  SOURCE_PATH ${SOURCE_PATH}
  PREFER_NINJA
  OPTIONS_DEBUG -DDISABLE_INSTALL_HEADERS=1 -DBUILD_TOOLS=0
)

And I'll post the whole cmake script for simplicity:

cmake_minimum_required(VERSION 3.8)
project(aubio C)

add_definitions(
    -DHAVE_STDLIB_H=1
    -DHAVE_STDIO_H=1
    -DHAVE_MATH_H=1
    -DHAVE_STRING_H=1
    -DHAVE_LIMITS_H=1
    -DHAVE_STDARG_H=1
    -DHAVE_C99_VARARGS_MACROS=1

    -DHAVE_SNDFILE=1
    -DHAVE_WAVWRITE=1
    -DHAVE_WAVREAD=1
    -DHAVE_LIBAV=1
    -DHAVE_SWRESAMPLE=1
)

set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
option(BUILD_TOOLS "Build and install tools" ON)


find_path(LIBSNDFILE_H sndfile.h)
find_library(LIBSNDFILE_LIB NAMES libsndfile-1 libsndfile)
find_library(AVCODEC_LIB avcodec)
find_library(AVUTIL_LIB avutil)
find_library(AVDEVICE_LIB avdevice)
find_library(AVFILTER_LIB avfilter)
find_library(AVFORMAT_LIB avformat)
find_library(SWRESAMPLE_LIB swresample)
find_library(OGG_LIB ogg)
find_library(FLAC_LIB flac)
find_library(VORBIS_LIB vorbis)
find_library(VORBISENC_LIB vorbisenc)

include_directories(src ${LIBSNDFILE_H})
link_libraries(${LIBSNDFILE_LIB} ${OGG_LIB} ${FLAC_LIB} ${VORBIS_LIB} ${VORBISENC_LIB} ${AVCODEC_LIB} ${AVUTIL_LIB} ${AVDEVICE_LIB} ${AVFILTER_LIB} ${AVFORMAT_LIB} ${SWRESAMPLE_LIB} ws2_32.lib)

file(GLOB_RECURSE SOURCES src/*.c)

set_source_files_properties(src/io/sink_wavwrite.c PROPERTIES COMPILE_FLAGS /FIWinsock2.h)
set(UTIL_SOURCES examples/utils.c examples/jackio.c)

add_library(aubio ${SOURCES})


if(BUILD_TOOLS)
    set (EXAMPLE_EXECS aubiomfcc aubionotes aubioonset aubiopitch aubioquiet aubiotrack)
    foreach(EXAMPLE_EXEC ${EXAMPLE_EXECS})
        add_executable(${EXAMPLE_EXEC} examples/${EXAMPLE_EXEC}.c ${UTIL_SOURCES})
        target_link_libraries(${EXAMPLE_EXEC} PUBLIC aubio)
        target_compile_definitions(${EXAMPLE_EXEC} PUBLIC HAVE_WIN_HACKS=1)
    endforeach(EXAMPLE_EXEC)
endif()

set(CMAKE_DEBUG_POSTFIX d)

install(
    TARGETS aubio
    RUNTIME DESTINATION bin
    LIBRARY DESTINATION lib
    ARCHIVE DESTINATION lib
)

if(NOT DISABLE_INSTALL_HEADERS)
    install(
        DIRECTORY src/
        DESTINATION include/aubio
        FILES_MATCHING
        PATTERN "*.h"
        PATTERN "*_priv.h" EXCLUDE
        PATTERN "config.h" EXCLUDE
    )
endif()

if(BUILD_TOOLS)
    install(
        TARGETS ${EXAMPLE_EXECS}
        RUNTIME DESTINATION tools/aubio
        CONFIGURATIONS Release
    )
endif()

@bagong
Copy link
Contributor Author

bagong commented Jun 23, 2017

Is there anything I can do to get this committed? ;) I guess we can also close the two other PRs, this one was originally based on?

@sebastianneubauer
Copy link

sebastianneubauer commented Jun 25, 2017

When I try to build this PR, I get an error:

FATALunknown tool PKG-CONFIG -- unable to acquire.
CMake Warning (dev) at scripts/cmake/vcpkg_find_acquire_program.cmake:161 (file):
  Unexpected argument: SHA512=
Call Stack (most recent call first):
  ports/aubio/portfile.cmake:22 (vcpkg_find_acquire_program)
  scripts/ports.cmake:73 (include)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Error at scripts/cmake/vcpkg_find_acquire_program.cmake:161 (file):
  file DOWNLOAD error: cannot create directory '' - Specify file by full path
  name and verify that you have directory creation and file write privileges.
Call Stack (most recent call first):
  ports/aubio/portfile.cmake:22 (vcpkg_find_acquire_program)
  scripts/ports.cmake:73 (include)

Is that expected? What am I doing wrong, am I missing something?

@bagong
Copy link
Contributor Author

bagong commented Jun 25, 2017

Two things:

One may also ask whether the pkg-config and waf PRs have a value independent of being an dependency for the old aubio build-script, but I am fine with closing them as not required any more...

@ras0219-msft
Copy link
Contributor

Thanks for the improvements!

However, the tools failed to build for me with "missing config.h" errors. I went ahead and merged the core library build, and I'd be happy to accept a follow-up PR that enables the tools.

Thanks again for the back and forth here :)

@bagong
Copy link
Contributor Author

bagong commented Jun 28, 2017

Thanks for providing that alternative version. I remember now that I had commented out the config.h includes. They are not needed as the necessary definitions are provided in the cmake build script. We'd either have to provide/generate a config.h or ifdef the include. I think the later would be nicer, I'll try to discuss that upstream... Thanks for now!

@ras0219-msft
Copy link
Contributor

If a blank file will work, it should be pretty trivial to just do

file(WRITE "${CURRENT_BINARY_DIR}/config.h" "")
include_directories(${CURRENT_BINARY_DIR})

@bagong
Copy link
Contributor Author

bagong commented Jun 29, 2017

Ah, if you're fine with a trick like that I'll push it tomorrow ;) (busy right now)

@bagong bagong deleted the aubio branch June 29, 2017 10:58
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.

4 participants