Problem with writing a 16bit raw PCM file

As a small experimental music piece I am attempting to program a song in standard C. The code outputs a raw PCM file which can be imported into Audacity. At the moment everything works as expected, but I'm encountering problems when trying to write each sample as 16 bit as opposed to the current 8 bit I am using.

Up until the point of being written, the current sample is calculated as a float, and its bounds are kept pretty much within the range of a signed 8 bit integer. It is then written as a 8 bit integer before repeating the process for the next sample. This works fine and plays properly. The problem occurs when I try to write it as a 16bit raw PCM file - I multiply the float by 256 and copy the result to a integer, whereupon I use fwrite to write the resulting 16bit integer. This does not giv开发者_StackOverflowe the expected results when imported, resulting in a highly distorted version of what I was expecting.

I've added the valid code below, since the problem occurs only at the writing stage.

Working 8bit code:

if (out<-127) {out=-128;} else if (out>126) {out=127;}
putc(out,fo);

Not working 16bit code:

if (out<-127) {out=-128;} else if (out>126) {out=127;}
pcm=out*256;
fwrite(&pcm,2,1,fo);

I'm probably just missing something obvious, but I've been trying to work it out for hours. Thanks in advance!

I can't tell what exactly is wrong in your code without seeing it, but this will make you a nice 1 KHz sine wave 16-bit PCM openable in Audacity:

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

#ifndef M_PI
#define M_PI 3.14159265358
#endif

int main(void)
{
  FILE* f = fopen("sinewave.pcm", "wb");
  double t;
  for (t = 0; t < 1; t += 1./8000) // 8000 is the sample rate in Hz
  {
    double sample = 15000 * sin(2 * M_PI * 1000 * t); // 1000 Hz sine wave
    short s16 = (short)sample;
    unsigned char c;
    c = (unsigned)s16 % 256;
    fwrite(&c, 1, 1, f);
    c = (unsigned)s16 / 256 % 256;
    fwrite(&c, 1, 1, f);
  }
  fclose(f);
  return 0;
}

In Audacity navigate through File->Import->Raw Data:

Encoding: Signed 16-bit PCM
Byte Order: Little-endian
Channels: 1 Channel (Mono)
Sample rate: 8000

Import.

I would imagine looking at the waveform in Audacity would've given you some clues.

Have you checked:

• the endianness is correct?
• that you're not supposed to be using e.g. unsigned integers?
• you've correctly marked the file as 16-bit?

I don't know what the expected format is for PCM, but these are all likely candidates for the problem.

It's good practice to do type casts when doing conversions. For example, if out is a float, then

putc((int) out, fo);

will let the compiler know that you want to write your number as an integer.

Sure, the compiler will figure that out anyway for something like putc, but this doesn't work for referencing. If you declare the pcm variable as a float, then fwrite will write floating point data instead of what you want. So i'll ask the same question: is pcm an integer type?

Another question is: do you really need floating point here? You might need it if you can use the decimal precision (then again, you'll lose that precision by outputting into an 8-bit or 16-bit format), but it's a waste if you only do simple math with your samples. Therefore you can simplify things a lot by sticking to an integer type, and converting that to a char/int8_t when writing.

Going on a limb here, but since you want signed 16-bit values, try this:

int16_t pcm = out * 256;
fwrite(&pcm, sizeof(pcm), 1, fo);

Also, make sure you've marked your file correctly, ie. raw PCM signed 16 bit with the appropriate endian-ness. (edit: this isn't applicable for PCM)

To zombie this thread:

From the WAV Wiki:

There are some inconsistencies in the WAV format: for example, 8-bit data is unsigned while 16-bit data is signed

You have to resample your code from floating point to integer, which means you need to round at some point to avoid adding noise and DC offset to your signal

float sample = out * 256.f;   // amplify to new range.
int pcm32 = (int)floorf(sample + .5f); // round and convert to 32 bit pcm.

// saturate just before conversion if possible, that's always safer.
if (pcm32 > SHRT_MAX) pcm32 = SHRT_MAX;
if (pcm32 < SHRT_MIN) pcm32 = SHRT_MIN;

short int pcm16 = (short int)pcm32;  // keep the lowest 16 bits

Have you considered using the standard normalized range of -1.0 to +1.0 for your amplitudes?