libs/opus: New warnings including some somewhat worrying

[pljones]

Describe the bug

libs/opus produces some serious looking warnings on compile.

To Reproduce

Compiling on Linux.

Expected behavior

Compiles should be clean, without serious warnings.

Screenshots

gcc -c -pipe -O2 -D_REENTRANT -Wall -Wextra -fPIC -DAPP_VERSION=\"3.9.0dev-6f10b985\" -DCUSTOM_MODES -D_REENTRANT -DQT_NO_DEPRECATED_WARNINGS -DHAVE_LRINTF -DHAVE_STDINT_H -DSERVER_ONLY -DHEADLESS -DNO_JSON_RPC -DOPUS_X86_MAY_HAVE_SSE -DOPUS_X86_MAY_HAVE_SSE2 -DOPUS_X86_MAY_HAVE_SSE4_1 -DOPUS_X86_PRESUME_SSE=1 -DOPUS_X86_PRESUME_SSE2=1 -DCPU_INFO_BY_C -DOPUS_BUILD=1 -DUSE_ALLOCA=1 -DOPUS_HAVE_RTCD=1 -DHAVE_LRINTF=1 -DHAVE_LRINT=1 -DQT_NO_DEBUG -DQT_NETWORK_LIB -DQT_XML_LIB -DQT_CONCURRENT_LIB -DQT_CORE_LIB -I. -Isrc -Ilibs/opus/include -Ilibs/opus/celt -Ilibs/opus/silk -Ilibs/opus/silk/float -Ilibs/opus/silk/fixed -Ilibs/opus -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtNetwork -I/usr/include/x86_64-linux-gnu/qt5/QtXml -I/usr/include/x86_64-linux-gnu/qt5/QtConcurrent -I/usr/include/x86_64-linux-gnu/qt5/QtCore -I. -I/usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -o HP_variable_cutoff.o libs/opus/silk/HP_variable_cutoff.c
In file included from libs/opus/silk/float/structs_FLP.h:32,
                 from libs/opus/silk/float/main_FLP.h:33,
                 from libs/opus/silk/float/wrappers_FLP.c:32:
libs/opus/silk/float/wrappers_FLP.c: In function ‘silk_NSQ_wrapper_FLP’:
libs/opus/silk/main.h:296:18: warning: ‘silk_NSQ_del_dec_c’ reading 64 bytes from a region of size 32 [-Wstringop-overread]
  296 |     ((void)(arch),silk_NSQ_del_dec_c(psEncC, NSQ, psIndices, x16, pulses, PredCoef_Q12, LTPCoef_Q14, AR_Q13, \
      |     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  297 |                            HarmShapeGain_Q14, Tilt_Q14, LF_shp_Q14, Gains_Q16, pitchL, Lambda_Q10, LTP_scale_Q14))
      |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libs/opus/silk/float/wrappers_FLP.c:164:9: note: in expansion of macro ‘silk_NSQ_del_dec’
  164 |         silk_NSQ_del_dec( &psEnc->sCmn, psNSQ, psIndices, x16, pulses, PredCoef_Q12[ 0 ], LTPCoef_Q14,
      |         ^~~~~~~~~~~~~~~~
libs/opus/silk/main.h:296:18: note: referencing argument 6 of type ‘const opus_int16 *’ {aka ‘const short int *’}
  296 |     ((void)(arch),silk_NSQ_del_dec_c(psEncC, NSQ, psIndices, x16, pulses, PredCoef_Q12, LTPCoef_Q14, AR_Q13, \
      |     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  297 |                            HarmShapeGain_Q14, Tilt_Q14, LF_shp_Q14, Gains_Q16, pitchL, Lambda_Q10, LTP_scale_Q14))
      |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libs/opus/silk/float/wrappers_FLP.c:164:9: note: in expansion of macro ‘silk_NSQ_del_dec’
  164 |         silk_NSQ_del_dec( &psEnc->sCmn, psNSQ, psIndices, x16, pulses, PredCoef_Q12[ 0 ], LTPCoef_Q14,
      |         ^~~~~~~~~~~~~~~~
libs/opus/silk/main.h:275:6: note: in a call to function ‘silk_NSQ_del_dec_c’
  275 | void silk_NSQ_del_dec_c(
      |      ^~~~~~~~~~~~~~~~~~
libs/opus/silk/main.h:270:18: warning: ‘silk_NSQ_c’ reading 64 bytes from a region of size 32 [-Wstringop-overread]
  270 |     ((void)(arch),silk_NSQ_c(psEncC, NSQ, psIndices, x16, pulses, PredCoef_Q12, LTPCoef_Q14, AR_Q13, \
      |     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  271 |                    HarmShapeGain_Q14, Tilt_Q14, LF_shp_Q14, Gains_Q16, pitchL, Lambda_Q10, LTP_scale_Q14))
      |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libs/opus/silk/float/wrappers_FLP.c:167:9: note: in expansion of macro ‘silk_NSQ’
  167 |         silk_NSQ( &psEnc->sCmn, psNSQ, psIndices, x16, pulses, PredCoef_Q12[ 0 ], LTPCoef_Q14,
      |         ^~~~~~~~
libs/opus/silk/main.h:270:18: note: referencing argument 6 of type ‘const opus_int16 *’ {aka ‘const short int *’}
  270 |     ((void)(arch),silk_NSQ_c(psEncC, NSQ, psIndices, x16, pulses, PredCoef_Q12, LTPCoef_Q14, AR_Q13, \
      |     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  271 |                    HarmShapeGain_Q14, Tilt_Q14, LF_shp_Q14, Gains_Q16, pitchL, Lambda_Q10, LTP_scale_Q14))
      |                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libs/opus/silk/float/wrappers_FLP.c:167:9: note: in expansion of macro ‘silk_NSQ’
  167 |         silk_NSQ( &psEnc->sCmn, psNSQ, psIndices, x16, pulses, PredCoef_Q12[ 0 ], LTPCoef_Q14,
      |         ^~~~~~~~
libs/opus/silk/main.h:249:6: note: in a call to function ‘silk_NSQ_c’
  249 | void silk_NSQ_c(
      |      ^~~~~~~~~~
libs/opus/silk/float/wrappers_FLP.c: In function ‘silk_quant_LTP_gains_FLP’:
libs/opus/silk/float/wrappers_FLP.c:200:5: warning: ‘xX_Q17’ may be used uninitialized [-Wmaybe-uninitialized]
  200 |     silk_quant_LTP_gains( B_Q14, cbk_index, periodicity_index, sum_log_gain_Q7, &pred_gain_dB_Q7, XX_Q17, xX_Q17, subfr_len, nb_subfr, arch );
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from libs/opus/silk/float/structs_FLP.h:32,
                 from libs/opus/silk/float/main_FLP.h:33,
                 from libs/opus/silk/float/wrappers_FLP.c:32:
libs/opus/silk/main.h:211:6: note: by argument 7 of type ‘const opus_int32[20]’ {aka ‘const int[20]’} to ‘silk_quant_LTP_gains’ declared here
  211 | void silk_quant_LTP_gains(
      |      ^~~~~~~~~~~~~~~~~~~~
libs/opus/silk/float/wrappers_FLP.c:191:16: note: ‘xX_Q17’ declared here
  191 |     opus_int32 xX_Q17[ MAX_NB_SUBFR * LTP_ORDER ];
      |                ^~~~~~

Operating system

Ubuntu 22.04.1 LTS

Version of Jamulus

master

Additional context

Has the new dependency bot been and messed things up?

[hoffie]

From a quick glance, updating opus to latest master seems to fix this. I had once prepared to add opus as a submodule, but never got around to submitting a PR. Would it be worth it?

[pljones]

Am I right in thinking we've held back on updates to opus due to a belief there are local patches? Do we know what those are -- that is, do we have them clearly documented? If not, we need to get that done before making a decision at all. We may be able to work against a fork and continually rebase...

[hoffie]

Am I right in thinking we've held back on updates to opus due to a belief there are local patches? Do we know what those are -- that is, do we have them clearly documented?

I thought we had updated opus once already, but I can't find a PR. The reason for not updating opus is probably that there wasn't any newer official release and there was no reason to update to a development release. If there is one now, I don't see any reason to avoid that update.

The history looks pretty slim:

commit 7fa437292dcfd61d08e4fbb9ebd1cb0c500cd0e4
Author: Tony Mountifield <tony@mountifield.org>
Date:   Tue Jul 27 23:34:01 2021 +0100

    Add missing executable mode

commit 3e3fe44a7ff38da653c870cda0b716a3741de42d
Author: Stefan Weil <sw@weilnetz.de>
Date:   Mon Jun 29 21:18:54 2020 +0200

    Fix some typos (found by codespell)

    Signed-off-by: Stefan Weil <sw@weilnetz.de>

commit 36e6b37fa429dd77e4e5bf8a886dc3e66d8772aa
Author: Hector Martin <marcan@marcan.st>
Date:   Wed Jun 3 20:10:58 2020 +0900

    opus: fix equivalent bitrate calculation for <20ms frame sizes

commit 582f3e5270b844547b27062624b19bfc1f03a027
Author: Daniel Masato <masatod@amazon.com>
Date:   Fri Apr 10 11:43:23 2020 +0100

    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.
    See https://github.com/xiph/opus/issues/172.

commit ba63b7d82f4bfce37ca6c434cf01452ccab85a3b
Author: Daniel Masato <masatod@amazon.com>
Date:   Sat Apr 4 11:16:34 2020 +0100

    Upgrade to OPUS v1.3 - library source

    Downloaded from https://archive.mozilla.org/pub/opus/opus-1.3.1.tar.gz

There's only two patches which need confirmation again (I'm pretty sure they are from upstream). The other two (top-most) patches are Jamulus-specific changes as part of all-code-base adjustments (which shouldn't have been done to libs/ in the first place, I guess).

[ann0see]

We may be able to work against a fork and continually rebase...

Shouldn't a diff be enough from the version we are at now?

Otherwise, we can ask Volker.

[pljones]

Bumping to 3.10.0 as not seen as urgent enough for 3.9.1.

[pljones]

No one assigned. Dropping milestone and moving back to Triage.

[ann0see]

@corrados do you know of any Jamulus specific changes to the opus source code?

[corrados]

Very, very long ago I applied some patch to the Celt library. I documented all changes in a file named: "README_LLCON".

Some time later (in 2015), I modified OPUS slightly for eliminating artifacts and I fixed a compiler warning.

In 2017, there was a security issue found in OPUS. I merged the patch of that fix in the current Jamulus OPUS code.

I think none of these changes are relevant for the current Jamulus code.

[pljones]

Would it be worth us getting the Jamulus changes submitted to upstream for libopus and then using it natively, rather than with custom patches?

[ann0see]

Ok. So I believe only the 2015 patch is something custom for Jamulus. All the other stuff should be in the latest release of Opus (?)

[ann0see]

Actually, the current version doesn't even have the 2015 patch. I believe we can upgrade opus.

[pljones]

If the 2015 patch is still valid (i.e. it's beneficial), it might be worth rebasing it and resubmitting to the Opus developers once we have it integrated into Jamulus as a proof of concept.

[ann0see]

I'm not sure. But probably it's still valid. Someone better more than one with a well trained ear/ears should judge.