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

iOS support #3150

Merged
merged 16 commits into from
Jul 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions native_client/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ tf_cc_shared_object(
}) + tflite_copts(),
linkopts = select({
"//tensorflow:macos": [],
"//tensorflow:ios": ["-fembed-bitcode"],
"//tensorflow:linux_x86_64": LINUX_LINKOPTS,
"//tensorflow:rpi3": LINUX_LINKOPTS,
"//tensorflow:rpi3-armv8": LINUX_LINKOPTS,
Expand Down
10 changes: 7 additions & 3 deletions native_client/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@
#include <sstream>
#include <string>

#if defined(__ANDROID__) || defined(_MSC_VER)
#ifdef __APPLE__
#include <TargetConditionals.h>
#endif

#if defined(__ANDROID__) || defined(_MSC_VER) || TARGET_OS_IPHONE
reuben marked this conversation as resolved.
Show resolved Hide resolved
#define NO_SOX
#endif

Expand Down Expand Up @@ -244,7 +248,7 @@ GetAudioBuffer(const char* path, int desired_sample_rate)
sox_false // Reverse endianness
};

#ifdef __APPLE__
#if TARGET_OS_OSX
reuben marked this conversation as resolved.
Show resolved Hide resolved
// It would be preferable to use sox_open_memstream_write here, but OS-X
// doesn't support POSIX 2008, which it requires. See Issue #461.
// Instead, we write to a temporary file.
Expand Down Expand Up @@ -348,7 +352,7 @@ GetAudioBuffer(const char* path, int desired_sample_rate)
fclose(wave);
#endif // NO_SOX

#ifdef __APPLE__
#if TARGET_OS_OSX
res.buffer_size = (size_t)(output->olength * 2);
res.buffer = (char*)malloc(sizeof(char) * res.buffer_size);
FILE* output_file = fopen(output_name, "rb");
Expand Down
22 changes: 20 additions & 2 deletions native_client/definitions.mk
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,34 @@ NODE_PLATFORM_TARGET := --target_arch=arm64 --target_platform=linux
TOOLCHAIN_LDD_OPTS := --root $(RASPBIAN)/
endif # ($(TARGET),rpi3-armv8)

ifeq ($(TARGET),ios-simulator)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does that means we have different binaries built for the simulator vs for real hardware ?
Can we run tests on those real hardware binary from the simulator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hardware is ARM64, simulator is x86_64, so no, can't run tests on real hardware binary.

Copy link
Collaborator

Choose a reason for hiding this comment

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

They have no way to run ARM64 simulator? Bad, it means we'll require more dedicated hardware and baby-sitting setup :/

Copy link
Contributor Author

@reuben reuben Jul 20, 2020

Choose a reason for hiding this comment

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

I don't think we have the capacity to handle real hardware tests for iOS.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mean the capacity at iOS simulator level, or our capacity in term of workload? I'm concerned that we have nothing exercizing at all, even basically, the library on ARM64 context, because it's very likely that we will break things somehow :/

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 mean in terms of the workload. New iPhones + new iOS released every year, Apple is famous for breaking compatibility and forcing developers to change their workflows frequently, plus none of us are iOS developers so everything takes twice as long to setup. I agree it's unfortunate to not have ARM64 test coverage.

CFLAGS := -isysroot $(shell xcrun -sdk iphonesimulator13.5 -show-sdk-path)
SOX_CFLAGS :=
SOX_LDFLAGS :=
LDFLAGS :=
endif

ifeq ($(TARGET),ios-arm64)
CFLAGS := -target arm64-apple-ios -isysroot $(shell xcrun -sdk iphoneos13.5 -show-sdk-path)
SOX_CFLAGS :=
SOX_LDFLAGS :=
LDFLAGS :=
endif

# -Wl,--no-as-needed is required to force linker not to evict libs it thinks we
# dont need ; will fail the build on OSX because that option does not exists
ifeq ($(OS),Linux)
LDFLAGS_NEEDED := -Wl,--no-as-needed
LDFLAGS_RPATH := -Wl,-rpath,\$$ORIGIN
endif
ifeq ($(OS),Darwin)
CXXFLAGS += -stdlib=libc++ -mmacosx-version-min=10.10
LDFLAGS_NEEDED := -stdlib=libc++ -mmacosx-version-min=10.10
CXXFLAGS += -stdlib=libc++
LDFLAGS_NEEDED := -stdlib=libc++
LDFLAGS_RPATH := -Wl,-rpath,@executable_path
ifeq ($(TARGET),host)
CXXFLAGS += -mmacosx-version-min=10.10
LDFLAGS_NEEDED += -mmacosx-version-min=10.10
endif
endif

CFLAGS += $(EXTRA_CFLAGS)
Expand Down
4 changes: 4 additions & 0 deletions native_client/swift/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.DS_Store
build/
xcuserdata/
/deepspeech_ios/libdeepspeech.dylib
Loading