Descent3 icon indicating copy to clipboard operation
Descent3 copied to clipboard

Remove pragma ignoring -Wnarrowing

Open Lgt2x opened this issue 1 year ago • 6 comments

#21 turned off the -Wnarrowing compiler warning in the file libmve/snd8to16.h to enable building on Linux. The short array snd_8to16 uses values that exceed signed short limits.

Figure out a way to remove the pragma, either by increasing the array size to 4-byte ints or clamping values to [-32768;32767]

Lgt2x avatar Apr 17 '24 19:04 Lgt2x

Will this work for clamping?

#include <stdint.h>

// Clamp values
int16_t clamp(int32_t value) {
    if (value < -32768)
        return -32768;
    else if (value > 32767)
        return 32767;
    else
        return (int16_t)value;
}

signed short snd_8to16[256] = {
    0,      1,      2,      3,      4,      5,      6,      7,      8,      9,      10,     11,     12,     13,
    14,     15,     16,     17,     18,     19,     20,     21,     22,     23,     24,     25,     26,     27,
    28,     29,     30,     31,     32,     33,     34,     35,     36,     37,     38,     39,     40,     41,
    42,     43,     47,     51,     56,     61,     66,     72,     79,     86,     94,     102,    112,    122,
    133,    145,    158,    173,    189,    206,    225,    245,    267,    292,    318,    348,    379,    414,
    452,    493,    538,    587,    640,    699,    763,    832,    908,    991,    1081,   1180,   1288,   1405,
    1534,   1673,   1826,   1993,   2175,   2373,   2590,   2826,   3084,   3365,   3672,   4008,   4373,   4772,
    5208,   5683,   6202,   6767,   7385,   8059,   8794,   9597,   10472,  11428,  12471,  13609,  14851,  16206,
    17685,  19298,  21060,  22981,  25078,  27367,  29864,  32589,  35563,  38808,  42350,  46214,  50431,  55033,
    60055,  65535,  -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768,
    -32768, -32768, -32768, -32768, -32768, -32768, -32768, -32768};

JeodC avatar Apr 23 '24 01:04 JeodC

@Lgt2x

JeodC avatar Apr 25 '24 00:04 JeodC

Will this work for clamping?

That just clamps everything, but this needs careful testing to see if the code is actually clamping or wrapping around, even incorrectly.

Arcnor avatar Apr 25 '24 07:04 Arcnor

We need to figure out precisely what the array is used for, and what the best solution is: clamping, re-interpolate all values in the short range, controlling overflow etc. I opened the issue to point out the problem, but it needs analysis. For example jumping from 65535 to -32768 might not be what the game intends

Lgt2x avatar Apr 25 '24 07:04 Lgt2x

The comment in snd8to16.h still has the original generation algorithm, and using it with std::clamp() like this

recreate_snd8to16.cpp
#include <stdio.h>
#include <math.h>

#include <algorithm>
#include <iostream>
#include <iomanip>

int main(void)
{
  signed int snd_8to16[256];
  int i;
  for (i=0; i<44; ++i) {
    snd_8to16[i] = i;
  }
  for (i=44; i<128; ++i) {
    snd_8to16[i] = std::clamp( (int)floor(pow(65535,i/127.0)+.5),
             (int)std::numeric_limits<short>::min(), (int)std::numeric_limits<short>::max());
  }
  for (i=1; i<128; ++i) {
    snd_8to16[256-i] = -snd_8to16[i];
  }
  snd_8to16[128] = snd_8to16[129];

  int c = 1;
  for (signed short s : snd_8to16) {
    std::cout << std::setfill(' ') << std::setw(6) << s << ", ";
    if (c++ % 14 == 0) {
      std::cout << std::endl;
    }
  }
  std::cout << '\n';
}

results in:

     0,      1,      2,      3,      4,      5,      6,      7,      8,      9,     10,     11,     12,     13, 
    14,     15,     16,     17,     18,     19,     20,     21,     22,     23,     24,     25,     26,     27, 
    28,     29,     30,     31,     32,     33,     34,     35,     36,     37,     38,     39,     40,     41, 
    42,     43,     47,     51,     56,     61,     66,     72,     79,     86,     94,    102,    112,    122, 
   133,    145,    158,    173,    189,    206,    225,    245,    267,    292,    318,    348,    379,    414, 
   452,    493,    538,    587,    640,    699,    763,    832,    908,    991,   1081,   1180,   1288,   1405, 
  1534,   1673,   1826,   1993,   2175,   2373,   2590,   2826,   3084,   3365,   3672,   4008,   4373,   4772, 
  5208,   5683,   6202,   6767,   7385,   8059,   8794,   9597,  10472,  11428,  12471,  13609,  14851,  16206, 
 17685,  19298,  21060,  22981,  25078,  27367,  29864,  32589,  32767,  32767,  32767,  32767,  32767,  32767, 
 32767,  32767, -32767, -32767, -32767, -32767, -32767, -32767, -32767, -32767, -32767, -32589, -29864, -27367, 
-25078, -22981, -21060, -19298, -17685, -16206, -14851, -13609, -12471, -11428, -10472,  -9597,  -8794,  -8059, 
 -7385,  -6767,  -6202,  -5683,  -5208,  -4772,  -4373,  -4008,  -3672,  -3365,  -3084,  -2826,  -2590,  -2373, 
 -2175,  -1993,  -1826,  -1673,  -1534,  -1405,  -1288,  -1180,  -1081,   -991,   -908,   -832,   -763,   -699, 
  -640,   -587,   -538,   -493,   -452,   -414,   -379,   -348,   -318,   -292,   -267,   -245,   -225,   -206, 
  -189,   -173,   -158,   -145,   -133,   -122,   -112,   -102,    -94,    -86,    -79,    -72,    -66,    -61, 
   -56,    -51,    -47,    -43,    -42,    -41,    -40,    -39,    -38,    -37,    -36,    -35,    -34,    -33, 
   -32,    -31,    -30,    -29,    -28,    -27,    -26,    -25,    -24,    -23,    -22,    -21,    -20,    -19, 
   -18,    -17,    -16,    -15,    -14,    -13,    -12,    -11,    -10,     -9,     -8,     -7,     -6,     -5, 
    -4,     -3,     -2,     -1

Then someone has to test-listen to the movies to verify :)

th1000s avatar Apr 25 '24 23:04 th1000s

Since we are replacing libmve (really need to do that soon), is this issue needed?

JeodC avatar Apr 26 '24 20:04 JeodC

Closing, this code will go away anyway with #289

Lgt2x avatar May 06 '24 13:05 Lgt2x