JitterBufferPacket::data and data ownership

[Krzmbrzl]

The JitterBufferPacket::data field currently has type char *:

speexdsp/include/speex/speex_jitter.h

Line 61 in 095fd36

char *data; /**< Data bytes contained in the packet */

However, from the documentation it is not clear, whether the JitterBuffer will take ownership of the pointer or not. From the code I found here

speexdsp/libspeexdsp/jitter.c

Lines 440 to 447 in 095fd36

	if (jitter->destroy)
	{
	jitter->packets[i].data = packet->data;
	} else {
	jitter->packets[i].data=(char*)speex_alloc(packet->len);
	for (j=0;j<packet->len;j++)
	jitter->packets[i].data[j]=packet->data[j];
	}

it seems that it depends on whether JitterBuffer::destroy was set (via jitter-buffer_ctl) which I think could be translated as "If the caller has set up a special deleter for the jitter data, the JitterBuffer will take ownership of the contained data (in the sense that it calls that deleter on it once it is done with it) and otherwise it does not take ownership but instead copies the data to an internal buffer.

I think in the second scenario, it would be good for the overall semantics, if JitterBufferPacket::data would become const char * to indicate that it will not be modified. However, that probably causes issues with the first scenario and also with backwards compatibility.

So in the end, I think it should just be documented more explicitly what happens with the passed pointer and who is responsible for freeing it.

[Krzmbrzl]

Just found this, which supports my findings:

speexdsp/include/speex/speex_jitter.h

Lines 92 to 94 in 095fd36

	/** Assign a function to destroy unused packet. When setting that, the jitter
	buffer no longer copies packet data. */
	#define JITTER_BUFFER_SET_DESTROY_CALLBACK 4