xiph / speexdsp

Speex audio processing library - THIS IS A MIRROR, DEVELOPMENT HAPPENS AT https://gitlab.xiph.org/xiph/speexdsp
https://speex.org
Other
456 stars 187 forks source link

JitterBufferPacket::data and data ownership #37

Open Krzmbrzl opened 2 years ago

Krzmbrzl commented 2 years ago

The JitterBufferPacket::data field currently has type char *: https://github.com/xiph/speexdsp/blob/095fd36e189554bbcbfd9884630a53d7792409dc/include/speex/speex_jitter.h#L61

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 https://github.com/xiph/speexdsp/blob/095fd36e189554bbcbfd9884630a53d7792409dc/libspeexdsp/jitter.c#L440-L447 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 commented 2 years ago

Just found this, which supports my findings: https://github.com/xiph/speexdsp/blob/095fd36e189554bbcbfd9884630a53d7792409dc/include/speex/speex_jitter.h#L92-L94