<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
Hi Mark,</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<br>
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
Thanks for more feedback,</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<span title="Search for suggestions"><br>
</span></div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<span title="Search for suggestions">I will create tasks in the OPUS github page for this to improve the CMake integration as I don't have bandwidth to fix all these things asap.</span><br>
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<br>
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
It is CMake best practices to only build either static or dynamic library. If the library consumer needs both then do a dual build.</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<br>
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
Regarding testing only tests that was depending on the exported API in shared lib was built hence only 2 vs 4 for static lib.</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
The small unitttests was not added by choice but can ofc be added. Added task for this.</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<br>
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<div style="border-bottom-color:currentColor; border-left-color:currentColor; border-right-color:currentColor; border-top-color:currentColor; font-family:Calibri,Helvetica,sans-serif; font-size:12pt; font-size-adjust:none; margin-bottom:0px; margin-top:0px">
<font style="background-color:rgb(255,255,255)">The reason for the difference between BUILD_TESTING and OPUS_BUILD_PROGRAMS is that BUILD_TESTING is a ctest variable while OPUS_ is our own defined.</font></div>
<div style="border-bottom-color:currentColor; border-left-color:currentColor; border-right-color:currentColor; border-top-color:currentColor; font-family:Calibri,Helvetica,sans-serif; font-size:12pt; font-size-adjust:none; margin-bottom:0px; margin-top:0px">
I agree that the printout should be fixed to reflect the option prefix OPUS_ for all the OPUS_ specific options. Added task for this.<br>
</div>
<div style="border-bottom-color:currentColor; border-left-color:currentColor; border-right-color:currentColor; border-top-color:currentColor; font-family:Calibri,Helvetica,sans-serif; font-size:12pt; font-size-adjust:none; margin-bottom:0px; margin-top:0px">
<br>
</div>
<div style="border-bottom-color:currentColor; border-left-color:currentColor; border-right-color:currentColor; border-top-color:currentColor; font-family:Calibri,Helvetica,sans-serif; font-size:12pt; font-size-adjust:none; margin-bottom:0px; margin-top:0px">
Added three separate tasks for the options.</div>
<div style="border-bottom-color:currentColor; border-left-color:currentColor; border-right-color:currentColor; border-top-color:currentColor; font-family:Calibri,Helvetica,sans-serif; font-size:12pt; font-size-adjust:none; margin-bottom:0px; margin-top:0px">
<br>
</div>
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
I don't have a ARM based Linux device so I cannot test the warning issue, but I created a task for it for anyone with a Linux ARM device to pick up.</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<br>
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
The platform I tested the CMake integration on was (for other flavors one need to use toolchain files):<br>
</div>
<ul>
<li>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
Windows:</div>
</li><ul>
<li>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
X86</div>
</li><li>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
X64</div>
</li><li>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
ARM</div>
</li></ul>
<li><font color="#b00000" face="Calibri,Helvetica,sans-serif">Linux (Ubuntu):</font></li><ul>
<li>X64</li></ul>
<li>Mac OSX:</li><ul>
<li>X64</li></ul>
<li>Android:</li><ul>
<li>X86</li></ul>
<li>Android</li><ul>
<li>ARM<font style="background-color:rgb(255,255,255)"><span style="display:inline!important; background-color:rgb(255,255,255); font-size:11pt; font-size-adjust:none"><br>
</span></font></li></ul>
</ul>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<font style="background-color:rgb(255,255,255)"><span style="display:inline!important; background-color:rgb(255,255,255); font-size:11pt; font-size-adjust:none"></span></font></div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
<span>https://github.com/xiph/opus/issues/120<br>
</span>
<div>https://github.com/xiph/opus/issues/121​</div>
<div>https://github.com/xiph/opus/issues/122​</div>
<div>https://github.com/xiph/opus/issues/123​</div>
<div>https://github.com/xiph/opus/issues/124​</div>
<div>https://github.com/xiph/opus/issues/125​</div>
<div>https://github.com/xiph/opus/issues/126​</div>
<span><a id="LPNoLP422387" href="https://github.com/xiph/opus/issues/127">https://github.com/xiph/opus/issues/127</a></span><br>
<font style="background-color:rgb(255,255,255)"><span style="display:inline!important; background-color:rgb(255,255,255); font-size:11pt; font-size-adjust:none"><br>
</span></font></div>
<div><font style="background-color:rgb(255,255,255)"><span style="display:inline!important; background-color:rgb(255,255,255); font-size:11pt; font-size-adjust:none"></span></font></div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
//Marcus</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
 <br>
</div>
<div style="color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif; font-size:12pt">
</div>
<div id="appendonsend"></div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font color="#000000" face="Calibri, sans-serif" style="font-size:11pt"><b>From:</b> Mark Harris <markh.sj@gmail.com> on behalf of Mark Harris <mark.hsj@gmail.com><br>
<b>Sent:</b> Saturday, April 13, 2019 22:12<br>
<b>To:</b> Marcus Asteborg<br>
<b>Cc:</b> Jean-Marc Valin; opus@xiph.org<br>
<b>Subject:</b> Re: [opus] Opus cmake build</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">Hi Marcus,<br>
<br>
Thanks for the fixes.  I did some more cmake build testing and<br>
encountered a few issues:<br>
<br>
The option -DFORTIFY_SOURCE=2 should be -D_FORTIFY_SOURCE=2, as the<br>
macro has a leading underscore.  In the autotools build it defines this<br>
if it is not already defined (m4/ax_add_fortify_source.m4).<br>
<br>
When custom modes are not enabled, the cmake build is nevertheless<br>
installing the include file opus_custom.h.  This file is specific to<br>
custom modes and should only be installed if custom modes were enabled.<br>
<br>
It appears that the documentation is not being built or installed by the<br>
cmake build; the autotools build builds and installs the documentation<br>
by default unless configured with --disable-doc.<br>
<br>
The option -DBUILD_SHARED_LIBS=ON causes a shared library to be built,<br>
however it then does not build a static library.  The autotools build<br>
builds both by default, although --disable-shared or --disable-static<br>
may be used to disable them.  Normally an installation would have both.<br>
 How can both be built with cmake?  Is it it necessary to build twice?<br>
<br>
When configured with -DBUILD_TESTING=ON, ctest runs only 2 tests if<br>
-DBUILD_SHARED_LIBS=ON was also used, or 4 tests without that option.<br>
How would someone run all 14 of the tests that are run using the<br>
autotools "make check"?  I wouldn't expect fewer tests to be run when<br>
shared libraries are built; the autotools build runs all tests<br>
regardless of whether a shared library, static library, or both are built.<br>
<br>
>  For programs use:<br>
> -DBUILD_PROGRAMS=ON<br>
<br>
I tried the option -DBUILD_PROGRAMS=ON that you mentioned, but it still<br>
said that BUILD_PROGRAMS was disabled:<br>
<br>
  -- The following features have been disabled:<br>
<br>
   * USE_ALLOCA, Use alloca for stack arrays (on non-C99 compilers)<br>
   * CUSTOM_MODES, Enable non-Opus modes, e.g. 44.1 kHz & 2^n frames<br>
   * BUILD_PROGRAMS, Build programs<br>
   * FIXED_POINT, compile as fixed-point (for machines without a fast<br>
enough FPU)<br>
   * X86_PRESUME_SSE4_1, assume target CPU has SSE4_1 support<br>
   * X86_PRESUME_AVX, assume target CPU has AVX support<br>
<br>
  -- Configuring done<br>
  -- Generating done<br>
  CMake Warning:<br>
    Manually-specified variables were not used by the project:<br>
<br>
      BUILD_PROGRAMS<br>
<br>
By looking through CMakeLists.txt I figured out that I need to specify<br>
-DOPUS_BUILD_PROGRAMS=ON.  Is it possible to make BUILD_TESTING and<br>
BUILD_PROGRAMS consistent?  Also the output of cmake is misleading, as<br>
it says BUILD_PROGRAMS not OPUS_BUILD_PROGRAMS.  For some settings it is<br>
even worse; for example it says FLOAT_API but the actual -D option that<br>
needs to be used to change that setting is -DOPUS_ENABLE_FLOAT_API=OFF.<br>
 Can these be made consistent as well?<br>
<br>
There also seems to be several configure options (see ./configure<br>
--help) that have no equivalent, including --enable-assertions,<br>
--disable-asm, --disable-intrinsics, --with-NE10, and rare options like<br>
--disable-rfc8251, --disable-hardening, --enable-fixed-point-debug,<br>
--enable-float-approx, --enable-fuzzing, and --enable-check-asm.<br>
<br>
I tried a cmake build on a non-x86 Linux host but encountered numerous<br>
warnings of the form:<br>
  celt/float_cast.h:129:10: warning: #warning "Don't have the functions<br>
lrint() and lrintf ()." [-Wcpp]<br>
  celt/float_cast.h:130:10: warning: #warning "Replacing these functions<br>
with a standard C cast." [-Wcpp]<br>
<br>
The host does have both the lrint and lrintf functions; the problem is<br>
that cmake did not define either HAVE_LRINT or HAVE_LRINTF.<br>
<br>
 - Mark<br>
</div>
</span></font></div>
</body>
</html>