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

[Bug] CMake build adds sse flags for all source files, not only for SSE-optimized ones #154

Closed
WPMGPRoSToTeMa opened this issue Nov 6, 2019 · 6 comments

Comments

@WPMGPRoSToTeMa
Copy link

WPMGPRoSToTeMa commented Nov 6, 2019

For example:

opus/opus_functions.cmake

Lines 142 to 155 in f9d3d43

check_include_file(smmintrin.h HAVE_SMMINTRIN_H) # SSE4.1
if(HAVE_SMMINTRIN_H)
if(MSVC)
if(CMAKE_SIZEOF_VOID_P EQUAL 4)
check_flag(SSE4_1 /arch:SSE2) # SSE2 and above
else()
set(SSE4_1_SUPPORTED 1 PARENT_SCOPE)
endif()
else()
check_and_set_flag(SSE4_1 -msse4.1)
endif()
else()
set(SSE4_1_SUPPORTED 0 PARENT_SCOPE)
endif()

check_and_set_flags adds the specified flag to compile options if supported:

function(check_and_set_flag NAME FLAG)
include(CheckCCompilerFlag)
check_c_compiler_flag(${FLAG} ${NAME}_SUPPORTED)
if(${NAME}_SUPPORTED)
add_definitions(${FLAG})
endif()
endfunction()

On the other hand, Makefile.am adds SSE options only for SSE-optimized source files:

opus/Makefile.am

Lines 329 to 344 in ad8fe90

if HAVE_SSE
SSE_OBJ = $(CELT_SOURCES_SSE:.c=.lo)
$(SSE_OBJ): CFLAGS += $(OPUS_X86_SSE_CFLAGS)
endif
if HAVE_SSE2
SSE2_OBJ = $(CELT_SOURCES_SSE2:.c=.lo)
$(SSE2_OBJ): CFLAGS += $(OPUS_X86_SSE2_CFLAGS)
endif
if HAVE_SSE4_1
SSE4_1_OBJ = $(CELT_SOURCES_SSE4_1:.c=.lo) \
$(SILK_SOURCES_SSE4_1:.c=.lo) \
$(SILK_SOURCES_FIXED_SSE4_1:.c=.lo)
$(SSE4_1_OBJ): CFLAGS += $(OPUS_X86_SSE4_1_CFLAGS)
endif

Can we fix that for CMake builds? This may lead to SSE 4.1 compiler optimizations in non-SSE-optimized files and therefore to invalid instruction errors on machines without SSE 4.1 support.

@xnorpx
Copy link
Contributor

xnorpx commented Dec 17, 2019

let me try to fix that

sizeak pushed a commit to glean-notes/opus that referenced this issue Jan 9, 2020
…s when target use runtime check xiph#154. For windows we only use /arch flag when target is presumed to support SSE xiph#132. Added support for enable float approx option"
xnorpx added a commit to xnorpx/opus that referenced this issue Mar 13, 2020
 on non-windows when target use runtime check (GH xiph#154). For windows we only use
 /arch flag when target is presumed to support SSE (GH xiph#132).
xnorpx added a commit to xnorpx/opus that referenced this issue Apr 1, 2020
 on non-windows when target use runtime check (GH xiph#154). For windows we only use
 /arch flag when target is presumed to support SSE (GH xiph#132).
xnorpx added a commit to xnorpx/opus that referenced this issue Apr 8, 2020
 on non-windows when target use runtime check (GH xiph#154). For windows we only use
 /arch flag when target is presumed to support SSE (GH xiph#132).
xnorpx added a commit to xnorpx/opus that referenced this issue Apr 8, 2020
 on non-windows when target use runtime check (GH xiph#154). For windows we only use
 /arch flag when target is presumed to support SSE (GH xiph#132).
xnorpx added a commit to xnorpx/opus that referenced this issue Apr 8, 2020
…n-windows when target use runtime check (GH xiph#154). For windows we only use /arch flag when target is presumed to support SSE to avoid AVX function pollution (GH xiph#132).
xnorpx added a commit to xnorpx/opus that referenced this issue Apr 14, 2020
…n-windows when target use runtime check (GH xiph#154). For windows we only use /arch flag when target is presumed to support SSE to avoid AVX function pollution (GH xiph#132).
xnorpx added a commit to xnorpx/opus that referenced this issue Apr 14, 2020
…n-windows when target use runtime check (GH xiph#154). For windows we only use /arch flag when target is presumed to support SSE to avoid AVX function pollution (GH xiph#132).
xnorpx added a commit to xnorpx/opus that referenced this issue Apr 16, 2020
…n-windows when target use runtime check (GH xiph#154). For windows we only use /arch flag when target is presumed to support SSE to avoid AVX function pollution (GH xiph#132).
XiphGitlabBot pushed a commit that referenced this issue Apr 21, 2020
…n-windows when target use runtime check (GH #154). For windows we only use /arch flag when target is presumed to support SSE to avoid AVX function pollution (GH #132).

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>
@xnorpx
Copy link
Contributor

xnorpx commented Apr 21, 2020

this has been merged to master

@WPMGPRoSToTeMa
Copy link
Author

@xnorpx thank you so much! Do you know when a new version with this fix will be released?

@xnorpx
Copy link
Contributor

xnorpx commented Apr 21, 2020

That I don't know sorry would have to consume master for now.

@rillian
Copy link
Contributor

rillian commented Apr 21, 2020

Closing as fixed. Thanks for pushing things upstream!

@rillian rillian closed this as completed Apr 21, 2020
@rillian
Copy link
Contributor

rillian commented Apr 21, 2020

Thanks for fixing!

davidebeatrici pushed a commit to mumble-voip/opus that referenced this issue May 28, 2020
…n-windows when target use runtime check (GH xiph#154). For windows we only use /arch flag when target is presumed to support SSE to avoid AVX function pollution (GH xiph#132).

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>
daleglass added a commit to daleglass/vircadia that referenced this issue Jun 7, 2020
This will build the very latest Opus available (May 30 2020). The absolute
minimum version of Opus is commit 927de8453c502586c03e25c169ec08f2a93ebc02,
released on April 21, 2020.

This incorporates a fix for xiph/opus#154
sumitkandpal added a commit to airtimemedia/opus that referenced this issue Jun 26, 2023
* Avoid processing LPC coeffs beyond the given order in NEON optimizations

* Fix NEON optimizations buffer read overrun

Thanks to Ray Essick

* Don't update null data pointer after each multistream decoder

The data pointer could be null in the case of DTX or packet loss.

* OPUS_GET_IN_DTX handles Silk middle channel only

Signed-off-by: Felicia Lim <flim@google.com>

* CMake: add option to set BUILD_SHARED_LIBS variable

From https://cmake.org/cmake/help/latest/variable/BUILD_SHARED_LIBS.html:

"Global flag to cause add_library() to create shared libraries if on.

If present and true, this will cause all libraries to be built shared unless the library was explicitly added as a static library. This variable is often added to projects as an option() so that each user of a project can decide if they want to build the project using shared or static libraries."

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* CMake: Add shared library to features

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* CMake: Add alias Opus::opus for opus library. Useful for CMake superbuild pattern.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* CMake: use PACKAGE_VERSION for the pkg-config file

The old variable `OPUS_LIBRARY_VERSION`, does not line up with what's used in autoconf, which uses the PACKAGE_VERSION instead, which in turn lines up with what version the projects `opusfile` and `libsndfile` check for, for example.
While at it, I also cleaned up the accidental double up in code.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Revert "Attenuate SILK PLC gain only for unvoiced speech"

This reverts commit 4f5557c.

Signed-off-by: Felicia Lim <flim@google.com>

* Fixes to the the activity flag that is passed to Silk so it represents the final activity flag used in the DTX decision

This flag was modified after calling the Silk encoder function. This commit corrects that behavior by introducing those modifications before calling the Silk encoder.

Slightly modified comments by Felicia Lim

Signed-off-by: Felicia Lim <flim@google.com>

* Revert "Fixes to the the activity flag that is passed to Silk so it represents the final activity flag used in the DTX decision"

This reverts commit ea3b30f.

* Reland "Fixes to the the activity flag that is passed to Silk so it represents the final activity flag used in the DTX decision"

This flag was modified after calling the Silk encoder function. This commit corrects that behavior by introducing those modifications before calling the Silk encoder.

Slightly modified comments by Felicia Lim

Signed-off-by: Felicia Lim <flim@google.com>

* Fix for an assertion when running the fixed point tests

Signed-off-by: Felicia Lim <flim@google.com>

* Fix signed integer overflows in silk_noise_shape_quantizer_del_dec

* Fix another signed integer overflow in silk_noise_shape_quantizer_del_dec

* Fix celt decoder assertion when using OPUS_CUSTOM

When using OPUS_CUSTOM, `CELTDecoder->end` can be larger than 21.
Assert against 25 instead in OPUS_CUSTOM builds.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - make package version parsing more robust

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - add CPack and default to TGZ package

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - Add support for detecting the presence of lrint and lrintf.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - Fix CMake install include directory

Install include directory must be `include/opus`, not `include`.

Old path is still here for compatibility.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - add math library for test programs demo and compare when building dynamic library

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - intinsics fixes, only compile SSE source with SSE flags on non-windows when target use runtime check (GH xiph#154). For windows we only use /arch flag when target is presumed to support SSE to avoid AVX function pollution (GH xiph#132).

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - Add option for testing to improve cli

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - Add OPUS prefix to feature info to show correct commandline option

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - Fix OPUS_STACK_PROTECTOR option

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* CMake Changes

    - Fix typo in OPUS_USE_NEON description.

    - Set OPUS_PRESUME_NEON for iOS platforms as all armv7 and higher
      iOS devices support NEON.

    - Fix detection of aarch64 for OPUS_CPU_ARM and adding sources from
      celt_sources_arm. (previously would miss armcpu.c and arm_celt_map.c)

    - Change "armv7-a" to "arm" in MATCHES checks against
      CMAKE_SYSTEM_PROCESSOR as systems like the RPi3 report as
      "armv7l".

    - Rename OPUS_MAY_SUPPORT_NEON to OPUS_MAY_HAVE_NEON as this name is
      used everywhere else in the CMake build system. Without this,
      runtime capability detection is broken on aarch64.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - Add variable length detection and alloca detection

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - add option for float_approx for IEEE 754 compatible targets

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* cmake - add option for fast math

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Fix a typo in in opus_custom.h.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* gitlab-ci: Add a build description.

Describe builds for the gitlab continuous integration
service runners. This does a trial build under both
autotools and cmake, so we get some coverage for changes
on that hosting platform.

After the same script in the vorbis and ogg projects.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Repository moved to gitlab.xiph.org

* fix equivalent bitrate calculation for <20ms frame sizes

Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* trivial_example: open raw pcm files in binary mode.

The simple codec round-trip example file in the doc directory
opens an input and output pcm file. It was working fine on
POSIX systems, but not on Windows, which treats text files
differently.

This is confusing in a example, so it's better to add an
explicit binary flag to the fopen() calls. This does nothing
on unix-like systems, but should make the example work for
developers on Windows.

Thanks to Wavesonics who reported this on irc.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* trivial_example: Check the return value of fread().

Silence a gcc warning by checking the return value of the fread()
call instead of the feof() guard. This prevents an infinite loop
in the case of a read error. Otherwise, when end-of-file is reached
fread() will certainly return a smaller number of elements read
than requested, so both cases are handled now.

Add a comment to clarify that we're dropping a partial frame on
purpose to keep the code simple.

Also add more braces around conditional bodies for less error-prone
style.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Build trivial_example by default.

Add doc/trivial_example.c to the autotools build so we get
some minimal verification that the code compiles.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Update Doxygen config file, header and footer to Doxygen 1.8.18

Signed-off-by: Ralph Giles <giles@thaumas.net>

* Build and run test for cmake build in gitlab-ci

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Disable message box when calling abort(). The message box is causing hangs in tests.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Build time improvement, for MSVC use intrin0.h instead of intrin.h and remove usage of stdio.h in production code

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Fix intrin0.h include guard.

This lighter-weight intrinsics header is available starting
with Microsoft Visual Studio 2017, so the previous change
to allow this header failed when building with Visual
Studio 2015.

Restores the appveyor continuous integration build.

* Add support to CMake build for building frameworks on Apple systems.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - add headers to project

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - only publish opus_custom.h if custom modes is enabled xiph#121

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - move cmake related files to cmake folder to make root dir less busy

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - add include guards to cmake files

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - move all compiler feature detection to OpusConfig

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - add option to disable intrinsics

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake/CFeatureCheck.cmake: fix feature tests failing when Opus is a submodule

CMAKE_SOURCE_DIR corresponds to the top project's source directory.
CMAKE_BINARY_DIR corresponds to the top project's binary directory.

The usage of these variables doesn't cause any problems when Opus is built as a standalone project.

This is not the case when Opus is added as submodule: the variables are set by the project that calls "add_subdirectory()".

The fix consists in using PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR, which always refer to the current project.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - refactor and cleanup options

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* opus draft: fix run failed under dash

[[ ]], the compound command is not supported by all
shell interpreter. [ ], the buildin command is more
common.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* fix doc/build_draft.sh run error

Add newer source directories to the destdir file tree so we
can include all sources referenced from opus_sources.mk.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* Silence clang silk_encoder alignment warning

* Prefer SSE and ASM implementation of float2int before lrintf for MSVC

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Fix trailing whitspace in previous commit

Signed-off-by: Ralph Giles <giles@thaumas.net>

* Fix arm build with rtcd enabled.

The autotools build doesn't set OPUS_HAVE_RTCD for arm targets,
assuming all the supported intrinsics will work on the runtime
cpu.

The cmake build however defines this by default when the neon
extension is available on the target. On Linux, the runtime
cpu detection reads /proc/cpuinfo, so removing stdio.h from
celt/os_support.h meant that the cmake build for arm targets
failed.

We don't currently have ci runs for that configuration, so
this only became apparent through manual testing.

Signed-off-by: Marcus Asteborg <maastebo@microsoft.com>
Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Add CI to check for trailing whitespaces in merge requests

* Remove trailing whitespaces in vla.c that was missed previously

* Fix and clean up opus_decode_fuzzer

Use the fuzzed sub-length of the input data instead of the whole input.

* configure: adjust x86 get cpu info inline assembly method for PIC case

.. just like the way it is done in celt/x86/x86cpu.c.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* gitlab-ci: Run jobs on the gcc:9 image.

Use a versioned gcc container image for more consistent
test results. This is the same version we're using for
other projects, but of course it will need to be bumped
periodically.

The current gcc release is 10.2. The oldest supported
release is 8.4, so this is in the middle of the support
window.

Signed-off-by: Marvin Scholz <epirat07@gmail.com>

* cmake - add warning flags for clang, gcc etc.

* cmake - Fix typo bug in enable_float_api option

* cmake - fix bugs around consuming Opus as a submodule in cmake and package version parsing

* CMake: Make _FORTIFY_SOURCE optional

* cmake - MINGW check for ssp lib and link if security features is enabled

* opus_decoder_fuzzer: limit the number of decodes to avoid timeout

* silk: Fix incorrect ifdef in debug.c

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* celt: Fix broken SSE pre-processor check due to typo

This broke 5 years ago in 43120f0

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Replace WIN32 with _WIN32 everywhere

_WIN32 is defined on all Windows platforms by every compiler that
targets Windows. We do not need WIN32 at all.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>
Resolves xiph#104

* repacketizer_demo: check for read errors to fix compiler warnings

Actually check for read errors instead of just storing the
return value in a variable that then never gets checked.

Also fixes "conversion from 'size_t' to 'int', possible loss
of data" compiler warnings on Windows with MSVC caused by
storing the size_t returned by fread() into an int variable.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Fix MSVC warning about trunction from double to float

Specify the precision as float to avoid truncating from double.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Add support for Meson build system

Tested on:
 - Linux/x86* with gcc
 - Android armv7 arm64 x86 x86_64 with clang
 - Windows x86 x86_64 with Visual Studio 2017
 - Windows x86 x86_64 with MinGW
 - macOS x86_64 with clang
 - iOS arm64 x86_64 with clang

Co-authored by: Nirbheek Chauhan <nirbheek@centricular.com>

https://gitlab.xiph.org/xiph/opus/-/merge_requests/13

* cmake - change logging of api version to debug

Signed-off-by: Ralph Giles <giles@thaumas.net>

* CMakeLists.txt: specify path to target file in add_test() directives

This is required in case the output path for tests is changed
by a project adding Opus as submodule.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* CMakeLists.txt: specify working directory in add_test() directives

This is required for Windows because it doesn't have RPATH,
thus it fails to find Opus if it's not in the same directory
as the executables or in PATH.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* cmake - add option for assertions

Signed-off-by: Ralph Giles <giles@thaumas.net>

* cmake - add option for hardening

Signed-off-by: Ralph Giles <giles@thaumas.net>

* cmake - add option for fuzzing

Signed-off-by: Ralph Giles <giles@thaumas.net>

* cmake - add option for check asm

Signed-off-by: Ralph Giles <giles@thaumas.net>

* cmake - add option for fixed point debug

Signed-off-by: Ralph Giles <giles@thaumas.net>

* Meson: Fix doc build when opus is a subproject

meson.source_root() and meson.build_root() have been deprecated in
latest Meson release because they are a trap. They point to the root of
parent project instead of root of current subproject. Meson 0.56.0 added
meson.project_source/build_root() but it is just as easy to use
meson.current_source/build_dir() in the root meson.build file and avoids
bumping required meson version.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* docs: fix simple typo, neareast -> nearest

There is a small typo in celt/fixed_generic.h.

Should read `nearest` rather than `neareast`.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* ci: fix pipeline run for merge requests

This way CI pipeline runs for branches and tags
and makes it show up in merge requests where
a branch is used as source branch.

Makes all jobs show up in merge request CI
indicator (not just the whitespace job) and
only runs a single CI pipeline, without the
additional detached pipeline.

https://docs.gitlab.com/ce/ci/yaml/#workflowrules-templates

Signed-off-by: Ralph Giles <giles@thaumas.net>

* Fix float-approx negative left shift UB

Reported by toto.

* Sending refresh DTX packets every 400 ms independently of the encoded frame size.

Signed-off-by: Felicia Lim <flim@google.com>

* celt_lpc: avoid overflows when computing lpcs in fixed point

The LPCs are computed in 32-bit, so increase the allowed range from +/-8
to +/-64 to avoid overflows caught during fuzzing. Before downshifting
back down to the +/-8 range in the final 16-bit output, perform bandwidth
extension to avoid any additional overflow issues.

* Fix trailing whitespace.

This was introduced in February, and fails the corresponding
check in gitlab ci runs.

Also indent the subsequent lines to match and correct typos.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - add support to run ctest on android #2347

Signed-off-by: Ralph Giles <giles@thaumas.net>

* Relax comparison to 0 to avoid a floating point divide-by-zero error.

* Revert relaxing comparison to 0 for fixed point only

* meson: fix get-version script for git worktrees

For git worktree directories .git is not a directory
but a file that points to the real .git dir.

The `update_version` script used by other builds
works correctly with git worktrees.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* Remove an unused parameter

* Check channels/stream counts and mapping when creating the multistream
encoder

* Fix buffer overflow in xcorr_kernel_sse4_1

Before, an overflow can occur in the last loop if `len` is not a
multiple of 4 as OP_CVTEPI16_EPI32_M64 tries to load 64 bits, but there
are insufficient bits allocated in `x`.

* Disable dangerous SSE 4.1 intrinsic optimizations

These could result in 16-byte-aligned loads on unaligned data, causing
a segfault.

* Initialize non-zero test arrays.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Prevent int32 overflow when applying HARM FIR filter in NSQ.c by using a saturating sum. This matches behavior in NSQ_del_dec.c.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Update and re-enable SILK SSE4.1 optimisations

* build test scripts

* Cleanup testing directories to save space

* Fix lrint/lrintf detection

Prevents using lrint/lrintf when compiling with -std=c90 even though the
functions are in libm. This was causing tests to fail, likely due to
incorrect prototypes.

* print rate used for testvectors

* Correct redundancy handling with lost/DTX frames

In xiph#253, the encoder generates a
Hybrid frame with redundancy, to switch to CELT-only mode, and then
activates DTX immediately afterwards.  The decoder ran Hybrid PLC,
which isn't right.  Use CELT PLC instead if there was already a
transition to CELT via redundancy at the end of the previous frame.

Also do not use a stale CELT decoder to decode a second redundancy
frame when the first redundancy frame for a transition from SILK-only
mode was lost.  Instead of mixing in old audio from the last time
that CELT was used, ignore the second redundancy frame in this case.
Alternatively the CELT decoder could be reset before decoding, but
it would not be ready until after the 2.5 ms of audio that is needed.

Reviewed by Jean-Marc Valin.

* Improve background noise estimation for CELT DTX

We now update the background noise estimate even in frames classified
as transient. It shouldn't be a problem because we're using min
statistics. Also, it avoids problems when update frames get
missclassified as transient.

In addition, we now use the duration of losses rather than the
number of lost packets to make decisions. That should make
PLC/DTX behaviour more consistent across frame sizes.

* Fix 8101b33 to decode ignored redundancy

Even if the redundancy is ignored, the final range from the decoder is
needed for testing.

Reviewed by Timothy B. Terriberry.

* Fixes wrap-around in silk_inner_prod16_sse4_1()

Thanks Tim

* Silence some warnings for fixed-point debug builds

Reviewed by Timothy B. Terriberry.

* Fixes --disable-rtcd

Make sure we don't try to use the rtcd table when rtcd is disabled.
That code still needs a lot more cleaning up.

* Fixes valgrind failure caused by silk_find_pred_coefs_*()

The function copies NLSFs from the stack to the state which for
order 10 means we were copying uninitialized values. That in turn
breaks check-asm when comparing the state under valgrind.

Reviewed by Timothy B. Terriberry.

* Check the return value of __get_cpuid().

This function can fail if CPUID is not supported or the maximum
 supported value of EAX is less than the requested one.
Check the return value and explicitly disable all SIMD if it does
 fail.
This was happening before implicitly because of the initialization
 of info[] to zero, but being explicit about it makes it less likely
 someone will break this behavior because they did not realize what
 was going on.

* Work around a valgrind false-positive in CPUID.

Valgrind versions prior to 3.17.0 assume that an uninitialized value
 in ECX causes the whole output of CPUID to be uninitialized, even
 though ECX is only "read" by CPUID for certain values of EAX.
Work around that by guaranteeing that ECX is initialized.

* Adds fuzzing to CPU detection

Makes ti possible to randomize (with --enable-fuzzing) the CPU flags
so we can better test all the intrinsics implementations.

* Estimate the inner product accuracy to fix check-asm

Estimate the rounding error so that we can have a useful margin of
error when checking the asm against the C code even when the float
operations get reordered due to -ffast-math.

* Adds OPUS_SET_INBAND_FEC(2) option

Unlike OPUS_SET_INBAND_FEC(1), the encoder does not necessarily
switch to SILK if we have music.

* Add asan/ubsan support in random tests

* Work around UBSan unaligned access errors.

The underlying objects are all 8-bit integers.
Verified that the generated assembly still just uses MOVD.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Avoid left shifts of negative values in debug macros

Reviewed by Mark Harris

* Fix some 16-bit overflows (using 32-bit macros)

Reviewed by Mark Harris

* Fix fixed-point overflow in pitch downsampling

Reviewed by Mark Harris

* Avoid undefined behaviour within the debug macros

Even when the macro itself would overflow.

Reviewed by Mark Harris

* Avoids incrementing uninitialized values

The values were never used, but ubsan + valgrind would complain.

Reviewed by Mark Harris

* Silence GCC 11+ -Wmaybe-uninitialized warnings

Reviewed by Timothy B. Terriberry.

* Fix quoting and whitespace errors in build test

Reviewed by Jean-Marc Valin.

* Fix warning with --disable-rfc8251

* Replace assert with test_failed function in test

This will fix -Wunused-but-set-variable on gcc
9.3 release build. Also remove unused assert.h.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Remove unused variable in tests

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Fix opus.h for doxygen when referencing alternative values

Doxygen was not able to resolve the references because it looked
for OPUS_APPLICATION_VOIP/@ref.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* doc: Use consistent alternative notation

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - Add OPUS_BUILD to test targets

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Fix warnings when compiling with FUZZING enabled

* Update x86 CPU detection configure check.

Commit 6577534 switched from using __get_cpuid() to
 __get_cpuid_count(), but the corresponding configure check was not
 updated.
Since __get_cpuid_count() was introduced much later, make sure we
 check for the function we actually use.

Thanks to Mark Harris for the report.

* meson: Fix reporting of cpu family if intrinsics not supported

Signed-off-by: Doug Nazar <nazard@nazar.ca>

* Fix uninitialized field on custom mode malloc fail

* cmake - fix lrintf, lrint detection

This commit addresses the issues of not finding lrintf and lrint. We
switch to check_symbol_exists instead per cmake documentation. Also
make sure to link math lib for detection for nix.

For MSVC the issue for non x86 builds was that the standard was set to
default which is 199409L. This resulted in not using lrintf even that
it was found. To address this we set the C standard to C11 and it will
only apply to newer versions of MSVC where the /std flag is supported.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - fix rtcd detection on x86 non windows

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* cmake - move warning C4244 to level 4

Opus compare is used to generate test vectors so no cosmetic changes
is taken. Hence we move this warning to level 4 for opus compare.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* update doc on custom mode

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* Fix C90-related warnings

* Make silk/x86 header indentation consistent.

The indentation for nested #ifs was all over the place.

* Don't compile x86 cpu detection without RTCD.

Also #error if RTCD is enabled without a detection method, like Arm.
A number of SILK functions also still used the lookup tables, even
 when RTCD was disabled.
Fix those, too.

* Only build platform RTCD sources when enabled.

To avoid issues with empty compilation units.

* Silence Clang 13+ null-pointer-subtraction warning

* Silence MSVC C4244 warning

When building with FLOAT_APPROX.

Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Fix NORM_ALIASING_HACK

We need to move the history out of the way before we write to the
shape array X, or else we get corruption of the audio.

Signed-off-by: Jean-Marc Valin <jmvalin@amazon.com>

* Relaxing checks for MULT16_32_QX()

MULT16_32_QX() is now implemented using a signed-unsigned multiply,
so the second argument can now have one extra bit compared to the
old signed-signed implementation.

Reviewed by Mark Harris

* Using saturating round to fix some wrap-arounds

Reviewed by Mark Harris

* More ubsan fixes for the debug macros themselves

Reviewed by Mark Harris

* Ensuring we can see where crashes occur

Reviewed by Mark Harris

* Re-tuning the use of LTP scaling

Making LTP scaling depend on the bitrate and whether FEC is on.
The thresholds for scaling 1 and 2 are now independent.

* More FEC tuning: lowering the LBRR bitrate a bit

* Smooth out the LBRR rate estimate

Reduces fluctuations in the non-FEC target bitrate.

* Change pitch scaling behavior wrt nFramesPerPacket

Not sure if it was the original intent, but we now reduce the
loss percentage threshold for pitch scaling as 1/nFramesPerPacket
since only the first frame will have pitch scaling anyway.
As a side effect, this brings back the original behavior of
disabling pitch scaling for 0% loss.

* Fix typo in MacroDebug.h comment.

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* opus.m4: fix -Wstrict-prototypes

Signed-off-by: Sam James <sam@gentoo.org>
Signed-off-by: Mark Harris <mark.hsj@gmail.com>

* Make CELT FFT twiddle complex type aligned

This makes kiss_twiddle_cpx 4-byte aligned (instead of 2-byte) for
fixed-point builds. Tested with an armv6j+nofp development board, CELT
encoding becomes 1.4x as fast, and decoding over 2x.

Performance gain is mostly attributed to the proper alignment of the
static const array mdct_twiddles960.

Co-authored-by: David Gao <davidgao@google.com>
Signed-off-by: Felicia Lim <flim@google.com>

* Bump LT version

Added OPUS_SET_INBAND_FEC(2) since previous version

* Avoid "ISO C forbids an empty translation unit"

Add dummy typedef to avoid the warning

* oops, avoid using a reserved identifier

* docs: replace fgrep with grep -F

It's been deprecated for decades and in Debian system it's starting
to print warnings. Just use grep -F instead.

Signed-off-by: Ralph Giles <giles@thaumas.net>

* meson: fix build on arm64

Would fail like:

Checking if "compiler supports ARMv7/AArch64 NEON intrinsics" : links: NO
Checking if "compiler supports ARMv7/AArch64 NEON intrinsics with -mfpu=neon" : links: YES
Checking if "compiler supports AArch64 NEON intrinsics" : links: NO
Checking if "compiler supports AArch64 NEON intrinsics with -mfpu=neon" : links: NO
Message: Compiler does not support AArch64 NEON intrinsics
../silk/meson.build:28:45: ERROR: Unknown variable "have_arm_intrinsics_or_asm".

since commit 0808841.

* ci: add arm64 CI

* ci: add ci-fairy linter to make sure commits are GPG signed

* cmake instructions

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* CAR-2144 test some changes

* Fix Gitlab CI

Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>

* CAR-2144 remove old endif

* CAR-2144 remove run_config script

* Feature/car 2144   fix opus cmake sumit (#6)

* CAR-2144 set intall to OFF

* CAR-2144 remove opus cpack

---------

Signed-off-by: Felicia Lim <flim@google.com>
Signed-off-by: Jean-Marc Valin <jmvalin@jmvalin.ca>
Signed-off-by: Ralph Giles <giles@thaumas.net>
Signed-off-by: Mark Harris <mark.hsj@gmail.com>
Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Marcus Asteborg <maastebo@microsoft.com>
Signed-off-by: Marvin Scholz <epirat07@gmail.com>
Signed-off-by: Doug Nazar <nazard@nazar.ca>
Signed-off-by: Jean-Marc Valin <jmvalin@amazon.com>
Signed-off-by: Sam James <sam@gentoo.org>
Co-authored-by: Felicia Lim <flim@google.com>
Co-authored-by: Gustaf Ullberg <gustaf.ullberg@gmail.com>
Co-authored-by: Davide Beatrici <davidebeatrici@gmail.com>
Co-authored-by: Marcus Asteborg <maastebo@microsoft.com>
Co-authored-by: Nathaniel R. Lewis <linux.robotdude@gmail.com>
Co-authored-by: DeadSix27 <DeadSix27@users.noreply.github.com>
Co-authored-by: Jesús de Vicente Peña <devicentepena@webrtc.org>
Co-authored-by: Niclas Olmenius <niclas@voysys.se>
Co-authored-by: evpobr <evpobr@gmail.com>
Co-authored-by: willson-chen <willson.chenwx@gmail.com>
Co-authored-by: Hua, Chunbo <chunbo.hua@intel.com>
Co-authored-by: Ralph Giles <giles@thaumas.net>
Co-authored-by: Mark Harris <mark.hsj@gmail.com>
Co-authored-by: Hector Martin <marcan@marcan.st>
Co-authored-by: Simon Jackson <si@sonocent.com>
Co-authored-by: Davide Beatrici <git@davidebeatrici.dev>
Co-authored-by: sezero <sezero@users.sourceforge.net>
Co-authored-by: Zeno Endemann <zeno.endemann@googlemail.com>
Co-authored-by: Nirbheek Chauhan <nirbheek@centricular.com>
Co-authored-by: Tim-Philipp Müller <tim@centricular.com>
Co-authored-by: Xavier Claessens <xavier.claessens@collabora.com>
Co-authored-by: Tim Gates <tim.gates@iress.com>
Co-authored-by: Jean-Marc Valin <jmvalin@jmvalin.ca>
Co-authored-by: Tom Denton <tomdenton@google.com>
Co-authored-by: Francis Quiers <fquiers@cisco.com>
Co-authored-by: Jean-Marc Valin <jmvalin@amazon.com>
Co-authored-by: Timothy B. Terriberry <tterribe@xiph.org>
Co-authored-by: Alexander Traud <pabstraud@compuserve.com>
Co-authored-by: Timothy B. Terriberry <territim@amazon.com>
Co-authored-by: Doug Nazar <nazard@nazar.ca>
Co-authored-by: Nathan E. Egge <eggenath@amazon.com>
Co-authored-by: Sam James <sam@gentoo.org>
Co-authored-by: Zheng Lv <lvzheng@google.com>
Co-authored-by: David Gao <davidgao@google.com>
Co-authored-by: Claudio Saavedra <csaavedra@igalia.com>
Co-authored-by: Marcus Asteborg <xnorpx@outlook.com>
Co-authored-by: sumitkandpal <sumit@airtime.com>
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

No branches or pull requests

3 participants