Skip to content

Commit

Permalink
Enable FLAC support on Windows.
Browse files Browse the repository at this point in the history
  - Informational source:
    - See `copts`, `defines`, and `local_defines` at https://bazel.build/reference/be/c-cpp#cc_library for informational purposes.
  - Primary change:
    - Move `FLAC__NO_DLL` from `copts` to `defines` which are propagated to all users of the library.
    - This propagates to `iamf_tools/iamf/cli:flac_encoder` which allows it to link correctly.
  - Drive-by changes:
    - Move other FLAC defines from `copts` to `local_defines` which is preferred.
    - Enable `flac_encoder` tests on Windows GitHub CI now that this component works correctly.
    - Adjust some `linkopts` and `defines` which are not needed on all platforms.
    - Remove obsolete `flac_encoder_windows.cc` which was only useful to provide stubs.

PiperOrigin-RevId: 671855541
  • Loading branch information
jwcullen committed Sep 6, 2024
1 parent 55f8ca9 commit f967676
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 58 deletions.
2 changes: 1 addition & 1 deletion .github/actions/iamf-tools-builder/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ runs:
# Windows works best with a very short path as per https://bazel.build/configure/windows#long-path-issues.
echo "startup --output_user_root=C:/" >> .bazelrc
# Disable some windows tests until some platform-specific bugs are fixed.
TEST_FILTER=*:-FlacEncoder*:*TestVector.ValidateTestSuite*:EncoderMainLibTest.*
TEST_FILTER=*:-*TestVector.ValidateTestSuite*:EncoderMainLibTest.*
else
TEST_FILTER=*
fi
Expand Down
31 changes: 22 additions & 9 deletions external/flac.BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,39 @@ flac_hdrs = glob([

flac_copts = [
"-w",
"-DFLAC__USE_VISIBILITY_ATTR",
"-DPACKAGE_VERSION=\\\"1.4.3\\\"",
"-DHAVE_STDINT_H",
"-DHAVE_LROUND",
"-DNDEBUG",
"-D_FORTIFY_SOURCE=2",
"-DFLAC__HAS_OGG=0",
"-DFLAC__NO_DLL", # Static library for Windows
"-Iexternal/flac/src/libFLAC/include",
"-Iexternal/flac/include",
]

flac_linkopts = ["-lm"]
flac_local_defines = [
"FLAC__HAS_OGG=0",
"FLAC__USE_VISIBILITY_ATTR",
"PACKAGE_VERSION=\\\"1.4.3\\\"",
"HAVE_STDINT_H",
"HAVE_LROUND",
"NDEBUG",
"_FORTIFY_SOURCE=2",
]

# Defines which need to propagate to all downstream users.
flac_defines = [
"@platforms//os:windows": ["FLAC__NO_DLL"],
"//conditions:default": [],
]

flac_linkopts = select({
"@platforms//os:windows": [],
"//conditions:default": ["-lm"],
})

cc_library(
name = "flac",
srcs = flac_srcs,
hdrs = flac_hdrs,
copts = flac_copts,
defines = flac_defines,
linkopts = flac_linkopts,
local_defines = flac_local_defines,
textual_hdrs = flac_textual_includes,
)

Expand Down
8 changes: 1 addition & 7 deletions iamf/cli/codec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,9 @@ cc_library(
],
)

# TODO(b/362229054): Enable FLAC support on Windows and unify `flac_encoder.cc` and
# `flac_encoder_windows.cc`.

cc_library(
name = "flac_encoder",
srcs = select({
"@platforms//os:windows": ["flac_encoder_windows.cc"],
"//conditions:default": ["flac_encoder.cc"],
}),
srcs = ["flac_encoder.cc"],
hdrs = ["flac_encoder.h"],
deps = [
":encoder_base",
Expand Down
41 changes: 0 additions & 41 deletions iamf/cli/codec/flac_encoder_windows.cc

This file was deleted.

0 comments on commit f967676

Please sign in to comment.