Kitt-AI / snowboy

Future versions with model training module will be maintained through a forked version here: https://github.com/seasalt-ai/snowboy
Other
3.08k stars 997 forks source link

Provide C++11 ABI style binary of libsnowboy-detect.a #99

Open kskalski opened 7 years ago

kskalski commented 7 years ago

When I compile with GCC 5 I get speech.cc:(.text+0x4ec): undefined reference to snowboy::SnowboyDetect::SnowboyDetect(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)' speech.cc:(.text+0x500): undefined reference tosnowboy::SnowboyDetect::SetSensitivity(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)'

I've seen a recent addition of -D_GLIBCXX_USE_CXX11_ABI=0 into C++ examples, which I think is related to the same problem. However in my case I link to some other libraries, which are only available in CXX11 ABI on my system, so it seems I have to use it for all of my project and in this case it causes failure shown above.

chenguoguo commented 7 years ago

I'll leave this up for a while. I'm a little bit reluctant to move everything to gcc 5.1 or above.

I haven't tried compiling with another library compiled with GLIBCXX_USE_CXX11_ABI set to 1. What happens if you set -D_GLIBCXX_USE_CXX11_ABI=0 when you compile your binary?

kskalski commented 7 years ago

My exact problem is that libprotobuf 3.0.0 on raspbian is compiled with gcc 5 and contains some symbols, which are incompatible when using old ABI. For reference, there is a page detailing compatibility and how to check if given lib is likely to have problems: https://gcc.gnu.org/wiki/Cxx11AbiCompatibility

My fallback is to just recompile it from sources for my project and don't use the OS lib.

As for compiling the lib with GLIBCXX_USE_CXX11_ABI=1 (or using gcc-5) I encourage you to try - it's possible that it will just work if your code doesn't reference C++ objects from other dependencies. Then you can provide CXX11 ABI version of the lib, which will be useful in the future in any case.

The exact error I get when compiling my code with GLIBCXX_USE_CXX11_ABI=0 is: speech.o: In function google::protobuf::internal::GetEmptyStringAlreadyInited()': speech.cc:(.text._ZN6google8protobuf8internal27GetEmptyStringAlreadyInitedEv[_ZN6google8protobuf8internal27GetEmptyStringAlreadyInitedEv]+0x3c): undefined reference togoogle::protobuf::internal::emptystring' because I'm referencing lib using new ABI. I guess it's not possible to build binary which is referencing C++ objects from two different libraries using different ABI versions - thus I get error from either one of them.

chenguoguo commented 7 years ago

GLIBCXX_USE_CXX11_ABI=1 is a way to go, yes. But I'd like to take it slowly, after full checks. Don't want to break people's existing setups.

kskalski commented 7 years ago

To be clear - I think in foreseeable future it would be best to have two versions of the library (yeah, I guess it should apply also to other architectures, so basically doubling the number of builds): compiled with GLIBCXX_USE_CXX11_ABI=0 and with GLIBCXX_USE_CXX11_ABI=1 I imagine basically new files like lib/rpi/snowboy-detect-cxx11.a would be added

The only way to avoid it is to not expose any C++ objects in public interface, which is already violated, so not the best option.

An important question at this point is whether snowboy's code is easy to compile as CXX11 ABI - I don't know what dependencies you use, but if there is some C++ lib, you will likely need to link against appropriate version of it too.

BillWSY commented 7 years ago

Our solution was to build a wrapper class that only expose plain C types in the interface. The first line of the .cc file is #define _GLIBCXX_USE_CXX11_ABI 0, so it can call Snowboy correctly. The rest of the system use char* to pass the parameters.

You don't need GLIBCXX_USE_CXX11_ABI=0 in the global compile options.

kskalski commented 7 years ago

Ah, that's smart, so the binary can be linked as long calls to snowboy's class are made within block having _GLIBCXX_USE_CXX11_ABI 0 defined. I will try that out, thanks!

BillWSY commented 7 years ago

In fact, the <string> library would use the old STL namespace when _GLIBCXX_USE_CXX11_ABI is defined as 0. So make sure it is defined to 0 before including <string>, directly and indirectly.

kskalski commented 7 years ago

It worked. In fact I guess if the library provided const char* variant for snowboy::SnowboyDetect constructor and for SetSensitivity method it would all work fine without a wrapper or a different build of lib.​

fcolecumberri commented 7 years ago

Hi there, I'm having almost the same problem, I'm making an application that uses c++11 libraries and I'm trying to incorporate this project, but the strings are incompatible, and in my case I would need to remake a lot just to make a way around, if you could just compile a libsnowboy-detect++11.a it would not break others implementation and it wold help a lot.

kskalski commented 7 years ago

You can include following in your project:

snowboy_wrapper.h

#pragma once

#include <cstdint>

class Snowboy {
 public:
  struct Model {
    const char* filename;
    float sensitivity;
  };

  Snowboy(const char* resource_name, Model model, float audio_gain);

  int SampleRate() const;
  int NumChannels() const;
  int BitsPerSample() const;

  int RunDetection(const int16_t* data, int num_samples);

 private:
  void* detector_;
};

snowboy_wrapper.cc

#undef _GLIBCXX_USE_CXX11_ABI
#define _GLIBCXX_USE_CXX11_ABI 0
#include "snowboy_wrapper.h"

#include "snowboy/include/snowboy-detect.h"

static snowboy::SnowboyDetect* c(void* p) { return reinterpret_cast<snowboy::SnowboyDetect*>(p); }
static const snowboy::SnowboyDetect* c(const void* p) {
  return reinterpret_cast<const snowboy::SnowboyDetect*>(p);
}

Snowboy::Snowboy(const char* resource_name, Model model, float audio_gain)
    : detector_(new snowboy::SnowboyDetect(resource_name, model.filename)) {
  // Initializes Snowboy detector.
  // If you have multiple hotword models (e.g., 2), you should set
  // <model_filename> and <sensitivity_str> as follows:
  //   model_filename = "resources/snowboy.umdl,resources/alexa.pmdl";
  //   sensitivity_str = "0.4,0.4";
  c(detector_)->SetSensitivity(std::to_string(model.sensitivity));
  c(detector_)->SetAudioGain(audio_gain);
  c(detector_)->ApplyFrontend(true);
}

int Snowboy::SampleRate() const { return c(detector_)->SampleRate(); }

int Snowboy::NumChannels() const { return c(detector_)->NumChannels(); }

int Snowboy::BitsPerSample() const { return c(detector_)->BitsPerSample(); }

int Snowboy::RunDetection(const int16_t* data, int num_samples) {
  return c(detector_)->RunDetection(data, num_samples);
}

It should work when you use Snowboy object for using the library.

ghost commented 6 years ago

@BillWSY Hey Bill. Do you know what a savior you are for people like me that read your little comment a few months later? You have no idea what 2 minutes of your time means for people like me that come along months later. You're the man Bill. The man. Wish I could buy you a beer.

chenguoguo commented 6 years ago

Perhaps, you could submit a PR for that Bill? @BillWSY

BillWSY commented 6 years ago

I think @kskalski already provided a well-rounded wrapper. I have something that is very similar, but it will be a duplicate.

chenguoguo commented 6 years ago

@kskalski would you like to make your wrapper part of the project?

kskalski commented 6 years ago

As I noted in https://github.com/Kitt-AI/snowboy/issues/99#issuecomment-271569868 I think it would be better if the primary snowboy class just added another constructor and appropriate API with const char* parameters, which would remove the necessity of using a wrapper.

I'm not sure if this would require choosing different names for methods with const char* parameter, the overloads might get hard or impossible to use in this case.

In any case, feel free to include the code in the project, here is a commit with some made-up names / layout https://github.com/Kitt-AI/snowboy/commit/651114d5e910e7715bf93d79f96ef3b0dec0a3b7

NicoHood commented 6 years ago

Can anyone tell me where the source of libsnowboy-detect.a can be found?

chenguoguo commented 6 years ago

The source code was not released, only libraries were available to the public.

tatasadi commented 6 years ago

Could someone tell me where should I put this wrapper? How should I use it?

tatasadi commented 6 years ago

@kskalski Could you please read my last comment and answer it?

kskalski commented 6 years ago

Not sure if I understand the question - you put it in our client code / compile together with classes using snowboy in your app

tatasadi commented 6 years ago

@kskalski thank you for your answer. I mean where in client code? (avs-device-sdk/third-party/snowboy/include/ ??) which file should I change to use this wrapper in make? my make command is:

cd /home/ubuntu/Projekte/avs-device-sdk/sdk-build && cmake /home/ubuntu/Projekte/avs-device-sdk/sdk-source/avs-device-sdk -DKITTAI_KEY_WORD_DETECTOR=ON -DKITTAI_KEY_WORD_DETECTOR_LIB_PATH=/home/ubuntu/Projekte/avs-device-sdk/third-party/snowboy/lib/ubuntu64/libsnowboy-detect.a -DKITTAI_KEY_WORD_DETECTOR_INCLUDE_DIR=/home/ubuntu/Projekte/avs-device-sdk/third-party/snowboy/include -DGSTREAMER_MEDIA_PLAYER=ON -DPORTAUDIO=ON -DPORTAUDIO_LIB_PATH=/home/ubuntu/Projekte/avs-device-sdk/third-party/portaudio/lib/.libs/libportaudio.a -DPORTAUDIO_INCLUDE_DIR=/home/ubuntu/Projekte/avs-device-sdk/third-party/portaudio/include && make

Your help would be much appreciated.

kskalski commented 6 years ago

I don't know your build system, but the wrapper should be part of your app. Do you have any your own source file in your app? Find it in the build system (CMakeLists.txt I guess) and add the wrapper files there too.

johnnyhuziqin commented 6 years ago

I am using ubuntu 16.04, with gcc 5.4 installed, i have the same compile error, here is how I fix: add a line to the CmakeList.txt in the skd-source folder

cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
add_definitions(-D_GLIBCXX_USE_CXX11_ABI=0)
# Set project information
project(AlexaClientSDK VERSION 1.7.0 LANGUAGES CXX)

run cmake (with all needed args) run make clean run make by this, compile ok. hope this help you.

avp24 commented 6 years ago

@johnnyhuziqin thank you for your help. With your steps I could able to complete by Build process. @kskalski @BillWSY @chenguoguo Thanks for your explanations. Thanks again!

BlueBlastoise2 commented 5 years ago

Hi, I am currently making modification on Amazon's AVS sample app and make it to work with Kitt.AI. I have put the wrapper in the folder. I am still getting the error. I am not quite sure what I did wrong. My project is here https://github.com/j42lin/avs-device-sdk/tree/master/SampleApp/src

I put it under avs-device-sdk, and this is the error I got

[100%] Building CXX object SampleApp/src/CMakeFiles/SampleApp.dir/PortAudioMicrophoneWrapper.cpp.o
[100%] Linking CXX executable SampleApp
../../KWD/KittAi/src/libKITTAI.so: undefined reference to `snowboy::SnowboyDetect::SetSensitivity(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'
../../KWD/KittAi/src/libKITTAI.so: undefined reference to `snowboy::SnowboyDetect::SnowboyDetect(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)'
collect2: error: ld returned 1 exit status
SampleApp/src/CMakeFiles/SampleApp.dir/build.make:388: recipe for target 'SampleApp/src/SampleApp' failed
make[3]: *** [SampleApp/src/SampleApp] Error 1
CMakeFiles/Makefile2:8752: recipe for target 'SampleApp/src/CMakeFiles/SampleApp.dir/all' failed
make[2]: *** [SampleApp/src/CMakeFiles/SampleApp.dir/all] Error 2
CMakeFiles/Makefile2:8764: recipe for target 'SampleApp/src/CMakeFiles/SampleApp.dir/rule' failed
make[1]: *** [SampleApp/src/CMakeFiles/SampleApp.dir/rule] Error 2
Makefile:2227: recipe for target 'SampleApp' failed
make: *** [SampleApp] Error 2
checkomkar commented 5 years ago

I am using ubuntu 16.04, with gcc 5.4 installed, i have the same compile error, here is how I fix: add a line to the CmakeList.txt in the skd-source folder

cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
add_definitions(-D_GLIBCXX_USE_CXX11_ABI=0)
# Set project information
project(AlexaClientSDK VERSION 1.7.0 LANGUAGES CXX)

run cmake (with all needed args) run make clean run make by this, compile ok. hope this help you.

Thank you. This helped a lot. Cheers!

EpicIntegratedSystems commented 5 years ago

I am using ubuntu 16.04, with gcc 5.4 installed, i have the same compile error, here is how I fix: add a line to the CmakeList.txt in the skd-source folder

cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
add_definitions(-D_GLIBCXX_USE_CXX11_ABI=0)
# Set project information
project(AlexaClientSDK VERSION 1.7.0 LANGUAGES CXX)

run cmake (with all needed args) run make clean run make by this, compile ok. hope this help you.

Thank you. This helped a lot. Cheers!

This also worked for me. Thanks a ton!

Ubuntu 18.04 gcc 8.2.0