Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

It actually doesn't look too bad. Some minor nitpicks

  1. You shouldn't cast the return value of mallocYou shouldn't cast the return value of malloc.

  2. fByteRate and fBlockAlign can be const since they're being computed from const values only.

  3. I'm not entirely sure why pretty much all variables in WriteWavePCM are prefixed with f. I suppose this is meant to indicate that they are related to data being written to a file. Since most code in this method is related to writing things to a file this seems redundant to me and a bit distracting but YMMV.

  4. A magic constant is being used here fChunkSize = 36 + fSubchunk2Size; - replace with named constant (calculated or defined - depends where the 36 comes from).

  5. fBitsPerSample is defined to 16 which assumes that short is 16bits (since it's related to writing the short* sound data into the file). This may not be the case on all platforms. So fBitsPerSample should either be sizeof(*sound) or being passed in.

  6. You use inconsistent bracing style:

     int main(int argc, char* argv[]){
    

vs

    if (!sound)
    {
  1. You have defined N_CHANNELS but the main loop is hard coded to increment by 2, so this

     for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    

should be

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += N_CHANNELS, j++)

It actually doesn't look too bad. Some minor nitpicks

  1. You shouldn't cast the return value of malloc.

  2. fByteRate and fBlockAlign can be const since they're being computed from const values only.

  3. I'm not entirely sure why pretty much all variables in WriteWavePCM are prefixed with f. I suppose this is meant to indicate that they are related to data being written to a file. Since most code in this method is related to writing things to a file this seems redundant to me and a bit distracting but YMMV.

  4. A magic constant is being used here fChunkSize = 36 + fSubchunk2Size; - replace with named constant (calculated or defined - depends where the 36 comes from).

  5. fBitsPerSample is defined to 16 which assumes that short is 16bits (since it's related to writing the short* sound data into the file). This may not be the case on all platforms. So fBitsPerSample should either be sizeof(*sound) or being passed in.

  6. You use inconsistent bracing style:

     int main(int argc, char* argv[]){
    

vs

    if (!sound)
    {
  1. You have defined N_CHANNELS but the main loop is hard coded to increment by 2, so this

     for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    

should be

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += N_CHANNELS, j++)

It actually doesn't look too bad. Some minor nitpicks

  1. You shouldn't cast the return value of malloc.

  2. fByteRate and fBlockAlign can be const since they're being computed from const values only.

  3. I'm not entirely sure why pretty much all variables in WriteWavePCM are prefixed with f. I suppose this is meant to indicate that they are related to data being written to a file. Since most code in this method is related to writing things to a file this seems redundant to me and a bit distracting but YMMV.

  4. A magic constant is being used here fChunkSize = 36 + fSubchunk2Size; - replace with named constant (calculated or defined - depends where the 36 comes from).

  5. fBitsPerSample is defined to 16 which assumes that short is 16bits (since it's related to writing the short* sound data into the file). This may not be the case on all platforms. So fBitsPerSample should either be sizeof(*sound) or being passed in.

  6. You use inconsistent bracing style:

     int main(int argc, char* argv[]){
    

vs

    if (!sound)
    {
  1. You have defined N_CHANNELS but the main loop is hard coded to increment by 2, so this

     for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    

should be

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += N_CHANNELS, j++)
fixed typo
Source Link
ChrisWue
  • 20.6k
  • 4
  • 43
  • 107

It actually doesn't look too bad. Some minor nitpicks

  1. You shouldn't cast the return value of malloc.

  2. fByteRate and fBlockAlign can be const since theythey're being computed from const values only.

  3. I'm not entirely sure why pretty much all variables in WriteWavePCM are prefixed with f. I suppose this is meant to indicate that they are related to data being written to a file. Since most code in this method is related to writing things to a file this seems redundant to me and a bit distracting but YMMV.

  4. A magic constant is being used here fChunkSize = 36 + fSubchunk2Size; - replace with named constant (calculated or defined - depends where the 36 comes from).

  5. fBitsPerSample is defined to 16 which assumes that short is 16bits (since it's related to writing the short* sound data into the file). This may not be the case on all platforms. So fBitsPerSample should either be sizeof(*sound) or being passed in.

  6. You use inconsistent bracing style:

     int main(int argc, char* argv[]){
    

vs

    if (!sound)
    {
  1. You have defined N_CHANNELS but the main loop is hard coded to increment by 2, so this

     for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    

should be

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += N_CHANNELS, j++)

It actually doesn't look too bad. Some minor nitpicks

  1. You shouldn't cast the return value of malloc.

  2. fByteRate and fBlockAlign can be const since they being computed from const values only.

  3. I'm not entirely sure why pretty much all variables in WriteWavePCM are prefixed with f. I suppose this is meant to indicate that they are related to data being written to a file. Since most code in this method is related to writing things to a file this seems redundant to me and a bit distracting but YMMV.

  4. A magic constant is being used here fChunkSize = 36 + fSubchunk2Size; - replace with named constant (calculated or defined - depends where the 36 comes from).

  5. fBitsPerSample is defined to 16 which assumes that short is 16bits (since it's related to writing the short* sound data into the file). This may not be the case on all platforms. So fBitsPerSample should either be sizeof(*sound) or being passed in.

  6. You use inconsistent bracing style:

     int main(int argc, char* argv[]){
    

vs

    if (!sound)
    {
  1. You have defined N_CHANNELS but the main loop is hard coded to increment by 2, so this

     for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    

should be

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += N_CHANNELS, j++)

It actually doesn't look too bad. Some minor nitpicks

  1. You shouldn't cast the return value of malloc.

  2. fByteRate and fBlockAlign can be const since they're being computed from const values only.

  3. I'm not entirely sure why pretty much all variables in WriteWavePCM are prefixed with f. I suppose this is meant to indicate that they are related to data being written to a file. Since most code in this method is related to writing things to a file this seems redundant to me and a bit distracting but YMMV.

  4. A magic constant is being used here fChunkSize = 36 + fSubchunk2Size; - replace with named constant (calculated or defined - depends where the 36 comes from).

  5. fBitsPerSample is defined to 16 which assumes that short is 16bits (since it's related to writing the short* sound data into the file). This may not be the case on all platforms. So fBitsPerSample should either be sizeof(*sound) or being passed in.

  6. You use inconsistent bracing style:

     int main(int argc, char* argv[]){
    

vs

    if (!sound)
    {
  1. You have defined N_CHANNELS but the main loop is hard coded to increment by 2, so this

     for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    

should be

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += N_CHANNELS, j++)
Source Link
ChrisWue
  • 20.6k
  • 4
  • 43
  • 107

It actually doesn't look too bad. Some minor nitpicks

  1. You shouldn't cast the return value of malloc.

  2. fByteRate and fBlockAlign can be const since they being computed from const values only.

  3. I'm not entirely sure why pretty much all variables in WriteWavePCM are prefixed with f. I suppose this is meant to indicate that they are related to data being written to a file. Since most code in this method is related to writing things to a file this seems redundant to me and a bit distracting but YMMV.

  4. A magic constant is being used here fChunkSize = 36 + fSubchunk2Size; - replace with named constant (calculated or defined - depends where the 36 comes from).

  5. fBitsPerSample is defined to 16 which assumes that short is 16bits (since it's related to writing the short* sound data into the file). This may not be the case on all platforms. So fBitsPerSample should either be sizeof(*sound) or being passed in.

  6. You use inconsistent bracing style:

     int main(int argc, char* argv[]){
    

vs

    if (!sound)
    {
  1. You have defined N_CHANNELS but the main loop is hard coded to increment by 2, so this

     for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    

should be

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += N_CHANNELS, j++)