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 in output samplerate #27

Merged
merged 3 commits into from
Nov 30, 2023
Merged

Conversation

armandobelardo
Copy link
Contributor

Addresses #26

@armandobelardo
Copy link
Contributor Author

armandobelardo commented Nov 29, 2023

@chrisstaite I was wondering if you've seen this issue before, I was trying to build locally to test out the change but I'm running into some cpp build issues

libtool: link: sed 's|^|_|' < ../include/libmp3lame.sym > .libs/libmp3lame-symbols.expsym
libtool: link: gcc -dynamiclib  -o .libs/libmp3lame.0.dylib  .libs/VbrTag.o .libs/bitstream.o .libs/encoder.o .libs/fft.o .libs/gain_analysis.o .libs/id3tag.o .libs/lame.o .libs/newmdct.o .libs/presets.o .libs/psymodel.o .libs/quantize.o .libs/quantize_pvt.o .libs/reservoir.o .libs/set_get.o .libs/tables.o .libs/takehiro.o .libs/util.o .libs/vbrquantize.o .libs/version.o .libs/mpglib_interface.o   -Wl,-force_load,../mpglib/.libs/libmpgdecoder.a  -lm  -O3 -arch x86_64 -mmacosx-version-min=10.6 -arch x86_64 -mmacosx-version-min=10.6   -install_name  /Users/armandobelardo/Git/oss/forks/lameenc/build/lame/src/lame/lib/libmp3lame.0.dylib -compatibility_version 1 -current_version 1.0 -Wl,-exported_symbols_list,.libs/libmp3lame-symbols.expsym
ld: Undefined symbols:
  _init_xrpow_core_sse, referenced from:
      _init_xrpow_core_init in quantize.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[6]: *** [libmp3lame.la] Error 1
make[5]: *** [all-recursive] Error 1
make[4]: *** [all-recursive] Error 1
make[3]: *** [all] Error 2
make[2]: *** [lame/src/lame-stamp/lame-build] Error 2
make[1]: *** [CMakeFiles/lame.dir/all] Error 2
make: *** [all] Error 2

UPDATE: I brute forced this a bit and forced the CMake file to build for my arch and it then built in full
I was able to test this local version in my application and confirmed it does indeed address #26

if (APPLE)
    set(BUILT_FILE "lameenc-1.0.0-cp34-cp34m-macosx_10_6_intel.whl")
    set(HOST "")
    # if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "x86_64" OR "${CMAKE_OSX_ARCHITECTURES}" STREQUAL "")
    #   set(ARCH_FLAGS "-arch x86_64")
    # endif ()
    # if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
    set(ARCH_FLAGS "${ARCH_FLAGS} -arch arm64")
    set(HOST_PLATFORM "_PYTHON_HOST_PLATFORM=macosx-11.0-arm64")
    set(HOST "--host=aarch64")
    # endif ()
    set(LAME_FLAGS "${ARCH_FLAGS} -mmacosx-version-min=10.6")
    set(LAME_CONFIGURE "<SOURCE_DIR>/configure" "CFLAGS=${LAME_FLAGS}" "LDFLAGS=${LAME_FLAGS}"
        "--prefix=<BINARY_DIR>" "${HOST}"
        "--enable-expopt" "--enable-nasm" "--disable-frontend" "--disable-analyzer-hooks"
        "--disable-debug" "--disable-dependency-tracking")
    set(LAME_INSTALL $(MAKE) install)
elseif (WIN32)

Copy link
Owner

@chrisstaite chrisstaite left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple of touch ups please.

setup.py Outdated Show resolved Hide resolved
lameenc.c Outdated Show resolved Hide resolved
@chrisstaite chrisstaite merged commit 65e41fc into chrisstaite:main Nov 30, 2023
28 checks passed
@armandobelardo armandobelardo deleted the ab/resample branch November 30, 2023 14: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.

2 participants