Rawaudio dev

[dingodoppelt]

Add a new "raw" audio quality setting

This PR adds uncompressed audio ("raw") to the quality settings so there is no Opus compression along the way
Discussion in #3654

This feature improves latency as well. I gained 2ms by using uncompressed audio while having a better audio quality.

This is work in progress, please help me test it

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want
• My code follows the style guide
• I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• I've filled all the content above

[dingodoppelt]

This seems to cause issues from time to time. After closing the client I sometimes get an error with free() complaining about wrong sizes. Needs further investigation. Some help here would be appreciated

[ann0see]

I'd prefer not to check for the Jamulus version number but rather based on capabilities - we don't have 4.0.0 out yet and it might break during the dev process.

[dingodoppelt]

I'd prefer not to check for the Jamulus version number but rather based on capabilities - we don't have 4.0.0 out yet and it might break during the dev process.

I wanted to reuse information already available as much as possible so I just added the code where there were version checks already implemented. (For sequence number and pan feature)
Capabilities would be nice but also would require more changes to client, channel, server and protocol which I don't really have an idea on how to make that backwards compatible. We should rather replace all version checks with some capabilities struct that client and server can agree upon so everything lands in one place. I just don't feel like the right person to take on that challenge and rather pursue my hacky approach, as long as it works for everybody.
The version check with 4.0.0 could be replaced by a point release 3.11.1 and would work right away.

[ann0see]

Tested it and yes, the noise would be unacceptable. What is our fallback if max is selected but the server doesn't support it?

[dingodoppelt]

Tested it and yes, the noise would be unacceptable. What is our fallback if max is selected but the server doesn't support it?

I just noticed that if you connect to a server with Max selected you get the noise unless you switch audio quality again while connected. The server code is fine and doesn't need changes, I misplaced the check for my introduced bRawAudioSupported in the client code. I'll have a closer look
Edit: Funny, the noise doesn't happen on legacy servers, only on rawaudio :D

[dingodoppelt]

We still get crashes on windows, especially when using more coplex setups including audio routing software. Linux, Mac and android builds work fine so far. Sounds great but still needs more testing and fixes

[dingodoppelt]

The last commits fixed the crash on windows and make the client fall back to opus reliably. This is now ready to be tested thoroughly.

[dingodoppelt]

A buffersize of 256 on Max quality setting gives garbled audio and the packet sizes seem wrong and contain blocks of zeroes. Only that particular setting is affected. Opus still works

[softins]

A buffersize of 256 on Max quality setting gives garbled audio and the packet sizes seem wrong and contain blocks of zeroes. Only that particular setting is affected. Opus still works

I plan to try out this enhancement over the next few days. I've had a look through the diffs so far. Could you specify exactly the steps to produce this error?

[dingodoppelt]

A buffersize of 256 on Max quality setting gives garbled audio and the packet sizes seem wrong and contain blocks of zeroes. Only that particular setting is affected. Opus still works

I plan to try out this enhancement over the next few days. I've had a look through the diffs so far. Could you specify exactly the steps to produce this error?

This is reproducible with a buffersize of 256 samples only. The packets should be well below MTU and show a length of 1026 in wireshark.
This is may be related to the use of the conversion buffer or the iSndCrdFramSizeFactor not being used in packet size calculation.

Edit: The packets become bigger than the MTU allows for on 256 samples buffersize and get fragmented once I corrected the calculation of the packet sizes. Does this mean we need to disable raw audio for buffersizes of 256 or is there some mechanism to receive fragmented packets?

[softins]

I've just tried a build of rawaudio-dev here, between two separate hosts: server on a pi, client on a PC. It doesn't seem to be a MTU or fragmentation issue. The UDP packets are only 1068 bytes in size, and not fragmented.

Using a buffer size of 10.67ms (256) results in each packet containing two frames of audio, each with its own sequence number. In that setting, I was seeing one packet every 10.67ms coming from the Windows client, but still one packet every 5.33ms coming back from the server. They alternated between having zeros in the first frame and zeros in the second frame. So it could possibly be some issue in server.cpp that doesn't exist in client.cpp

Note that the client will encode according to the settings in the Client Settings dialog, but the server will encode according to the information in received in the NETW_TRANSPORT_PROPS message it received from the client.

Talking of which, the codec field in the NETW_TRANSPORT_PROPS message should specify a different value for RAW, rather than still saying OPUS, like this:

jamulus/src/util.h

Lines 484 to 492 in 849e823

	// Audio compression type enum -------------------------------------------------
	enum EAudComprType
	{
	// used for protocol -> enum values must be fixed!
	CT_NONE = 0,
	CT_CELT = 1,
	CT_OPUS = 2,
	CT_OPUS64 = 3 // using OPUS with 64 samples frame size
	};

So when sending props for raw encoding, it should either use CT_NONE or define a new CT_RAW=4.

[dingodoppelt]

I've just tried a build of rawaudio-dev here, between two separate hosts: server on a pi, client on a PC. It doesn't seem to be a MTU or fragmentation issue. The UDP packets are only 1068 bytes in size, and not fragmented.

This build is not taking into account iSndCrdFrameSizeFactor. From what I understood it should be mostly 1 and my code seems to only work when it is. iCeltNumCodedBytes should be multiplied by iSndCrdFrameSizeFactor. On 256 samples buffer size it will create packets that get fragmented. Wireshark shows the fragmentation. Should I push these changes for you to test? I might have gotten something fundamentally wrong here, but I'd say the problem is mainly in the client since the server happily plays back everything you throw at it.

[softins]

Ah, so the issue is that the client is not sending enough data to satisfy the server, and the server is therefore adding in packets of zeros to maintain the data rate.

Fragmentation should not be an issue, at least with IPv4, as fragmentation and re-assembly happens transparently at the IP layer. In fact, I don't think it will occur anyway, as the traffic from the server is not fragmented. We should just get packets from the client at 5.33ms instead of 10.67ms.

In fact, I've been doing some tests with Wireshark of all the various data rates, qualities and mono/stereo, and it seems that the packet interval is normally half the buffer time specified in the Client Settings. Except when "Small buffers" is not checked, and then 2.67 (64) is exactly the same as 5.33 (128).

[softins]

This build is not taking into account iSndCrdFrameSizeFactor. From what I understood it should be mostly 1 and my code seems to only work when it is. iCeltNumCodedBytes should be multiplied by iSndCrdFrameSizeFactor. On 256 samples buffer size it will create packets that get fragmented. Wireshark shows the fragmentation. Should I push these changes for you to test?

Yes please - I'm building directly from your rawaudio-dev branch.

[softins]

I think in client.cpp around line 1486, you need also to do a similar loop as a few lines above:

for ( i = 0, j = 0; i < iSndCrdFrameSizeFactor; i++, j += iNumAudioChannels * iOPUSFrameSizeSamples )

I don't have any more time today to try it...

[softins]

I think it would be better to recognise the audio frame by a sentinel byte. Protocol frames begin with 00 00 and must have a good checksum. Otherwise they are considered to be audio. Opus frames always begin with 00 for mono and 04 for stereo. So maybe for raw audio, the audio data could be prepended with a byte of f0 for mono and f4 for stereo? Then it could be recognised unambiguously. Both client and server need to recognise the format of a received frame correctly without relying on an out-of-band context.

[dingodoppelt]

I had misunderstood the packet size calculation and it seems fixed with the last commit.