Am I reconstructing the buffers correctly

Question

I am writing a networked audio application that sends the audio in 320 byte chunks, later I need to provide the player a bigger amount of bytes to play so I try to merge 2 or more chunks into one buffer and return it, well the code works but I hear some small audio artifacts, any ideas if the code is correct?

-(NSData*) getRawBufferWithSize:(int) requestedBufferSize
{
 if(audio.isBuffering && audio.bufferTimer>[[NSDate date] timeIntervalSince1970] && !AUDIO_BYPASS_NET)
 {
 return nil;
 }
 audio.isBuffering = NO;
 int readyBufferSize = 0;
 int bytesPending = requestedBufferSize;
 if(returnBufferSize<requestedBufferSize)
 {
 returnBuffer = (char*)realloc(returnBuffer, requestedBufferSize);
 returnBufferSize = requestedBufferSize;
 }
 while (readyBufferSize<requestedBufferSize)
 {
 NSData* newBuffer;
 @synchronized(playbackRawBuffer)
 {
 if([playbackRawBuffer count] != 0)
 {
 newBuffer = [[playbackRawBuffer objectAtIndex:0] retain];
 [playbackRawBuffer removeObjectAtIndex:0];
 }
 else
 {
 break;
 }
 } 
 char* bytes = (char*)[newBuffer bytes];
 int readableBytes = newBuffer.length;
 int amountOfBytesToRead = bytesPending;
 int amountOfBytesToSaveForLater = 0;
 if(readableBytes<amountOfBytesToRead)//if we can read less than we want
 {
 amountOfBytesToRead = readableBytes;
 }
 else if(readableBytes>amountOfBytesToRead)//if we can read more bytes than we want (we save the remaining bytes)
 {
 amountOfBytesToSaveForLater = readableBytes - amountOfBytesToRead;
 }
 bytesPending -= amountOfBytesToRead;
 memcpy((char*)returnBuffer+readyBufferSize, bytes, amountOfBytesToRead);
 [newBuffer release];
 readyBufferSize+=amountOfBytesToRead;
 if(amountOfBytesToSaveForLater>0)
 {//save these bytes back in the buffers array
 if(tBufSize<amountOfBytesToSaveForLater)
 {
 tBuf = (char*)realloc(tBuf, amountOfBytesToSaveForLater);
 tBufSize = amountOfBytesToSaveForLater;
 }
 memcpy(tBuf, &(bytes[amountOfBytesToRead]), amountOfBytesToSaveForLater);
 @synchronized(playbackRawBuffer)
 {
 [playbackRawBuffer insertObject:[NSData dataWithBytes:tBuf length:amountOfBytesToSaveForLater] atIndex:0];
 }
 break;
 }
 }
 if(readyBufferSize>0)
 {
 return [NSData dataWithBytes:returnBuffer length:readyBufferSize];
 }
 else
 {
 audio.isBuffering = YES;
 audio.bufferTimer = [[NSDate date] timeIntervalSince1970]+0.1;
 return nil;
 }
}

Answer 1

This is the wrong way to sue realloc:

returnBuffer = (char*)realloc(returnBuffer, requestedBufferSize);

If it fails you have just leaked the old returnBuffer.
The way realloc should be used is:

char* tmp = (char*)realloc(returnBuffer, requestedBufferSize);
if (tmp != NULL)
{
 returnBuffer = tmp;
}
else { /* Deal with realloc() failure */ }

But why are you dealing with this. I would use a std::vector<char> it deals with all the memory management issues for you (and your post id tagged C++).

TO be honest I have trouble following the rest of your code.

• Anton Banchev: Well in this case the requested buffer size is from 600 to 700 bytes of data, if realloc cannot manage to extend it to this size then I think I have more serious problems than a memory leak :)
• Loki Astari: @AntonBanchev: That's not really the point. You are writing code that is known bad pattern. I reject people from an interview for making such obvious mistakes.
• microtherion: +1 for "use std::vector", although maybe for this particular problem, std::deque would work even better.

Answer 2

I'm not sure what your sample rate is, but two 320-byte blocks put together is likely to be such a short time interval that merging them into 640 entirely wrong bytes would just sound like a quick "artifact."

What evidence do you have that this merge works correctly at all? It's hard to interpret on it's own, and if all you've got to go on currently is an audible artifact, there's a possibility that it is just 640 bytes of junknoise.

Suggestion: run the method on it's own with known input, expected output, and validate that it does merge at all.