Client audio transmission is currently synchronous, uses mutexes, and allocates memory - shouldn't all this be in its own thread?

[onar3d]

Hi!

I have a successful early prototype of Jamulus working as an audio plugin.

But I have in the process seen that for transmitting audio, several things happen in the audio callback method “CSoundBase::ProcessCallback”, which are very likely to cause audio artefacts/dropouts:

1: In CChannel::PrepAndSendPacket, there is a QMutexLocker. Locking is really to be avoided in the audio callback.

2: pSocket->SendPacket is invoked synchronously to send the packet. It should definitely instead be sending in its own thread, with a lock-free fifo buffer to feed the sending thread packets, no?

3: Then, in SendPacket there is also memory allocation, and further locking, but I guess by addressing 2 these are fine.

4: With CChannel::bDoAutoSockBufSize set to true, there’s further locking and possible allocation, which I guess is most simply resolved by keeping it as false.

Those are the ones I’ve identified till now.

My plan is to make Jamulus start another thread, just like the receiving one, also for sending.

Do you know, has anyone made anything similar that I can look at or will I be the first?

If you are aware of any fork/branch, or have any other tips on this, I’d love to know!

Thank you.

[WolfganP]

Are you talking about client or server side?
It may be worthy to read the discussion for multi-threads at #455 for insights on what was done server side and maybe provide more clarity to your questions.

[onar3d]

Are you talking about client or server side?
It may be worthy to read the discussion for multi-threads at #455 for insights on what was done server side and maybe provide more clarity to your questions.

I should have clarified this, yes, definitely Client side!

Server side there is no "real-time" audio thread that's as time sensitive as the audio callback in an audio plugin, there none of the points mentioned should be problems.

But of course the same kinds of changes can also be made to the server to improve its performance, I'll read the thread you linked, thank you!

[corrados]

and allocates memory

I tried not to allocate any memory. Where did you find places were memory is allocated?

Places which do allocate memory are still TODO and marked accordingly, e.g.
https://github.com/corrados/jamulus/blob/9e9c6043d46b5b1c023b04533b5257ab744bd3a5/src/socket.cpp#L266

[corrados]

In CChannel::PrepAndSendPacket, there is a QMutexLocker

That's uncritical. The lock only happens if you change your network properties, e.g., if you switch from Mono to Stereo. If you do not change any setting, you will never get in any lock condition there.

[corrados]

pSocket->SendPacket is invoked synchronously to send the packet

The only place where it can lock is sendto. But according to, e.g., https://stackoverflow.com/questions/4165174/when-does-a-udp-sendto-block, it only blocks if the buffer is full which should never be the case for normal Jamulus operation.

[corrados]

Then, in SendPacket there is also memory allocation

Can you please tell me the exact position in the code you are referring to?

[corrados]

CChannel::bDoAutoSockBufSize set to true, there’s further locking

This only locks if iCurSockBufNumFrames != iNewNumFrames which is only a very short fraction of time the case. I do not see any issue with that, actually.

If you want to start modifying the code, you have to proof that your code performs better then the existing code (which I doubt)...

[onar3d]

Hi @corrados, thank you very much for the quick reply!

Good to know about the contexts for the mutexes, I have not fully learned the code base so I was not certain what contexts they would be blocking in during real use, I agree that they are not a concern for normal use.

You are right that most likely in a desktop context there are very rarely going to be glitches from the sending of packets.

The memory allocation I mentioned was referring to the comments you yourself pointed out, I haven't found other points.

I am still considering making this additional thread for the sending of packets though, because from what I know (but I am no expert), in the context I was hoping to run this plugin, (linux preemptRT), any locking or allocation is very sensitive as it causes a context switch from the real-time designated thread.

So it is not so much towards performance improvement in terms of total CPU utilization, but to ensure this audio thread just never gets interrupted for any reason, even if that may mean extra CPU use in the remainder of the threads.

Thanks!

[corrados]

I am still considering making this additional thread for the sending of packets though, because from what I know (but I am no expert), in the context I was hoping to rum this plugin, (linux preemptRT), any locking or allocation is very sensitive as it causes a context switch from the real-time designated thread.

Would be interesting to know if it makes a difference when running Jamulus as a plugin.

BTW: What plugin type are you developing? Many years ago I implemented some tests for VST integration, see, e.g., https://github.com/corrados/jamulus/blob/master/src/vstmain.cpp and #82.

[onar3d]

Currently it's a VST2 indeed, where your old implementation was a good starting point, but I am undecided as to whether I'll keep it as such.

[ann0see]

Hi @onar3d are you still working on it? We're planning to move the VST code to a feature branch #2405 since no official work is planned.

[onar3d]

Hi!
No, I haven't looked at it in a good while, thank you for checking back with me though!

[ann0see]

Ok. If you plan working on a plug-in just feel free to do so (open issue/PR/Discussion).