distorted output during processing .wav file

Question

I want to process a .wav file for example reducing amplitude; when i use following code the output becomes distorted and that's not pleasant.

#include <iostream>
#include <fstream>
#include <string>
using namespace std;
int main()
{
    char* wav_mem;
    ifstream wav_file;
    wav_file.open("1.wav", ios::binary | ios::ate);
    int file_size = wav_file.tellg();
    wav_mem = new char[file_size];
    wav_file.seekg(0, ios::beg);
    wav_file.read(wav_mem, file_size);
    int16_t sample = 0;
    wav_file.close();
    for(int i = 44; i <= file_size; i += 2)
    {   
        sample = ((wav_mem[i + 1] << 8) | (wav_mem[i]));
        sample = (int16_t)(sample * 0.5);
        wav_mem[i] = sample;
        wav_mem[i+1] = (sample >> 8);
    }
    ofstream out_file;
    out_file.open("out.wav", ios::binary);
    out_file.write(wav_mem, file_size);
}

How can I fix the distortion?

Answer 1

I solved the problem, i messed up samples when i was trying to convert two bytes to 16 bits, here is the final code:

#include <iostream>
#include <fstream>
#include <string>
#include <string.h>
using namespace std;

int main()
{
    ifstream wav_file;
    ofstream out_file;
    wav_file.open("input.wav",ios::binary|ios::ate);
    size_t file_size = wav_file.tellg();
    char * wav_buf = new char[file_size];
    wav_file.seekg (0,ios::beg);
    wav_file.read (wav_buf, file_size);
    wav_file.close();
    int16_t wav_smpl(0);
    char * wav_out = new char[file_size];
    memcpy(wav_out, wav_buf, 44);
    for (size_t i = 0 ; i < file_size ; i += 2) 
    {
        memcpy(&wav_smpl , wav_buf + (i + 44) , 2);
        wav_smpl *= 3;
        memcpy(wav_out + (i + 44) , &wav_smpl , 2);
    }
    out_file.open("output.wav",ios::binary);
    out_file.write(wav_out, file_size);
    out_file.close();
    return 0;
}

Answer 2

Assuming your actions on the .wav file itself is sound (otherwise I don't know much about it to know if that's the problem) some potential flaws could be:

1. Collecting the output of tellg in int might cause an overflow. Maybe use auto to get the right type ?
2. wav_mem[i+1] = (sample >> 8); when i == file_size in the loop that might cause an overflow access (beyond the length of wav_mem) ?

Edit: In-fact you can only access [0, file_size) indices in a defined manner due to your wav_mem = new char[file_size]; line. So when i = file_size both wav_mem[i] and wav_mem[i+1] will be UB.

Answer 3

I think the problem may lie in the use of the bitshift operator >> on signed integers. The actual behavior of << according to the standard has changed in C++14 and is going to change again in C++20 (cf. "Bitwise shift operators"). Either way, it is not a logical bitshift but an arithmetic bitshift.

Instead, I would use reinterpret_cast to convert both bytes into one 16-bit integer. I used something like this in the past:

int16_t num;
for (size_t i = 0; i < N && wav_file.read(reinterpret_cast<char*>(&num), 2); ++i) {
    audio[i] = double(num);
}

/* do stuff */

for (double x : audio) {
    num = static_cast<int16_t>(x);
    out_file.write(reinterpret_cast<char*>(&num), 2);
}

Note that this assumes a LittleEndian architecture, as RIFF uses LittleEndian.

Answer 4

Putting aside the mentioned earlier overflow in tellg and undefined behaviour in wav_mem[i + 1] I think this line is main problem:

sample = (int16_t)(sample * 0.5);

Behind the scenes the sample is converted here to double. Converting back and forth to double and from double may cause minor (but I suppose hearable) rounding errors which I suppose may be the source of distortion. Instead of this use:

sample /= 2;